-
-
Notifications
You must be signed in to change notification settings - Fork 599
Allow foreign keys on nonlocal tables #9989
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
Changes from 11 commits
d613452
6d70219
ecd150a
9ca908f
34ca74a
139bced
963e485
bd1b42e
d9076bd
3f583e6
4b44b80
0f9d804
fd470d8
9a163d5
3ca970a
ba3fbd0
0e2ddb4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,6 +76,30 @@ var InMemDoltDB = "mem://" | |
| var ErrNoRootValAtHash = errors.New("there is no dolt root value at that hash") | ||
| var ErrCannotDeleteLastBranch = errors.New("cannot delete the last branch") | ||
|
|
||
| // TableResolver allows the user of a DoltDB to configure how table names are resolved on roots. | ||
| // This is useful because the user-backed system table dolt_nonlocal_tables allows table names to resolve to | ||
| // tables on other refs, but sqle.Database is necessary to resolve those refs. | ||
| type TableResolver interface { | ||
| GetDoltDBTableInsensitiveWithRoot(ctx *sql.Context, root RootValue, tblName TableName) (trueTableName TableName, table *Table, found bool, err error) | ||
| } | ||
|
|
||
| type SimpleTableResolver struct{} | ||
|
|
||
| var _ TableResolver = SimpleTableResolver{} | ||
|
|
||
| func (t SimpleTableResolver) GetDoltDBTableInsensitiveWithRoot(ctx *sql.Context, root RootValue, tblName TableName) (trueTableName TableName, table *Table, found bool, err error) { | ||
| trueTableNameString, exists, err := root.ResolveTableName(ctx, tblName) | ||
| if err != nil || !exists { | ||
| return TableName{}, nil, false, err | ||
| } | ||
| trueTableName = TableName{ | ||
| Name: trueTableNameString, | ||
| Schema: tblName.Schema, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably should be trueTableName.Schema. I don't know if ResolveTableName actually case-corrects the schema name but it probably should.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ResolveTableName only returns the table name string, and looking at the current implementation, having it case-correct the schema and return the normalized schema may require additional changes to the rootValueStorage interface. I agree it should probably both case-correct the schema name and return a TableName, and I think migrating to using TableName everywhere is important, but that seems like it would be a larger refactor that's probably better saved for a separate PR. |
||
| } | ||
| table, exists, err = root.GetTable(ctx, trueTableName) | ||
| return trueTableName, table, exists, err | ||
| } | ||
|
|
||
| // DoltDB wraps access to the underlying noms database and hides some of the details of the underlying storage. | ||
| type DoltDB struct { | ||
| db hooksDatabase | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is kind of a mouthful, what about just Resolve or ResolveTable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because doltdb.Database implements it and I needed the method name to distinguish itself from all of the other table name resolution methods. And it's not infeasible that we might need to add other methods to this interface that differ in subtle ways (although we should make attempts not to do so.)
But you're right that this interface shouldn't pay for the Database type's sins. I made a wrapper type in doltdb to implement the interface instead, and renamed the interface.