Merged PRs

dolt

• 9193: go: store/datas/pull: puller.go: Fix incorrect location for Close call on PullChunkTracker.
  When a pull or push failed with an error in a HasMany call or on the initiating read of a set of chunks from the ChunkFetcher, the initiating goroutine used to close the PullChunkTracker. However, the writes to the PullChunkTracker are issued on the thread which reads and processes the chunks themselves. This meant that it was possible for the PullChunkTracker to be Closed while that thread was still issuing writes against it.
  Making the chunk-reading thread responsible for the Close is more correct.
  This fixes #9190.
• 9192: go: statspro: Make it possible for callers of StatsController.Stop() to block on the worker thread shutdown.
  Add pre-finalize blocking on the worker thread shutdown to dolt_gc safepoint controllers.
  Add blocking on the worker thread shutdown to dolt_stats_stop(), so that a caller knows that stats have actually stopped by the time it returns.
• 9186: Fix --branch global option always flagging error "no valid repository"
  Fix issue #6979
  We define noValidRepository to ensure the command has a valid target, but in that definition we call mrEnv.GetEnv() on the wrong string, so it's always null with the --branch flag. We should call it just on the name of the database, excluding the branch.
  A string with the database and branch is also needed later, so we need to save both variables.
• 9181: go: sqle/dprocedures: fetch,pull,push: Rebase the remote database before we interact with it.
  We need to rebase a remote in order to see its latest changes.
  Fixes #9164.
• 9179: refactor: replace map iteration with maps.Copy
  Optimize code using a more modern writing style.

go-mysql-server

• 2967: Unwrap wrapped values before computing their hash.
  (This PR changes the types in one of the tables used in tests. This improves the test coverage for TEXT columns, especially when they're used in conjunction with other tables with VARCHAR columns. None of the existing tests were testing the original type: this should be strictly increasing our test coverage.)
  The following plans involve computing a hash of rows to store in an in-memory hash set:
  • Intersect, Except, and Distinct
  • HashLookup
  • InSubquery
  • HashSubquery
  • FullJoinIter
  • ConcatJoin
  • Recursive CTE
  • UpdateJoinIter
    We weren't previously unwrapping wrapped values before computing hashes. The default hash implementation used the struct's %v representation to compute the hash, which has two problems:
  • The hash of the %v representation of a wrapper struct is not the same as the hash of the value that the wrapper is semantically equivalent to.
  • The hash of the %v representation of a wrapper struct depends on internal state, such as whether the wrapped has already been unwrapped once before (and cached the unwrapped value in an internal buffer)
    The simplest fix is to unwrap values before computing a row hash in the HashOf function.
    However, this fix comes at a cost: it now requires the engine to unwrap all values if they get used in any of the above plans. This will hurt performance for any of the above plans if they don't actually need to unwrap the value. For example, an UpdateJoinIter on a table with a TEXT column will now load that column from disk, even if its value is never used.
    A better fix might be to use the Hash() function that is already defined on the sql.Wrapper interface. For all existing Wrapper implementations, this returns the Dolt content address of the value, and is the same regardless of whether or not that address has previously been resolved. However, this would still return a different hash than an equivalent string. If we wanted them to return the same hash, Dolt would need to define a custom hash for strings that computes the Dolt content address of the string if it were to be stored as a Dolt chunk. This would likely be slower than Go's builtin hash for strings, although the performance might be comparable? This would likely result in worse performance for plans that don't use TEXT columns.
• 2966: Fix float to bigint conversion result
  Fixes: #9178
  Test to be added to dolt engine tests
• 2958: Correctly map enum values when altering an enum column.
  When altering the possible values of an enum column, the mapping between enum strings and their indexes may change. In order to alter the column correctly, we need to map the old indexes onto the new ones.
  This PR fixes two bugs in that mapping process:
  • Previously, we were converting the row to the new schema before setting the enums to their new index. This means that if the old row had an index that didn't exist in the new schema, this conversion would fail. By setting the enums to their new index before we convert the row to the new schema, we avoid this problem.
  • Enum indexes are 1-indexed, with the 0 value representing an "invalid" value. One way to get an invalid enum in MySQL is an INSERT IGNORE statement where the provided value for the enum is out of range. Dolt also allows setting the column to 0 directly, although MySQL does not. Previously, we were not able to alter enum columns if they contained a 0 value, but now we can.

Closed Issues

• 9180: Unintuitive error messages when using global options --branch and --use-branch on invalid branches
• 6979: --branch global option doesn't work by itself
• 9190: Panic when trying to push wikipedia db
• 9166: More panic: interface conversion: interface {} is *val.TextStorage, not string
• 9178: Problem modifying a column to change from float to bigint
• 9164: push/pull synchronization fails with file remote in multi-node setup

Performance