Skip to content

fix(sql): preserve signed zero in float set operations - #30184

Open
iamlinjunhong wants to merge 40 commits into
matrixorigin:mainfrom
iamlinjunhong:fix/30155-setop-signed-zero
Open

iamlinjunhong wants to merge 40 commits into
matrixorigin:mainfrom
iamlinjunhong:fix/30155-setop-signed-zero

Conversation

@iamlinjunhong

@iamlinjunhong iamlinjunhong commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

What this PR does / why we need it:

MySQL 8.0.46 preserves FLOAT/DOUBLE negative and positive zero as separate set members: the reported UNION/INTERSECT/EXCEPT counts are 2/0/1, while MatrixOne returns 1/1/0. Use a set-only UINT64 equality key after common-type resolution and regenerate it during prepared specialization. Preserve original result representatives, existing NaN/scale normalization, PAD SPACE handling, global duplicate elimination and spill recovery. Scalar comparison, GROUP BY, SELECT DISTINCT, distinct aggregates and UNION ALL retain their existing equality domains.

Retaining independent UNION arms also preserves correlation cardinality, producer boundaries and separate prepared parameters previously hidden by textual identical-arm collapse. Proven aggregate/LIMIT boundaries reuse their original producer; supported EXISTS/IN paths retain existing predicate/NULL semantics. Selective predicates on unchanged integer/bool outputs can still reach both branches. The scalar fallback uses stream-occurrence identity plus existing WINDOW, LEFT/LoopJoin, aggregation and SINGLE owners to distinguish empty input, one NULL and multiple distinct members, including duplicate outer occurrences and mixed dependent expressions.

The fallback remains coordinator-local at DOP 1, with one shared statement-attempt account (256 MiB or the smaller inherited limit), a 16,777,216 candidate-pair budget and one of two CN permits. False predicates consume work; continuation yields do not refund it. Work/concurrency rejection returns terminal error 20833. A permit is released only after exact valid zero-live accounting; private initialization/cleanup failures keep uncertain owners out of reuse pools. Prepared attempts obtain fresh identity and leases.

Rebase onto official 7038c8520e5fc1ba456914b3c389ccedb69e22f7 preserves VIEWS metadata at functions603/604 and MORPC126, LOAD IGNORE diagnostics at602/125, and TIMESTAMPADD SECOND at601/124. This PR's single set-key/scalar-marker capability advances to function605, sentinel606 and MORPC127. Node96 and Instruction65 are unchanged. Placement, destination/send/receive, actual-stream admission, native construction/Prepare, retained CTAS and persisted read/authoring gates agree. Missing or126 workers fall back to the capable coordinator and are rejected as actual set-key destinations before sending. Marked stages remain coordinator-local.

Requirements compose independently: VIEWS-only streams require126, LOAD-only125; adding a set key or marker requires127. The new VIEWS/key/marker matrix covers unknown and124/125/126/127 capabilities, response release and actual-stream requirements; the existing stream-handshake test now includes127. Immediate-predecessor126 controls cover native/retained execution, CTAS Compile/Run and persisted admission. Earlier unpublished set-key601/602/603 fixtures remain historical, never aliases to released TIMESTAMPADD601, LOAD602 or VIEWS603/604.

The integration preserves stable view headings and expanded-star regeneration, shared-CTE publication and scalar column demand, prepared FIELD/CASE domains, ENUM/SET assignment domains and DUMP create-only publication. The RFC revision-26 receipt binds 456 source fingerprints, commands, terminal results, log hashes and evidence reuse:

  • All six complete owning suites pass: protobuf plan/pipeline, functions, planner, compiler and colexec. The first command's obsolete VIEWS605 Any() expectation is recorded as failed; the corrected complete protobuf plan suite passes, retaining VIEWS603/604, independent setkey605 and unknown606 assertions.
  • Four complete type/parser/external suites, all52 frontend DUMP roots plus ViewSchema/prepared/BIT controls, fileservice exclusive-write tests, six public SQL roots and the ENUM/SET MySQL-protocol root pass. Public roots retain signed-zero, external pruning, shared CTE UNION, TIMESTAMPADD DUMP, ALTER index and view check-option cases.
  • Fresh FLOAT/DOUBLE605 wire is tested against the complete actual official7038 Go-source overlay: official126 constructs VIEWS603/604, LOAD602, TIMESTAMPADD601 and ordinary controls but does not implement605. Its real query-service126 response makes the candidate choose the coordinator and refuse an incompatible actual destination, with balanced releases. This is native implementation and response replay evidence, not a live mixed-version cluster.
  • Format, vet and incremental lint pass for33 packages; the final protobuf test correction is also checked independently. Session-interrupted linker-only/empty logs are not counted as passes; only incomplete native and ENUM stages were resumed, with completed results retained.

