Allow foreign keys on nonlocal tables #9989

nicktobey · 2025-10-23T08:18:15Z

This change allows users to add foreign key relations on nonlocal tables (table names that match entries in dolt_nonlocal_tables and resolve to tables on other branches).

The biggest obstacle was that the foreign key verification logic operates on the DB directly, but resolving the references in the dolt_nonlocal_tables table requires a Dolt SQL engine. Because we now use the engine for most operations, the engine is guaranteed to exist, but it wasn't obvious how to allow the storage code to access the engine in a way that didn't break encapsulation or create dependency cycles.

The way this PR accomplishes this is by creating a new interface called doltdb.TableResolver, which has the method GetDoltTableInsensitiveWithRoot, which can resolve table names at a supplied root value. This object can be instantiated by the Dolt Session and passed into the DB layer.

I'm not thrilled about adding the extra confusingly similar methods to doltdb.Database, but hopefully the differences between them are clear.

…g table names on roots. This is used for code in doltdb that needs to account for dolt_nonlocal_tables.

…te.sh

coffeegoddd · 2025-10-23T08:50:40Z

@nicktobey DOLT

comparing_percentages
100.000000 to 100.000000

version	result	total
`bd1b42e`	ok	5937471

version	total_tests
`bd1b42e`	5937471

correctness_percentage
100.0

coffeegoddd · 2025-10-23T08:57:33Z

@coffeegoddd DOLT

comparing_percentages
100.000000 to 100.000000

version	result	total
`d9076bd`	ok	5937471

version	total_tests
`d9076bd`	5937471

correctness_percentage
100.0

coffeegoddd · 2025-10-24T00:09:04Z

@nicktobey DOLT

comparing_percentages
100.000000 to 100.000000

version	result	total
`3f583e6`	ok	5937471

version	total_tests
`3f583e6`	5937471

correctness_percentage
100.0

…d add docstrings.

coffeegoddd · 2025-10-24T01:20:59Z

@nicktobey DOLT

comparing_percentages
100.000000 to 100.000000

version	result	total
`4b44b80`	ok	5937471

version	total_tests
`4b44b80`	5937471

correctness_percentage
100.0

zachmu

Overall seems fine, the table resolution logic is getting away from us but I don't know if you need to address that right now.

Testing should be shored up a bit.

I can't see any obvious reason this is breaking Doltgres. I would expect that some place resolve.Table() was getting called is now not being called, but I didn't see any place that was happening. If you have a link to specific failures I might have better advice.

zachmu · 2025-10-24T18:27:16Z

go/libraries/doltcore/doltdb/doltdb.go

+// 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)


This is kind of a mouthful, what about just Resolve or ResolveTable

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.

zachmu · 2025-10-24T18:28:12Z

go/libraries/doltcore/doltdb/doltdb.go

+	}
+	trueTableName = TableName{
+		Name:   trueTableNameString,
+		Schema: tblName.Schema,


Probably should be trueTableName.Schema. I don't know if ResolveTableName actually case-corrects the schema name but it probably should.

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.

zachmu · 2025-10-24T19:13:55Z

go/libraries/doltcore/sqle/database.go

+	}
+}
+
+func (db Database) getNonlocalTableNames(ctx *sql.Context, root doltdb.RootValue) (nonlocalTableNames []string, error error) {


Method comment

zachmu · 2025-10-24T19:17:36Z

go/libraries/doltcore/sqle/database.go

 }

