test(join): add broad FULL OUTER JOIN coverage across types/constraints/table kinds - #2
Open
Ariznawlll wants to merge 17 commits into
Open
test(join): add broad FULL OUTER JOIN coverage across types/constraints/table kinds#2Ariznawlll wants to merge 17 commits into
Ariznawlll wants to merge 17 commits into
Conversation
…ts/table kinds Complements the core phase1-4 coverage in fullouterjoin.sql (matrixorigin#24230) by exercising FULL OUTER JOIN across the surface area a typical user hits, not just the execution paths. Four new suites: - fulljoin.sql - core semantics, NULL handling, WHERE, aggregation, CTE, self-join, mix with INNER/LEFT, large-scale invariants - fulljoin_types.sql - per-data-type join-key coverage (all int widths + unsigned, decimal, double, bool, bit, char/varchar/text, binary/varbinary/blob, date/datetime/ timestamp/time, uuid, enum, json, vector payload, null-safe <=>) - fulljoin_constraints.sql - PK/UK/NOT NULL/DEFAULT/AUTO_INC/CHECK/ ON UPDATE/FK-cascade/generated/secondary index, plus a 3-table FK-network combo - fulljoin_tables.sql - temporary / partitioned (HASH/RANGE/ KEY/LIST) / view / CTE / recursive CTE / mo_catalog cluster table / fulltext- indexed table / IVFFLAT / HNSW, plus a non-sys tenant section (incl. negative test that a non-sys tenant cannot create a cluster table) Cross-validated against MySQL 8.0 via LEFT UNION RIGHT rewrites for every case where MySQL can build an equivalent schema; the rest use MO-internal self-check as the oracle. Known-buggy queries are wrapped in -- @bvt:issue#24247 (FULL OUTER JOIN USING does not coalesce the merged column). Results verified with mo-tester -m run -g: 506 SQL, 504 success, 2 ignored (USING under matrixorigin#24247), 0 failed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This backports matrixorigin#24201 to `main`. For pessimistic `INSERT ... SELECT ...` statements that escalate to table locks, `Compile.compileLock` was still keeping the target table on the pre-pipeline lock path. That can make a large source scan hold the target-table lock before the write side starts running and block later writers for too long. This backport keeps the same table-lock semantics, but lets `LockOp` acquire the table lock when the first batch reaches the target pipeline. It also preserves EOF-time table locking for the zero-row path so inserts that produce no rows still keep the intended table-lock behavior. Also included: - a compile unit test for the pre-pipeline lock routing - a lockop unit test for the zero-row EOF table-lock path Approved by: @aunjgr, @ouyuanning, @iamlinjunhong
- avoid reading and scanning the hidden commit-ts column on every normal block read when block-level commit-ts zonemap proves the whole block is visible to the snapshot - keep row-level commit-ts filtering for snapshot/data-branch fallback reads when the commit-ts range overlaps the requested snapshot - avoid treating a user-visible trailing `TS` column as hidden commit-ts metadata - reuse already loaded object metadata while loading block columns so the read path does not fetch the same metadata twice - add blockio coverage for row-level filtering, zonemap fast path, and visible TS-column behavior Validation: - `gofmt` applied - `git diff --check` passed - `go test ./pkg/vm/engine/tae/blockio ./pkg/objectio/ioutil` could not complete in this local environment because CGO headers `usearch.h` and `xxhash.h` are missing before package tests run Approved by: @XuPeng-SH
…trixorigin#24202) Upstream Sirius added a transparent GPU execution optimizer hook (sirius-db/sirius#518). With 'gpu_execution=true' (default), the optimizer captures the outer logical plan and tries to compile it on the GPU. For our wrapper 'SELECT * FROM gpu_execution(...)', the captured outer plan has 'gpu_execution' as a TableScan source, and the Sirius pipeline converter throws 'Unsupported scan function: gpu_execution', surfaced to MO as a 500 from the sidecar. The 'CALL gpu_execution(...)' form is the documented invocation in upstream Sirius (docs/super-sirius/README.md) and is not intercepted by the transparent hook, so it routes correctly through the dedicated gpu_execution table function bind/execute path that compiles the inner SQL into a Sirius plan. Verified end-to-end: all 22 TPC-H SF10 queries pass via the sidecar GPU path with the new wrapper (paired with the matching sirius bump in mo-sirius-sidecar). Approved by: @XuPeng-SH
…MIT (Fixes matrixorigin#24243) (matrixorigin#24244) `INSERT INTO ... SELECT ... ORDER BY col LIMIT 5000000` on a 100M row table causes OOM in CI nightly regression. The Top operator holds all LIMIT rows with ALL columns in the heap, consuming O(limit × row_width) memory — for 5M rows of wide data this reaches tens of GiB. This PR makes three targeted changes: ### 1. Top operator: spill to disk for large LIMIT (`top/top.go`, `top/types.go`) When LIMIT > 16384, the Top operator now: - Keeps only **sort-key columns** in the in-memory heap (not all columns) - Spills full rows to a temporary file via `batch.MarshalBinary` - Tracks `rowRef{batchIdx, rowIdx}` per heap entry to locate spilled rows during eval Heap memory drops from **O(limit × row_width)** to **O(limit × key_width)**. ### 2. Top operator: streaming eval in spill mode (`top/top.go`) Instead of materializing all LIMIT rows into one giant batch during eval, spill mode now: - Pops heap into sorted `orderedRefs` - Frees the heap batch immediately - Reads spilled batches and outputs **8192-row chunks** per `Call()` invocation Eval peak memory drops from **O(limit × row_width)** to **O(chunk_size × row_width)** (~10 MiB per chunk). ### 3. MergeTop: fix memory leak from `defer` in loop (`mergetop/top.go`) `defer bat.Clean(proc.Mp())` was placed inside a `for` loop in `build()`. Since `defer` only fires on function return, every duplicated batch from each iteration accumulated in memory. Replaced with explicit `bat.Clean()` after each `processBatch` call and on error paths. Approved by: @ouyuanning, @aunjgr
…ixorigin#24258) Fixes two related FULL OUTER JOIN bugs. ### matrixorigin#24247 — `FULL OUTER JOIN ... USING(col)` did not coalesce the merged column Per the SQL standard, the merged USING column on a FULL OUTER JOIN is `COALESCE(left.col, right.col)`. MO previously resolved it to the left binding alone, which loses the right-side value on right-only rows (the column comes back as `NULL` even though a value exists on the right side). Repro before this PR: ```sql create table u1(id int); create table u2(id int); insert into u1 values (1),(2); insert into u2 values (2),(3); select id from u1 full outer join u2 using(id) order by id; -- Wrong: 1, 2, NULL (expected: 1, 2, 3) ``` Implementation: - New `outerUsingCols map[string][]string` on `BindContext` — for each USING column, the ordered list of contributing leaf tables. The list has length ≥ 2 only when the column came from a FULL OUTER JOIN USING arm (possibly nested through other FOJ-USING). - `addUsingCol` reads the lists from the **child** contexts (not from `bc`), so the merge-context overwrite during context propagation is harmless. Branch logic for LEFT/RIGHT/INNER vs OUTER is collapsed via `chosen`/`chosenCoalesce` aliases and a small switch on `chosen`-list semantics. - COALESCE emission at three sites where a bare/unqualified merged column is exposed: - SELECT-list star expansion (`doUnfoldStar`) - Unqualified column references (`qualifyColumnNames` and `baseBindColRef` fast-path) - The join equality predicate itself is also coalesce-aware (`buildUsingEqOperand` emits `COALESCE(arm1.col, arm2.col, ...)` when there are ≥ 2 arms), so an enclosing FOJ-USING matches against the inner merged column rather than just the inner-chosen side. Without this, nested FOJ-USING produced a duplicate id row. - Qualified references (`u1.id`, `u2.id`) keep current per-side behavior (NULL on the padded side). ### matrixorigin#24250 — `NATURAL FULL [OUTER] JOIN` parsed but silently became NATURAL RIGHT The grammar action for `NATURAL outer_join` was an `if/else` chain that fell through to RIGHT for FULL. Fixes: - New `JOIN_TYPE_NATURAL_FULL` constant in `tree/select.go`. - `mysql_sql.y`: rewrite the `NATURAL outer_join` reduction as a `switch` over LEFT/RIGHT/FULL. Regenerated `mysql_sql.go` via `cd pkg/sql/parsers && make mysql`. - `query_builder.go`: plumb `NATURAL_FULL` into the `Node_OUTER` joinType mapping and the natural-cols discovery branch. - Add `JOIN_TYPE_FULL` and `JOIN_TYPE_NATURAL_FULL` to the recursive-CTE rejection list — an oversight from matrixorigin#24192 when FULL was first added. ### Tests `test/distributed/cases/join/fullouterjoin.sql` gets two new sections: - §8 (matrixorigin#24247): `SELECT id`, `SELECT *`, `WHERE id = ...`, `GROUP BY id`, nested `(u1 FOJ u2 USING(id)) FOJ u3 USING(id)`, and qualified-reference behavior. - §9 (matrixorigin#24250): `NATURAL FULL OUTER JOIN`, `NATURAL FULL JOIN`, and `SELECT *` over a NATURAL FULL JOIN. mo-tester run on the file: 69/69 SUCCESS, 0 FAILED. Approved by: @iamlinjunhong, @ouyuanning, @heni02
…onst keys (matrixorigin#24259) `partition.Partition` is invoked once per sort key from `pkg/sql/colexec/order/order.go::sortAndSend` (and per partition spec from `pkg/sql/colexec/window/window.go`), reusing the same `diffs` slice across calls. The contract is that each call must **OR** new boundaries onto `diffs` — never overwrite an existing `true`. Two code paths in `pkg/partition/partition.go` violated that contract: 1. **Both-NULL overwrite.** In `genericPartition` / `bytesPartition`, when both adjacent rows were NULL under the current key the code did `diffs[i] = false`, erasing a boundary set by a prior key. This made multi-key `ORDER BY` over a FULL OUTER JOIN produce rows out of order: with the primary key NULL on the right-padded side, the secondary key was never sub-sorted within the `t1.s = NULL` partition. Repro from the issue: ```sql create table t1(s int, v varchar(5)); create table t2(s int, v varchar(5)); insert into t1 values (1,'a'),(5,'b'),(NULL,'x'); insert into t2 values (13,'c'),(14,NULL); select t1.s, t1.v, t2.s, t2.v from t1 full outer join t2 on t1.s = t2.s order by t1.s, t2.s, t1.v, t2.v; ``` Before this PR, `(NULL,NULL,14,NULL)` was emitted **before** `(NULL,NULL,13,'c')`. After: 13 before 14, as expected. 2. **Const-vector boundary collapse.** When `vec.IsConst()` the code returned `partitions = [0]` and, for `IsConstNull()`, even cleared all of `diffs`. A const key appearing mid-list therefore collapsed all previously found partitions in the same `Partition` call series. ### Fix In both `genericPartition` and `bytesPartition`, treat every "rows are equal under this key" outcome as a no-op on `diffs` (the both-NULL case and the entire const branch). Always rebuild `partitions` from the OR-accumulated `diffs` at the end. `diffs[0] = true` is set up front, so the const-vector path now naturally yields `[0]` via the final scan rather than via a special branch. This is the same semantics the non-const non-null path already had (`diffs[i] = diffs[i] || (v != w)`). ### Tests - `pkg/partition/partition_test.go::TestPartitionAccumulatesDiffs` exercises the reuse contract — multiple successive `Partition` calls on the same `diffs`/`partitions` slices — across all four problematic shapes: - fixed-width second key with all NULLs preserves the first key's boundaries - fixed-width const non-null second key preserves boundaries - fixed-width const NULL second key preserves boundaries - bytes second key with all NULLs preserves boundaries - bytes const second key preserves boundaries - sanity check that two non-trivial keys union their boundaries - `test/distributed/cases/join/fullouterjoin.sql` gets the issue's exact repro and a `NULLS LAST` (DESC) secondary-key variant. - `mo-tester -m run` on the test file: **45/45 SUCCESS, 0 FAILED**. - Unit tests for `pkg/sql/colexec/order/...`, `pkg/sql/colexec/window/...`, and `pkg/partition/...` all pass. Approved by: @iamlinjunhong, @heni02
…nto feat/add-full-outer-join-tests
…nto feat/add-full-outer-join-tests
…ation (matrixorigin#24246) - Add `UnpackNthElement` to decode only up to the target element in a serialized tuple, instead of deserializing the entire blob - Use fast path in `serial_extract` when the index parameter is constant (the common case in GROUP BY on composite secondary index) - Reduces `serial_extract` CPU overhead ~6x for DISTINCT queries on leading columns of composite secondary indexes Approved by: @ouyuanning, @aunjgr, @XuPeng-SH
mergify Bot
pushed a commit
that referenced
this pull request
May 7, 2026
…origin#24117) Move postEvict callbacks (value.Release + metrics updates) outside the global `queueLock` in the FIFO cache to eliminate lock convoy under memory pressure. ### Root Cause When MemCache (12GB FIFO cache) is 100% full, every `Set()` triggers `Evict()` which holds the global `queueLock` while executing `postEvict` callbacks. Under GC pressure (STW pauses up to ~1s), this creates a lock convoy where all concurrent cache operations serialize through the single lock: - Each `Set()` takes **300-444ms** (normal: <0.1ms, 3000-4400x slower) - Queries with ~90 cache ops inflate from 1-5s to **60-107s** - Exceeds client-side timeouts → **connection disconnections** Evidence from fileservice slow event trace: ``` Single S3FS.Read (total: 798ms) breakdown: disk cache read: 35ms set memory cache entry #1: 318ms ← queueLock wait + eviction set memory cache entry #2: 444ms ← queueLock wait + eviction 762ms (95%) spent in "set memory cache entry" ``` ### Fix 1. Collect evicted items under `queueLock` into a pending list 2. Release `queueLock` 3. Execute `postEvict` callbacks (value.Release + metrics) outside the lock The `item.valueOK` flag is still set to `false` under the **shard lock** (not queueLock) to prevent data races with concurrent `Get()` calls. ### Impact In stability testing (commit 725b723): - **TPCC**: 40 "Communications link failure" events - **Fulltext**: 57 "Lost connection" events - **Sysbench**: 0 errors (point queries <5s skip tombstone transfer) Approved by: @gouhongshen, @XuPeng-SH, @fengttt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds broad FULL OUTER JOIN test coverage on top of the core phase1–4 suite
introduced in fullouterjoin.sql (matrixorigin#24230). The four new files
exercise the user-visible surface rather than the execution paths:
NULLs on both sides, commutativity, empty corners, no-match / all-match,
duplicates on both sides, multi-column ON, USING, non-equi ON, WHERE
anti-left/anti-right/symmetric-diff/predicate-on-nullable-column,
COALESCE merged key, aggregation, derived tables, 3-table chain,
self FULL JOIN, mixed INNER/LEFT, per-type coverage (decimal/date/
char-vs-varchar), NULL-safe
<=>, EXPLAIN, large-dataset invariants.widths (signed + unsigned), decimal, double, bool, bit, char, varchar,
text, char-vs-varchar, binary, varbinary, blob, date, datetime,
timestamp, time, uuid, enum, json, vector columns as payload, and
=vs<=>NULL-safe semantics.PRIMARY KEY (single/composite), UNIQUE KEY (single/composite),
NOT NULL + DEFAULT, AUTO_INCREMENT, CHECK, ON UPDATE CURRENT_TIMESTAMP,
FOREIGN KEY with ON DELETE/ON UPDATE CASCADE, generated columns,
secondary indexes, and a 3-table FK-network combo (user → order →
item).
one/both sides, partitioned tables (HASH / RANGE / KEY / LIST),
partition × partition, views × views, CTE and recursive CTE,
derived tables,
mo_catalogcluster table joined with a normal table(sys session), FULLTEXT-indexed tables, IVFFLAT and HNSW vector
indexes, plus a non-sys tenant section covering basic FULL,
multi-column ON, anti-left/right, HASH partition, IVFFLAT, fulltext,
CTE, and a negative test that confirms a non-sys tenant cannot create
a cluster table.
Validation
Cross-validated every case against MySQL 8.0.45 via
LEFT JOIN UNION ALL RIGHT JOIN WHERE left.key IS NULLrewrites whereverMySQL can build an equivalent schema. For MO-only constructs (vector
indexes, cluster tables, temporary tables, partitioned tables, fulltext)
used MO-internal FULL vs LEFT-UNION-ALL self-check as the oracle.
The cross-check surfaced three FULL OUTER JOIN correctness defects, each
filed as a separate issue:
FULL OUTER JOIN ... USING (col)does not coalesce themerged column (right-only rows show
NULLinstead of the right value).The two USING queries in this PR are wrapped in
-- @bvt:issue#24247 ... -- @bvt:issueso they are ignored until thebug is fixed.
ORDER BYon top ofFULL OUTER JOINemits rows out ofkey order when the join produces mixed matched + NULL-padded rows.
Queries in this PR still use
ORDER BY; when [Bug]: ORDER BY on top of FULL OUTER JOIN produces rows out of order matrixorigin/matrixone#24248 is fixed, thatPR will need to regenerate the affected
.resultfiles. Output isdeterministic today, so CI remains green.
NATURAL FULL [OUTER] JOINsilently degrades toNATURAL RIGHT JOIN(grammar bug —natural_joingrammar does notaccept
FULLas anouter_joinalternative, and the if/else fallsthrough to RIGHT). This PR does not use
NATURAL FULL, so nothingis wrapped.
Results generated with
mo-tester -m genrs -g, and replay verified withmo-tester -m run -g:Test plan
mo-tester -p test/distributed/cases/join -m run -i fulljoin -gpasses with 504 / 506 success, 2 ignored under
#24247..resultfiles regenerated with-m genrs -g; output confirmeddeterministic across repeated runs.
engine defects separately filed as [Bug]: FULL OUTER JOIN ... USING (col) fails to coalesce the merged column matrixorigin/matrixone#24247, [Bug]: ORDER BY on top of FULL OUTER JOIN produces rows out of order matrixorigin/matrixone#24248, [Bug]: NATURAL FULL [OUTER] JOIN silently degrades to NATURAL RIGHT JOIN matrixorigin/matrixone#24250.
🤖 Generated with Claude Code