Repository navigation
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
1 of 7 tasks
2 of 7 tasks
Contributor
Author
|
Consolidated into #29953 as requested. The combined PR now contains the TopN ANY NULL execution regression coverage. |
XuPeng-SH
pushed a commit
that referenced
this pull request
Oct 12, 2026
## What type of PR is this? - [ ] API-change - [ ] BUG - [ ] Improvement - [x] Documentation - [ ] Feature - [x] Test and CI - [ ] Code Refactoring ## Which issue(s) this PR fixes: Fixes #29838 Related to #29968 (TopN ANY NULL regression coverage) ## What this PR does / why we need it: Spark 3.5.3's JDBC writer defaults to `READ_UNCOMMITTED`, which MatrixOne rejects before inserting rows. This documents the supported `isolationLevel=READ_COMMITTED` option for Spark JDBC writes, links the guide from both root READMEs, and adds a runnable compatibility probe. MatrixOne's isolation behavior is unchanged. The probe checks the default writer's MatrixOne 20105 error and empty target, then writes from two nonempty Spark writer partitions with `READ_COMMITTED` and verifies readback through a fresh Spark JDBC read and a separate JDBC connection. The TopN test is a separate upstream CI test-contract addition. It keeps unsupported tuple/interval types as negative cases, retains the `ANY` Prepare-success cases already on main in #29862, and exercises nonliteral `ANY NULL` through actual partition TopN execution. `TestPartitionTopNNonLiteralAnyNullOrder` checks output NULLs, payload/group cardinality, flat and constant NULL batches, Reset reuse, and pool cleanup. This consolidates the TopN coverage from #29975; no production code changes. ## Validation - Complete `pkg/sql/colexec/partition` package passed through the repository CGo wrapper on combined head `ca733a4b001b364ab467521b182fdb199d2c40ad` (`2.197s`). - `gofmt` and `git diff --check` passed. - Spark-side build and probe evidence is from the unchanged Spark files and production source head `73e54458d1a4cd209f9d8eda7795242935880551`: build passed; Spark 3.5.3 / Connector/J 8.0.33 / JDK 17 verified READ_UNCOMMITTED rejection (20105), empty target, successful READ_COMMITTED two-partition write, fresh Spark JDBC read and separate SQL readback, and cleanup. The probe was not rerun after the TopN test-only addition. - Focused BVT `test/distributed/cases/set/set_transaction.sql` passed 30/30 on base `035a5b61ba84ffc7dd2193135a7b170af04a4bff`. An attempt on source head `73e54458d1a4cd209f9d8eda7795242935880551` failed during mo-tester's pre-test database cleanup after connection retries, before any case ran; no BVT result is claimed for combined head `ca733a4b001b364ab467521b182fdb199d2c40ad`. - At the post-push snapshot, CI preflight, PR size, coverage eligibility, and Mergify Summary passed. UT Test on Ubuntu/x86, SCA on Linux/arm64, shared build, and UT Coverage on Ubuntu/x86 were pending; other listed jobs were skipped. Full remote CI has not passed. --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
XuPeng-SH
added a commit
that referenced
this pull request
Oct 12, 2026
…29978) ## What type of PR is this? - [ ] API-change - [ ] BUG - [ ] Improvement - [ ] Documentation - [ ] Feature - [x] Test and CI - [ ] Code Refactoring ## Which issue(s) this PR fixes: Fixes #29960 Fixes #29968 Related CI failure: #29921. ## What this PR does / why we need it: The TERM/PID fixture could acknowledge a signal before its trap returned. The sender then issued the next TERM too early and stalled waiting for an acknowledgment. A timeout's extra TERM could release the stall and misleadingly print `TERMS 200`. Acknowledgments now come from ordinary main-loop execution after the trap returns, with sequence/readiness assertions. A sender failure publishes its completion marker so the main process can join it and report the protocol error promptly. Logservice test listener candidates previously overlapped automatic client-port allocation. Probes closed their sockets before service startup; outbound traffic could occupy a candidate in that gap. On Linux, the existing allocator now excludes the configured inclusive `ip_local_port_range`, choosing the larger remaining interval while preserving its probes, process-local uniqueness and bounded attempts. The existing gossip topology and restart/convergence assertions remain exercised. TopN's unsupported-type regression used `T_any`, whose domainless NULL values now have a comparator. It now rejects `T_tuple` and positively exercises flat/constant all-NULL ANY keys across reset generations with source-row and mpool cleanup assertions. This adopts Lundomn's original #29975 commit `fbc1abc9f7bfdda6d907904d03ef34cb4793eac8`, preserving authorship, and closes partial batch-construction cleanup in a follow-up. The CI corrections are consolidated here as requested. ## Validation Linux/amd64, Go 1.27.1, CGo enabled, `matrixone_test`, bounded build/test concurrency. Matching native source/platform/profile provenance was verified before using the repository CGo test wrapper. - Full `optools`, `pkg/logservice`, and `pkg/sql/colexec/partition` packages pass in normal and race modes. - The three TERM/TopN regression tests pass with set coverage instrumentation; this is focused mode validation, not a full CI coverage-threshold verdict. - TERM regression passes 100 focused race repetitions; the gossip consumer including restart passes five focused race repetitions. - Modern PID and legacy fallback script controls complete all 200 signals. Mutating the same script to publish ACK inside the trap is rejected immediately with exit 98 in both controls. - Configured port-range, exhausted/invalid range, probe-error, occupied TCP/UDP, and socket cleanup controls pass. - Incremental repository lint reports zero issues; vet, gofmt and diff whitespace checks pass for the complete affected closure. - `gpt-6.1-sol` / `xhigh` produced the design and reviewed the complete final change. No unresolved material findings. The fix adds no sleeps, retry loops, reduced topology, or skipped coverage. Port probing still has an interval before the actual bind, so independent explicit binders can race it. Non-Linux uses a conservative candidate ceiling suitable for default macOS ranges; customized non-Linux ephemeral ranges are not inspected. BVT is not applicable to these CI/test-fixture contracts. Remote CI is pending after publication. --------- Co-authored-by: Lundomn <134036682+Lundomn@users.noreply.github.com>
This branch was successfully deployed
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
2 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.
What type of PR is this?
Which issue(s) this PR fixes:
Fixes #29968
What this PR does / why we need it:
The unsupported-order regression used a nonliteral
T_anykey and expectedPrepareto reject it, but domainlessT_anyNULL is intentionally orderable. The negative test now usesT_tuple, which has no comparator, and retains theErrNotSupportedassertion. A focused positive test runs nonliteral all-NULLT_anyorder keys through TopN across flat and constant batches, verifies emitted NULLs and valid source rows, and repeats after reset. No production behavior changes.Validation
mainbefore the edit.pkg/sql/colexec/partitionpackage passes with the repository CGo test wrapper.gofmtandgit diff --checkpass.The change is test-only; public SQL/BVT, race, and repository-wide UT were not run locally.