-func (db Database) getAllTableNames(ctx *sql.Context, root doltdb.RootValue, includeGeneratedSystemTables bool, includeRootObjects bool) ([]string, error) {
+func (db Database) getAllTableNames(ctx *sql.Context, root doltdb.RootValue, includeGeneratedSystemTables bool, includeRootObjects bool, includeNonlocalTables bool) ([]string, error) {


Don't use use a type alias for includNonLocalTables elsewhere?

zachmu · 2025-10-24T19:24:12Z

go/libraries/doltcore/sqle/dsess/session.go

 	return d.provider
 }

+func (d *DoltSession) GetTableResolver(ctx *sql.Context, dbName string) (doltdb.TableResolver, error) {


Not clear why this needs to live here.

It requires the session provider?

I inlined it into the GetTableResolver helper function in the same file and got rid of the explicit cast.

zachmu · 2025-10-24T19:27:22Z

go/libraries/doltcore/sqle/tables.go

 }

+func (t *WritableDoltTable) getDoltTableForFK(ctx *sql.Context, root doltdb.RootValue, lwrTableName string, sqlFk sql.ForeignKeyConstraint) (refTbl *doltdb.Table, err error) {
+	_, refTbl, nonlocalTableExists, err := t.db.getNonlocalDoltDBTable(ctx, root, doltdb.TableName{Name: lwrTableName, Schema: sqlFk.ParentSchema})


Not sure if this is the right time for this but food for thought:

The logic for resolving tables is getting more and more complex and special-cased. It would good to encapsulate it better, and to return some kind of TableMetadata object to inform interested callers about things like "this is a system table", "this table was resolved with a non-local override", etc.

100% agree. This is only going to get messier and would benefit from some kind of general interface.

I don't think this PR is the right place for that, but it might make sense to do before the next time we have to make things even more complicated. I can ruminate on approaches.

zachmu · 2025-10-24T19:28:55Z

integration-tests/bats/nonlocal.bats

  [[ "$output" =~ "Cannot commit changes on more than one branch / database" ]] || false
 }

 @test "nonlocal: test foreign keys" {


These are good but it would be nice to have some engine tests for this capability as well.

For engine tests, I would expect to see e.g. tests that foreign key constraints are correctly enforced and that they resolve to the correct root, etc.

coffeegoddd · 2025-10-26T09:09:46Z

@nicktobey DOLT

comparing_percentages
100.000000 to 100.000000

version	result	total
`0f9d804`	ok	5937471

version	total_tests
`0f9d804`	5937471

correctness_percentage
100.0

coffeegoddd · 2025-10-28T21:32:15Z

@nicktobey DOLT

comparing_percentages
100.000000 to 100.000000

version	result	total
`ba3fbd0`	ok	5937471

version	total_tests
`ba3fbd0`	5937471

correctness_percentage
100.0

coffeegoddd · 2025-10-29T19:19:45Z

@nicktobey DOLT

comparing_percentages
100.000000 to 100.000000

version	result	total
`0e2ddb4`	ok	5937471

version	total_tests
`0e2ddb4`	5937471

correctness_percentage
100.0

github-actions · 2025-10-30T02:39:43Z

@coffeegoddd DOLT

test_name	detail	row_cnt	sorted	mysql_time	sql_mult	cli_mult
batching	LOAD DATA	10000	1	0.05	2
batching	batch sql	10000	1	0.07	2
batching	by line sql	10000	1	0.08	1.63
blob	1 blob	200000	1	0.91	4.31	4.25
blob	2 blobs	200000	1	0.88	4.85	4.95
blob	no blob	200000	1	0.89	2.99	2.78
col type	datetime	200000	1	0.83	2.87	2.83
col type	varchar	200000	1	0.69	3.94	3.68
config width	2 cols	200000	1	0.74	3.07	3.12
config width	32 cols	200000	1	1.9	2.93	2.81
config width	8 cols	200000	1	0.96	3.04	3.29
pk type	float	200000	1	0.83	2.86	2.94
pk type	int	200000	1	0.81	3.05	2.85
pk type	varchar	200000	1	1.54	1.86	2.19
row count	1.6mm	1600000	1	5.71	3.36	3.35
row count	400k	400000	1	1.44	3.26	3.28
row count	800k	800000	1	2.85	3.31	3.34
secondary index	four index	200000	1	3.66	1.55	1.45
secondary index	no secondary	200000	1	0.88	3.06	2.86
secondary index	one index	200000	1	1.12	2.93	2.92
secondary index	two index	200000	1	1.98	2.07	1.82
sorting	shuffled 1mm	1000000	0	5.25	3.15	3.17
sorting	sorted 1mm	1000000	1	5.37	3.11	3.08

github-actions · 2025-10-30T02:47:32Z

@coffeegoddd DOLT

name	detail	mean_mult
dolt_blame_basic	system table	1.19
dolt_blame_commit_filter	system table	2.89
dolt_commit_ancestors_commit_filter	system table	0.64
dolt_commits_commit_filter	system table	1.04
dolt_diff_log_join_from_commit	system table	2.9
dolt_diff_log_join_to_commit	system table	2.96
dolt_diff_table_from_commit_filter	system table	1.2
dolt_diff_table_to_commit_filter	system table	1.2
dolt_diffs_commit_filter	system table	1.06
dolt_history_commit_filter	system table	1.45
dolt_log_commit_filter	system table	1.04

github-actions · 2025-10-30T03:06:29Z

@coffeegoddd DOLT

name	add_cnt	delete_cnt	update_cnt	latency
adds_only	60000	0	0	0.86
adds_updates_deletes	60000	60000	60000	4.49
deletes_only	0	60000	0	2.15
updates_only	0	0	60000	2.8

nicktobey and others added 9 commits October 23, 2025 01:16

Rename local variables pertaining to nonlocal tables.

d613452

Allow creating foreign key on nonlocal table.

6d70219

nonlocal tables appear in show_tables

ecd150a

Prevent creating a table if it would clash with a nonlocal_tables rule.

9ca908f

Create TableResolver interface for allowing custom rules for resolvin…

34ca74a

…g table names on roots. This is used for code in doltdb that needs to account for dolt_nonlocal_tables.

Propagate a TableResolver into foreign key logic.

139bced

Support nonlocal tables in foreign key verification

963e485

Update nonlocal table bats tests.

bd1b42e

[ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/upda…

d9076bd

…te.sh

coffeegoddd added the correctness_approved label Oct 23, 2025

Fix missing return when validating a FK pointing to a system table.

3f583e6

Rename added Database methods to clarify they return doltdb.Table, an…

4b44b80

…d add docstrings.

zachmu approved these changes Oct 24, 2025

View reviewed changes

Avoid dropping schema name when doing lookup.

0f9d804

nicktobey added 4 commits October 28, 2025 13:59

Refactor uses of TableResolver interface

fd470d8

Ensure uses of readNonlocalTables type

9a163d5

Add docstrings

3ca970a

Add engine tests for nonlocal tables

ba3fbd0

Add case sensitivity option to TableResolver interface.

0e2ddb4

nicktobey force-pushed the nicktobey/nonlocal-fks branch from 21158f5 to 0e2ddb4 Compare October 29, 2025 18:48

nicktobey merged commit ce73118 into main Oct 29, 2025
22 of 27 checks passed

BrewTestBot mentioned this pull request Nov 4, 2025

dolt 1.76.4 Homebrew/homebrew-core#252601

Merged

Hydrocharged deleted the nicktobey/nonlocal-fks branch December 15, 2025 06:49

Uh oh!

Allow foreign keys on nonlocal tables #9989

Allow foreign keys on nonlocal tables #9989

Uh oh!

Conversation

nicktobey commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coffeegoddd commented Oct 23, 2025

Uh oh!

coffeegoddd commented Oct 23, 2025

Uh oh!

coffeegoddd commented Oct 24, 2025

Uh oh!

coffeegoddd commented Oct 24, 2025

Uh oh!

zachmu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coffeegoddd commented Oct 26, 2025

Uh oh!

coffeegoddd commented Oct 28, 2025

Uh oh!

coffeegoddd commented Oct 29, 2025

Uh oh!

Uh oh!

github-actions bot commented Oct 30, 2025

Uh oh!

github-actions bot commented Oct 30, 2025

Uh oh!

github-actions bot commented Oct 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nicktobey commented Oct 23, 2025 •

edited

Loading