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.

Sign up for GitHub

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

Jump to bottom

Zachmu/query save feature #375

Merged
merged 38 commits into from
Feb 7, 2020
Merged

Zachmu/query save feature #375

merged 38 commits into from
Feb 7, 2020

Conversation

zachmu
Copy link
Member

@zachmu zachmu commented Feb 5, 2020

Implemented query saving via new dolt_query_catalog table, created by dolt sql -q -s.

Also:

  • Unified business logic for validating table names in every code path where a table is created
  • Separated out read-only, read-write, and alterable SQL tables
  • Refactored deeply nested error handling logic in mv and cp commands
  • Refactored import command to separate validation from execution logic
  • Added many tests

@zachmu
Added query saving args to dolt sql command
a0e0444 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
First pass at dolt query saving
80277c9 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Fixed off-by-one error in id assignment
016014a 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Hid dolt_query_catalog from show tables
ccdff29 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Better arg validation for saving a query. Added an order column to qu…
6fe1a87 
…ery catalog table.

Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Tests for query catalog table utility method
6f8dee5 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Moved dolt table namespace logic to doltdb package, and introduce IsS…
119dd5f 
…ytemTable method.

Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Added system table check for table creation in dolt table import, dol…
812a7e0 
…t schema import

Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Refactored table import validation logic to make it reusable
340bbf5 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Added table name validation to mv and cp, standardized table name err…
bd73290 
…or handling for schema and table import.

Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Added bats test for name validation on table import. Fixed a bug in v…
bad4e83 
…alidation logic.:

Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Added bats tests for table cp and table mv commands
3a52be0 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Added name validation tests to schema import bats tests
1e33bd0 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Disallow using reserved table names for all SQL code paths. Standardi…
e763e9a 
…zed error messages.

Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Tests for inserting into system tables.
dc16687 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Broke DoltTable out into 3 interfaces: the original (read only), Writ…
05e467d 
…ableDoltTable, and AlterableDoltTable. Wrote tests for writing into dolt_schemas table.

Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Bug fix for readability of tables in SQL execution
a7e7a53 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Fixed bugs in view creation, added tests
3d2a4fd 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Update tests for system tables
d032bf5 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Fully working update tests for system tables
5e4420b 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Delete tests for system tables
ed7f24b 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Replace tests for system tables
b04d98b 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Select tests for system tables
8b06ecb 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Tests of altering system tables. Failing.
f15143c 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
More iteration on alter system tables. This relies on a bug fix in go…
382dfe2 
…-mysql-server.

Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Working tests for altering system tables
d0ba56e 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Updating version of go-mysql-server
9870161 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
go mod tidy
0146eb3 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Handle error on query saving
e7fc9e8 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Added bats test for query saving
bda8c01 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Formatting
c4afb14 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu zachmu requested a review from bheni February 5, 2020 22:28
@zachmu
Fixed compile error
a81337b 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Removed special-casing of dolt_docs table from schema, mv, and cp com…
6545a8e 
…mands, and fixed tests.

Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Formatting
b4315d6 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu zachmu requested a review from timsehn February 5, 2020 23:53
timsehn
timsehn reviewed Feb 6, 2020
View reviewed changes
bats/sql.bats Outdated Show resolved Hide resolved
@zachmu
PR feedback: moved query catalog to its own bats test
7afff33 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
bheni
bheni approved these changes Feb 6, 2020
View reviewed changes
Copy link
Contributor

@bheni bheni left a comment

Choose a reason for hiding this comment

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

Good. Most of my comments were about the same thing. Comments on exported items should begin with the exact name and casing of the item being described.

go/cmd/dolt/commands/sql.go Outdated Show resolved Hide resolved
go/cmd/dolt/commands/sql.go Outdated Show resolved Hide resolved
go/cmd/dolt/commands/tblcmds/cp.go Outdated Show resolved Hide resolved
go/libraries/doltcore/doltdb/root_val.go Outdated Show resolved Hide resolved
go/libraries/doltcore/sqle/query_catalog_table.go Show resolved Hide resolved
go/libraries/doltcore/sqle/database.go Show resolved Hide resolved
go/libraries/doltcore/sqle/database.go Show resolved Hide resolved
go/libraries/doltcore/sqle/database.go Show resolved Hide resolved
go/libraries/doltcore/sqle/database.go Show resolved Hide resolved
go/libraries/doltcore/sqle/schema_table.go Show resolved Hide resolved
@zachmu
PR feedback. Consolidated two flags in the sql command, and added a b…
7e27ebc 
…ats test for conflicts.

Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Doc improvements for SQL command
de41aa1 
Signed-off-by: Zach Musgrave <zach@liquidata.co>
@zachmu
Merge branch 'master' into zachmu/query-save-feature
a3a58ce
@zachmu zachmu merged commit d02ea00 into master Feb 7, 2020
@Hydrocharged Hydrocharged deleted the zachmu/query-save-feature branch February 18, 2020 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timsehn timsehn timsehn left review comments

@bheni bheni bheni approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@zachmu @timsehn @bheni