Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metadata tables #17

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

dpordomingo
Copy link
Member

@dpordomingo dpordomingo commented Mar 10, 2017

WIP notice

  • more tests,

PR Description

I tried to follow these concept from mysql docs

INFORMATION_SCHEMA is a database within each MySQL instance, the place that stores information about all the other databases that the MySQL server maintains.
The INFORMATION_SCHEMA database contains several read-only tables. They are actually views, not base tables

Advantages of SELECT syntax:

  • It conforms to Codd's rules, because all access is done on tables. (https://en.wikipedia.org/wiki/Codd's_12_rules#Rules)
  • You can use the familiar syntax of the SELECT statement, and only need to learn some table and column names.
  • You can filter, sort, concatenate, and transform the results from INFORMATION_SCHEMA queries into whatever format your application needs.

If this PR is merged, it could be done the following:

SELECT * FROM SCHEMATA;
SELECT schema_name FROM SCHEMATA where schema_name="gitql";

SELECT * FROM TABLES;
SELECT table_name FROM TABLES WHERE table_schema="gitql";

SELECT * FROM COLUMNS;
SELECT column_name, data_type, is_nullable, column_key, column_default FROM COLUMNS WHERE table_name="commits";

Design:

The metadata –new– package, includes the implementation of the new INFORMATION_SCHEMA database, that contains the three metadata tables: SCHEMATA, TABLES and COLUMNS, that embeds mem.Database and mem.Table respectively.
The –new– three tables behaves like a view so they doesn't store data nor are writable.

Other changes, required by the main development:

  • Databases should be added through sql.Engine::AddDatabase(db sql.Database) error method,
  • sql.Catalog is now an Interface and operations over its handled databases should be done through its exposed methods:
    • Database(name string) (Database, error)
    • AddDatabase(db Database) error
    • NEW: Dbs() Databases
    • NEW: Table(dbName string, tableName string) (Table, error)
  • tables are fetched from the Database through the sql.Database::Table(string) (Table, error) method instead of accessing the database tables directly,
  • mem.NewCIDatabase(name string) constructor will build a mem/Database that will store the tables in a case insensitive way,
  • mem.Table embeds a new mem.underlayingData that handles the storage, row iteration...,
  • sql.Node implements fmt.Stringer in order to be debugging friendly,
  • it is implemented three new iterators:
    • DBIterator, iterates over the sql.Database handled by the given sql.Catalog,
    • TableIterator, iterates over the sql.Table stored by the given, for sql.Database,
    • ColumnIterator, iterates over the sql.Column defined by the sql.Schema of the passed sql.Table,

@codecov-io
Copy link

codecov-io commented Mar 10, 2017

Codecov Report

Merging #17 into master will decrease coverage by 0.24%.
The diff coverage is 54.59%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #17      +/-   ##
==========================================
- Coverage    53.3%   53.06%   -0.25%     
==========================================
  Files          34       41       +7     
  Lines        1724     2039     +315     
==========================================
+ Hits          919     1082     +163     
- Misses        743      885     +142     
- Partials       62       72      +10
Impacted Files Coverage Δ
sql/functionregistry.go 92.59% <ø> (ø) ⬆️
sql/plan/show_tables.go 57.14% <0%> (-3.47%) ⬇️
sql/plan/cross_join.go 60.65% <0%> (-2.06%) ⬇️
sql/plan/project.go 58.82% <0%> (-12.61%) ⬇️
sql/plan/group_by.go 70.83% <0%> (-8.61%) ⬇️
sql/unresolved_database.go 0% <0%> (ø) ⬆️
sql/core.go 0% <0%> (ø)
sql/plan/limit.go 54.83% <0%> (-3.79%) ⬇️
sql/plan/values.go 0% <0%> (ø) ⬆️
sql/row.go 50% <0%> (-3.13%) ⬇️
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 76770b0...fa7b443. Read the comment docs.

@dpordomingo dpordomingo force-pushed the metadata-tables branch 5 times, most recently from 66ceda6 to e5f762a Compare March 10, 2017 10:44
engine_test.go Outdated
@@ -110,7 +110,7 @@ func newEngine(t *testing.T) *sqle.Engine {
db.AddTable("mytable", table)

e := sqle.New()
e.AddDatabase(db)
assert.Nil(e.AddDatabase(db))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert.NoError if the return type is error.

mem/database.go Outdated
@@ -22,6 +22,6 @@ func (d *Database) Tables() map[string]sql.Table {
return d.tables
}

func (d *Database) AddTable(name string, t *Table) {
d.tables[name] = t
func (d *Database) AddTable(t sql.Table) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Makes sense. If we want to change the name of a table we could probably add a wrapper TableAlias in the future.

mem/table.go Outdated
@@ -4,13 +4,14 @@ import (
"fmt"
"io"

"gopkg.in/sqle/sqle.v0/memory"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why there is mem and memory package?

data IteratorData
}

func NewIter(data IteratorData) *Iter {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to make this type private and expose only the RowIter interface.

sql/catalog.go Outdated
Databases
FunctionRegistry
}

type DBStorer interface {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to Catalog

@dpordomingo dpordomingo force-pushed the metadata-tables branch 2 times, most recently from 68777a4 to 738af41 Compare April 10, 2017 07:21
@dpordomingo dpordomingo force-pushed the metadata-tables branch 6 times, most recently from 1ed786c to e15b2df Compare July 17, 2017 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants