Skip to content

test concurrent sort - #3040

Open
jycor wants to merge 1 commit into
mainfrom
james/insertionSort
Open

test concurrent sort#3040
jycor wants to merge 1 commit into
mainfrom
james/insertionSort

Conversation

@jycor

@jycor jycor commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor
Main PR
covering_index_scan_postgres 2065.00/s 2097.03/s +1.5%
groupby_scan_postgres 145.20/s 142.64/s -1.8%
index_join_postgres 669.60/s 671.58/s +0.2%
index_join_scan_postgres 851.23/s 855.83/s +0.5%
index_scan_postgres 32.47/s 32.38/s -0.3%
oltp_delete_insert_postgres 817.23/s 871.33/s +6.6%
oltp_insert 754.68/s 779.35/s +3.2%
oltp_point_select 3557.41/s 3589.78/s +0.9%
oltp_read_only 3503.31/s 3460.70/s -1.3%
oltp_read_write 2638.07/s 2680.41/s +1.6%
oltp_update_index 780.49/s 785.29/s +0.6%
oltp_update_non_index 824.94/s 822.48/s -0.3%
oltp_write_only 1865.38/s 1890.80/s +1.3%
select_random_points 2219.51/s 2207.44/s -0.6%
select_random_ranges 1618.29/s 1635.94/s +1.0%
table_scan_postgres 32.23/s 32.32/s +0.2%
types_delete_insert_postgres 835.83/s 858.73/s +2.7%
types_table_scan_postgres 14.43/s 14.51/s +0.5%

@itoqa

itoqa Bot commented Aug 5, 2026

Copy link
Copy Markdown

Ito QA test results
Commit: c172576: 14 test cases ran, 8 passed ✅, 6 additional findings ⚠️.

Summary

Coverage spans core database behavior including joins and sorting, concurrent sessions, transaction and connection recovery after errors, and protection against cross-session data leaks. It also exercises edge cases and compatibility such as prepared queries, explicit NULL ordering, large or disk-backed sorts, table clustering, and source-build readiness; ordinary query and recovery behavior is broadly healthy, while several existing compatibility and build gaps remain.

Safe to merge — none of the observed failures are attributable to this PR, with no regressions, new failures, or previously flagged failures remaining unresolved because of the change. The unrelated SQL compatibility and source-container build issues are medium-severity follow-up findings rather than merge blockers.

Tests run by Ito

View full run

Result Severity Type Description
Concurrent Two concurrent sessions each received 20,000 rows from its own dataset, with the expected ID ranges and NULL values. Both sessions also completed follow-up queries successfully.
Concurrent A valid sort returned all 20,000 rows while another connection reported its own missing-table error. Both connections remained usable afterward.
Concurrent After a sorted query failed inside a transaction, rolling back removed the temporary write. The same connection returned all 20,000 rows on retry, stayed usable, and did not expose the failed write to another connection.
Engine The sorted employee and team query returned the expected rows, with the employee missing a team listed first. A follow-up query on the same connection returned 42.
Engine A sorting query returned its expected division-by-zero error, and the same connection then completed a simple query with result 7.
Engine After the sort error, rolling back removed the failed write. A second connection saw no marker row, and the first connection still returned the employee list.
Engine A failing sorted statement stopped the later insert, and the next sorted query still returned the original employees in order.
Module The exact upgraded Dolt and go-mysql-server modules downloaded from an isolated cache, and checksum verification passed. The source build could not run because the test container did not include Docker, so the built server and ordered SQL query were not checked.
⚠️ Medium severity Concurrent The prepared query returned an error saying PREPARE is not supported, and the requested explicit NULL placement syntax was rejected before either query could exercise the mixed workload.
⚠️ Medium severity Module The source Docker image never gets created, so it cannot be started or used for the SQL check.
⚠️ Medium severity Module Building the source container stops with an error before an image is created, so the server cannot start and the SQL check cannot run.
⚠️ Medium severity Spill The descending sort query failed with an error saying the NULLS FIRST syntax is unimplemented. The expected result is a complete result set ordered from highest to lowest, with NULL values at the beginning.
⚠️ Medium severity Spill The expected disk-backed query did not return its sorted rows. The query uses descending order with NULL values first, but the server rejects that valid SQL combination before sorting begins.
⚠️ Medium severity Spill The index creation statements succeed, but each CLUSTER statement is rejected. The expected ordered head and tail scans therefore do not verify the clustered table.
Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

🟡 Prepared queries are not supported
  • Severity: Medium Medium severity
  • Description: The prepared query returned an error saying PREPARE is not supported, and the requested explicit NULL placement syntax was rejected before either query could exercise the mixed workload.
  • Impact: Clients that use SQL PREPARE cannot run those queries, and some valid explicit NULL ordering queries are rejected. Ordinary sorted queries still work, and no data loss or corruption was observed.
  • Steps to Reproduce:
    1. Connect two clients to the local PostgreSQL-compatible server.
    2. On one client, run a parameterized SQL PREPARE statement for an ORDER BY query and execute it more than once.
    3. On the other client, run a large ORDER BY query using explicit NULLS FIRST or NULLS LAST placement.
    4. Check whether both clients return sorted rows and remain usable for another query.
  • Stub / mock content: The local test server used a test-only authentication bypass so the recorded SQL checks could run without a password. No application data or production services were used.
  • Code Analysis: server/ast/prepare.go:23-30 defines nodePrepare, which returns NotYetSupportedError("PREPARE is not yet supported") for every non-nil tree.Prepare node. This is the SQL PREPARE entry point used by the failing psql command, so the prepared statement cannot be created or executed. The repository also has a distinct PostgreSQL wire prepared-message implementation in server/connection_handler.go:540-608 and :640-691: handleParse stores a parsed prepared statement and handleBind creates a bound portal. That separate path does not implement the SQL PREPARE statement requested by this test and therefore does not make the observed SQL failure pass. In server/ast/order_by.go:48-60, tree.NullsFirst is rejected unless the direction is ascending, and tree.NullsLast is rejected unless the direction is descending. Consequently, explicit NULLS LAST with ascending order and explicit NULLS FIRST with descending order fail during ORDER BY conversion. The PR diff in pr-context.json changes only the github.com/dolthub/dolt/go and github.com/dolthub/go-mysql-server versions in go.mod and their checksums in go.sum; it does not touch any of these server files. The smallest practical fix is to implement SQL PREPARE by wiring tree.Prepare into the existing prepared statement machinery, and separately map every supported NULLS FIRST/LAST combination to the execution engine's ordering representation instead of rejecting the unsupported direction combinations.
Evidence Package
🟡 Source Docker image cannot be built
  • Severity: Medium Medium severity
  • Description: The source Docker image never gets created, so it cannot be started or used for the SQL check.
  • Impact: The source Docker image cannot be built, so the application cannot be started from this image. Releases or deployments that rely on this build are blocked until the builder uses a compatible Go version.
  • Steps to Reproduce:
    1. Run docker build --target runtime --build-arg DOLTGRES_VERSION=source -t doltgres-module-batch4:source .
    2. Wait for the source build stage to run go mod download
    3. Observe that the build reports that Go 1.25.12 is running while the project requires Go 1.26.2, then exits without producing an image
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The production Dockerfile declares FROM golang:1.25-trixie AS build-from-source at line 12. Its source-build command at lines 20-24 runs go mod download and then builds the application. The checked-in go.mod declares go 1.26.2 at line 3. In the recorded build, the Go tool reports go.mod requires go >= 1.26.2 (running go 1.25.12; GOTOOLCHAIN=local), so the command sequence stops before the binary is created and before the later mv command can succeed. The PR context shows only dependency-version edits in go.mod and corresponding checksums in go.sum; it does not modify Dockerfile or the Go directive. The smallest practical fix is to change the Dockerfile builder image at line 12 to a Go 1.26.2-or-newer image compatible with the project, then rebuild the source image.
Evidence Package
🟡 Source image cannot be built
  • Severity: Medium Medium severity
  • Description: Building the source container stops with an error before an image is created, so the server cannot start and the SQL check cannot run.
  • Impact: Users of the source Docker build cannot create a runnable server because the image fails to build. The issue blocks deployment through this build path but does not damage existing data.
  • Steps to Reproduce:
    1. Run the repository Docker build with the source-build target and the source version argument.
    2. Let the build reach the Go compilation stage.
    3. Observe that the build exits before producing the database server image.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The failure is directly supported by the checked-in source configuration. Dockerfile line 12 selects golang:1.25-trixie, which provides Go 1.25.12 in the recorded build. go.mod line 3 requires Go 1.26.2, and the build output reports that Go 1.25.12 rejects this requirement with GOTOOLCHAIN=local. The failure was reproduced with a second source-image build using the same Dockerfile and dependency graph. The smallest practical fix is to change Dockerfile line 12 to a Go 1.26 builder image that is available for the chosen Debian base, or otherwise align the builder with the existing go.mod requirement. The PR diff changes only dependency versions in go.mod and go.sum; it does not change Dockerfile line 12 or go.mod line 3, so the evidence does not establish that this PR introduced the toolchain mismatch.
Evidence Package
🟡 Descending sorts reject NULL values first
  • Severity: Medium Medium severity
  • Description: The descending sort query failed with an error saying the NULLS FIRST syntax is unimplemented. The expected result is a complete result set ordered from highest to lowest, with NULL values at the beginning.
  • Impact: Users who run a valid descending sort that puts NULL values first receive an error instead of the expected sorted rows. Other sort queries can still work, but this query form cannot complete until the missing sort behavior is fixed.
  • Steps to Reproduce:
    1. Start the local SQL server and load the supplied UUID sort fixture.
    2. Run a query such as SELECT abort_decreasing FROM abbrev_abort_uuids ORDER BY abort_decreasing DESC NULLS FIRST.
    3. Check the query result; it returns an unimplemented syntax error instead of rows with NULL values first.
  • Stub / mock content: The test used a disposable local SQL server and the supplied regression fixture; no mocks, route interception, or application bypasses were applied.
  • Code Analysis: The source path is deterministic and does not depend on browser behavior or the incomplete fixture population. In server/ast/order_by.go, nodeOrderBy first maps tree.Descending to vitess.DescScr at lines 38-44. It then handles explicit NULLS FIRST at lines 48-56, but returns an error whenever the mapped direction is not ascending. That condition directly rejects the requested DESC NULLS FIRST combination before query execution. The supplied tuplesort regression fixture exercises explicit NULL placement at testing/go/regression/tests/tuplesort.sql:40-49, and the broader fixture also uses DESC NULLS FIRST in an aggregate sort at lines 246-249. The PR context contains only go.mod and go.sum dependency updates; no changed line provides or removes this ORDER BY behavior. The smallest practical fix is to add a correct NULLS FIRST mapping for descending ORDER BY expressions in this conversion path, or pass the explicit null-order setting into the execution comparator instead of returning the current direction-specific error.
Evidence Package
🟡 Disk-backed sorting rejects valid NULL ordering
  • Severity: Medium Medium severity
  • Description: The expected disk-backed query did not return its sorted rows. The query uses descending order with NULL values first, but the server rejects that valid SQL combination before sorting begins.
  • Impact: Users cannot run valid SQL queries that sort descending while placing NULL values first. The query fails before it returns results, but there is no evidence of data loss or corruption.
  • Steps to Reproduce:
    1. Load the tuplesort regression fixture into a local Doltgres server.
    2. Start a transaction and set session work_mem to a low value such as 100kB.
    3. Run the fixture query that aggregates rows with ORDER BY id DESC NULLS FIRST.
    4. Observe that the server returns an unsupported NULL-ordering error instead of running the sort; then roll back the transaction.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run. The test used a disposable local Doltgres instance and the supplied regression fixture.
  • Code Analysis: The production path is server/ast/order_by.go, in nodeOrderBy. Lines 38-44 convert tree.Descending to vitess.DescScr. Lines 48-60 then validate explicit NULL placement, and the tree.NullsFirst branch returns an error whenever direction is not ascending (lines 53-56). The supplied disk-backed fixture reaches this path through testing/go/regression/tests/tuplesort.sql:243-260 and specifically requests array_agg(id ORDER BY id DESC NULLS FIRST) at line 247. Because nodeOrderBy returns before constructing the vitess order expression, neither the bounded execution path nor the disk-spill sort implementation can process this query. The smallest practical fix is to translate explicit NULLS FIRST/LAST into an equivalent supported sort expression or add the corresponding NULL-ordering support in this conversion path, while preserving the requested direction. The PR diff contains only dependency version and checksum changes, so it provides no direct changed-code cause for this behavior.
Evidence Package
🟡 Clustering a table fails after index creation
  • Severity: Medium Medium severity
  • Description: The index creation statements succeed, but each CLUSTER statement is rejected. The expected ordered head and tail scans therefore do not verify the clustered table.
  • Impact: Users who need to cluster a table cannot complete that database maintenance operation, even after creating the required index. The command fails clearly, while existing rows remain available.
  • Steps to Reproduce:
    1. Create the abbrev_abort_uuids table and its four indexes from testing/go/regression/tests/tuplesort.sql.
    2. Start a transaction and run CLUSTER abbrev_abort_uuids USING abbrev_abort_uuids__abort_increasing_idx.
    3. Repeat with the other three indexes used by the fixture.
    4. Observe that each CLUSTER statement is rejected with a syntax error instead of clustering the table.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The PostgreSQL parser already represents this syntax as tree.AlterTableCluster: postgres/parser/sem/tree/alter_table.go:300-314 defines the command and formats CLUSTER ON <index>, and the parser registers AlterTableCluster as an AlterTableCmd at lines 133-169. The application conversion path in server/ast/alter_table.go:94-185 dispatches every ALTER TABLE command through nodeAlterTableCmds. Its type switch handles add/drop/rename/default/nullability/type/owner and several ignored command types, but has no case for *tree.AlterTableCluster. The default branch at lines 180-181 returns ALTER TABLE with unsupported command type %T, which explains why the parsed CLUSTER command cannot be executed. The fixture requires this path four times at testing/go/regression/tests/tuplesort.sql:88, 104, 120, and 136, after index creation at lines 57-58 and 69-70. The PR context contains only go.mod and go.sum dependency-version changes, so there is no changed application line that introduced this missing dispatch case. The smallest practical fix is to add a focused AlterTableCluster conversion/execution case, or an explicit supported path for the existing index-clustering operation, without changing the unrelated sort tests.
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor
Main PR
Total 42090 42090
Successful 18918 18917
Failures 23172 23173
Partial Successes1 5325 5325
Main PR
Successful 44.9465% 44.9442%
Failures 55.0535% 55.0558%

${\color{red}Regressions (1)}$

random

QUERY:          (SELECT unique1 AS random
  FROM onek ORDER BY random() LIMIT 1)
INTERSECT
(SELECT unique1 AS random
  FROM onek ORDER BY random() LIMIT 1)
INTERSECT
(SELECT unique1 AS random
  FROM onek ORDER BY random() LIMIT 1);
RECEIVED ERROR: expected row count 0 but received 1

Footnotes

  1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant