Refactor AutoIncrementTracker into generitc type with interfaces by nicktobey · Pull Request #11337 · dolthub/dolt

nicktobey · 2026-07-22T06:15:57Z

The purpose of this PR is to enable Doltgres to create a global state for Sequences that behaves similarly to Dolt's global state for auto increment columns. In order to allow for code reuse, we have to do the following:

Create a generic type containing the behavior common to both auto-increment and sequences.
Create new interfaces that can be implemented to describe the behavior unique to each feature.

(Bikeshedding note: I use the term "Relation" to describe a top-level object in the database, and encompasses both tables and root objects. I'm open to alternative names.)

The new interfaces can be found in go/libraries/doltcore/sqle/globalstate/sequences/state.go

SequenceState is an interface for a state machine that can be incremented. Each time it is incremented, it produces a new SQL value. SequenceStates can also be compared to each other to determine which one is "further along" and "merged" to create a single state that is further along in its sequence than any of the input states. The only implementation in Dolt is AutoIncrementState, which is just a uint64. Doltgres sequences will have a more complicated implementation, since they have more parameters that affect how the state is incremented.
SequencedRelation is a relation that holds a SequenceState. The only implementation in Dolt is doltdb.Table, which can hold an auto-increment value. Doltgres Sequences will also implement this interface, and possibly Doltgres tables with a SERIAL column.

The common logic is in the SequenceTracker type, which has an interface in go/libraries/doltcore/sqle/globalstate/sequence_tracker.go and an implementation in go/libraries/doltcore/sqle/dsess/sequence_tracker.go

AutoIncrementTracker is now a type alias for a specialization of SequenceTracker.

Finally, while GlobalState previously only held an AutoIncrementTracker, it now contains a map from arbitrary keys to SequenceTrackers. This will allow Doltgres to add an additional SequenceTracker for global tracking of Doltgres sequences.

There are a couple places in this PR that are TODOs, because they are corner cases that aren't reachable for AutoIncrement but are reachable for Doltgres sequences. They mostly concern how to handle when branches contain incompatible Sequences. I haven't decided exactly how this should be handled, and this PR will not be merged until we know what to do in that case, but it shouldn't affect the review of the rest of the PR.

…te.sh

coffeegoddd · 2026-07-22T06:39:55Z

@nicktobey DOLT

read_tests	from_latency	to_latency	percent_change
covering_index_scan	2.3	2.3	0.0
groupby_scan	144.97	144.97	0.0
index_join	1.93	1.93	0.0
index_join_scan	1.32	1.32	0.0
index_scan	227.4	223.34	-1.79
oltp_point_select	0.25	0.25	0.0
oltp_read_only	5.0	5.0	0.0
select_random_points	0.51	0.51	0.0
select_random_ranges	0.64	0.64	0.0
table_scan	204.11	204.11	0.0
types_table_scan	475.79	475.79	0.0

write_tests	from_latency	to_latency	percent_change
oltp_delete_insert	6.21	6.09	-1.93
oltp_insert	3.13	3.13	0.0
oltp_read_write	11.24	11.24	0.0
oltp_update_index	3.3	3.3	0.0
oltp_update_non_index	3.02	3.02	0.0
oltp_write_only	6.32	6.21	-1.74
types_delete_insert	6.79	6.79	0.0

coffeegoddd · 2026-07-22T06:48:08Z

@nicktobey DOLT

comparing_percentages
100.000000 to 100.000000

version	result	total
`3fc7f98`	ok	5937471

version	total_tests
`3fc7f98`	5937471

correctness_percentage
100.0

coffeegoddd · 2026-07-22T06:51:06Z

@nicktobey DOLT

test_name	from_latency_p95	to_latency_p95	percent_change
tpcc-scale-factor-1	45.79	45.79	0.0

test_name	from_server_name	from_server_version	from_tps	to_server_name	to_server_version	to_tps	percent_change
tpcc-scale-factor-1	dolt	`a0b1bf5`	52.75	dolt	`ddaf2b8`	53.16	0.78

coffeegoddd · 2026-07-22T06:55:21Z

@coffeegoddd DOLT

comparing_percentages
100.000000 to 100.000000

version	result	total
`d419eb8`	ok	5937471

version	total_tests
`d419eb8`	5937471

correctness_percentage
100.0

coffeegoddd · 2026-07-29T00:43:41Z

@nicktobey DOLT

comparing_percentages
100.000000 to 100.000000

version	result	total
`ddaf2b8`	ok	5937471

version	total_tests
`ddaf2b8`	5937471

correctness_percentage
100.0

Hydrocharged

LGTM! Just some minor nitpicks

Hydrocharged · 2026-07-27T12:47:17Z

@@ -0,0 +1,56 @@
+// Copyright 2023 Dolthub, Inc.


Wrong year (true for other new files as well)

Hydrocharged · 2026-07-29T13:21:19Z

-		mm:         mutexmap.NewMutexMap(),
-		init:       make(chan struct{}),
-		cancelInit: make(chan struct{}),
+func NewSequenceTrackerFromRoots[


In general, I think it would be worthwhile to add function and struct comments, in case someone else needs to modify these in the future.

Hydrocharged · 2026-07-29T13:37:12Z

-
-			if seq > newHighestValue {
-				newHighestValue = seq
+			if newHighestValue == nil {


It's a nitpick, but technically for Doltgres, negative sequences aren't the highest values, so I'd give this a different name.

Hydrocharged · 2026-07-29T13:38:27Z

+	// Next returns the next SQL value produced by the given relation, and advances that relation's state.
+	Next(ctx *sql.Context, relation string, insertVal interface{}) (ValueType, error)
+	// AddNewTable adds a new table to the tracker, initializing the auto increment value to the provided |initialState|.
+	AddNewTable(relation string, initialState StateType) error


Maybe AddNewRelation instead?

zachmu

Overall looks good, one pretty annoying comment about using doltdb.TableName in these interface.

zachmu · 2026-07-29T20:22:56Z

+
+// SequenceTracker knows how to get and set the current auto increment value for a relation (a table or a root object).
+// It's defined as an interface here because implementations need to reach into session state, requiring a dependency on this package.
+type SequenceTracker[


Unfortunately the tableName keys here should all be doltdb.TableName, not string. Otherwise you're going to run into issues with schemas in doltgres as soon as someone does something dumb. (this wasn't an issue before since Doltgres didn't consume these interfaces, so it missed the big pass that turned strings into doltdb.TableName)

Same with the Base interface

zachmu · 2026-07-29T20:24:00Z

+	DropTable(ctx *sql.Context, relation string, wses ...*doltdb.WorkingSet) error
+	// InitWithRoots fills the SequenceTracker with values pulled from each root in order.
+	InitWithRoots(ctx context.Context, roots ...doltdb.Rootish) error
+	Close()


Should document the contract and expectations for close

zachmu · 2026-07-29T20:24:44Z

+)
+
+// SequenceTrackerBase is the non-generic base interface for SequenceTracker
+// It is useful for establishing an upper bound on type parameters in some circumstances.


Is this the best way to do this? Don't want you to rat-hole too deep but this is a little odd.

zachmu · 2026-07-29T20:24:57Z

+	// Next returns the next SQL value produced by the given relation, and advances that relation's state.
+	Next(ctx *sql.Context, relation string, insertVal interface{}) (ValueType, error)
+	// AddNewTable adds a new table to the tracker, initializing the auto increment value to the provided |initialState|.
+	AddNewTable(relation string, initialState StateType) error


zachmu · 2026-07-29T20:25:41Z

+	"github.com/dolthub/go-mysql-server/sql"
+)
+
+// This package is a collection of interfaces that describe state machines that are controlled by the global state.


Isn't there a golang way to put a package-level comment-only file? Like doc.go or something?

zachmu · 2026-07-29T20:33:45Z

+	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/globalstate"
+)
+
+type DoltDBRelationSource struct{}


Could use a comment, this is a bit mysterious on its own

zachmu · 2026-07-29T20:40:19Z

+	ValueType comparable,
+] interface {
+	GetRelation(ctx context.Context, root doltdb.RootValue, tName doltdb.TableName) (relation RelationType, resolvedName string, found bool, err error)
+	GetRelations(ctx context.Context, root doltdb.RootValue, cb func(doltdb.TableName, RelationType) (bool, error)) error


Probably should be called IterRelations.

Might consider returning a seq.Iter instead of passing a callback func

zachmu · 2026-07-29T20:46:42Z

+	StateType sequences.SequenceState[StateType, ValueType],
+	ValueType comparable,
+] interface {
+	GetRelation(ctx context.Context, root doltdb.RootValue, tName doltdb.TableName) (relation RelationType, resolvedName string, found bool, err error)


Method doc plz

zachmu · 2026-07-29T20:48:49Z

 }

-func (a *AutoIncrementTracker) initializeTableAutoIncrement(ctx *sql.Context, tableName string) (uint64, bool, error) {
+func (a *SequenceTracker[RelationType, StateType, ValueType]) initializeTableAutoIncrement(ctx *sql.Context, tableName string, initialValue interface{}) (state StateType, hasState bool, err error) {


Probably want to rename these receiver params now

nicktobey and others added 5 commits July 16, 2026 14:09

Allow GlobalState to have multiple AutoIncrementTrackers

a8eba90

Add type-safe synchronized Map implementation.

debe9fe

Add globalstate/sequences package of interfaces

754c6bf

Have AutoIncrementTracker implement SequenceTracker

3fc7f98

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

d419eb8

…te.sh

coffeegoddd added the performance_approved label Jul 22, 2026

coffeegoddd added the correctness_approved label Jul 22, 2026

nicktobey added 2 commits July 28, 2026 17:12

Rename function to NewSequenceTracker

bcb1aaf

Rename and refactor code for managing global state.

ddaf2b8

Hydrocharged approved these changes Jul 29, 2026

View reviewed changes

nicktobey assigned nicktobey and zachmu and unassigned nicktobey Jul 29, 2026

zachmu approved these changes Jul 29, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

nicktobey commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coffeegoddd commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coffeegoddd commented Jul 22, 2026

Uh oh!

coffeegoddd commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coffeegoddd commented Jul 22, 2026

Uh oh!

coffeegoddd commented Jul 29, 2026

Uh oh!

Hydrocharged 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nicktobey commented Jul 22, 2026 •

edited

Loading

coffeegoddd commented Jul 22, 2026 •

edited

Loading

coffeegoddd commented Jul 22, 2026 •

edited

Loading