Capacity and race qualification remains historical and source-bound:26 profiles,165 samples, cold first invocation separate, one excluded warm-up and three measured rounds. Runtime/account/lease owners retain their accepted limits; no new-head latency/RSS, external/APPLY capacity or distributed scalar qualification is claimed. Existing Darwin/arm64 native facilities are read-only test inputs. Native implementation/module inputs and protobuf marker fields are unchanged; no fresh native rebuild or Linux hardware/performance qualification is claimed.

The starting13d1 head has successful enabled CI checks and an independent implementation approval. This update resolves the new official allocation conflict; new-head CI is left to the push-triggered run. No old-run rerun, issue creation or duplicate feedback reply is part of this delivery. The issue should remain open after merge for reporter verification.

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@mergify mergify Bot added the kind/bug Something isn't working label Oct 14, 2026
@matrix-meow matrix-meow added the size/L Denotes a PR that changes [500,999] lines label Oct 14, 2026
@iamlinjunhong
iamlinjunhong force-pushed the fix/30155-setop-signed-zero branch from 73912ed to d48be22 Compare October 14, 2026 13:46
@XuPeng-SH

XuPeng-SH commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

mo-bot review · mo-bot-local · done

Commit: d48be22e128ef9f3de3e0ff924533ab9626d867e

Review completed. Read the assessment.

@XuPeng-SH XuPeng-SH left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mo-bot review · mo-bot-local · review

Commit: d48be22e128ef9f3de3e0ff924533ab9626d867e

Assessment: request_changes

Reviewed the complete fourteen-file delta and affected consumers. Reusing physical equality keys is a sound way to isolate signed-zero set semantics. However, integration with correlated-query handling and predicate pushdown introduces two confirmed regressions.

Model: gpt-6-astra / xhigh

Session: 01a12d8f-7dea-77c2-a565-38d45b5564e4 · slot 0 · generation 0

  • P2 pkg/sql/plan/query_builder.go:4868: On t(a INT,b INT,f DOUBLE), execute SELECT a FROM t o WHERE EXISTS (SELECT f FROM t i WHERE i.a=o.a AND i.b=o.b UNION SELECT f FROM t i WHERE i.a=o.a AND i.b=o.b). Identical arms using i.a>o.a also trigger the regression.
    Impact: Previously supported queries now fail with error 20102. Prepared versions also fail when both arms select the integer constant 1. Preserve these correlation shapes when retaining float or prepared arms.
    Evidence: All six public SQL probes pass with exact merge-base overlays. The candidate fails float/prepared multiple-predicate and range-predicate cases; single-equality controls pass. The new collapse restrictions expose flatten_subquery.go:4388–4404, which accepts only one matching equality identity per arm. Results are retained in public-base.log and public-head.log.

  • P2 pkg/sql/plan/query_builder.go:4861: On t(k INT PRIMARY KEY,d DOUBLE), execute SELECT k,d FROM (SELECT k,d FROM t UNION SELECT k,d FROM t) u WHERE k=1.
    Impact: Adding float physical keys activates the blanket filter-pushdown barrier, converting a selective primary-key lookup into full scans and hash aggregation. Preserve pushdown for predicates proven invariant under the equality key, including this predicate on the untouched integer column.
    Evidence: With 4,096 finite rows, EXPLAIN PHYPLAN ANALYZE changes from one scanned row and 37 bytes of reported memory on base to two 4,096-row scans, an 8,192-input-row aggregate, and 761,904 bytes on HEAD. The integer-only control remains one scanned row and 25 bytes. The filter stays above aggregation because pushdown.go:944 rejects pushdown whenever PhysicalEqualityKeyList is populated. See perf-base.log and perf-head.log.

Validation:

  • Artifacts: /home/xupeng/.local/state/mo-bot/jobs/cf9b4aade4ab97927e6c6434/attempt-1. evidence-audit.json reconciles seven execution receipts with commands, source fingerprints, logs and overlay inputs. Final audit receipt 1792012368798373316-4.json exits 0; source remains unchanged.

  • Isolation preflight passed. Native provenance verified 192 current source inputs and 62 artifacts against the retained repository build-owner receipt and log.

  • Focused function, planner, group/spill and compile/remote-codec tests passed: receipt 1792012253239799077-3.json.

  • TestSetOperationSignedZero passed. Independent correlation probes passed on base and exposed four candidate failures: receipts 1792012264966570110-3.json and 1792012269810977520-3.json.

  • Both physical-work comparisons completed successfully: receipts 1792012294035663064-3.json and 1792012294734351744-3.json.

  • Frozen exact-head CI reports successful UT, static analysis, shared build, proxy/pessimistic multi-CN BVT and coverage.

Limitations:

  • The full linked issue body, raw CI logs and retained author validation artifacts were not supplied; local_validation was empty.

  • Local public SQL probes used one CN. Codec and compilation checks do not constitute a local multi-CN runtime run.

  • Performance evidence measures execution-plan work and memory, not sustained latency or concurrency. Full repository, BVT and race suites were not rerun locally.

This branch is being deployed

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

Labels

kind/bug Something isn't working size/XXL Denotes a PR that changes 2000+ lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants