Skip to content

feat(sirius): lower MO exact decimals for embedded GPU execution - #29757

Merged
XuPeng-SH merged 28 commits into
matrixorigin:mainfrom
aunjgr:feature/28968-mo-exact-decimal
Oct 10, 2026
Merged

XuPeng-SH merged 28 commits into
matrixorigin:mainfrom
aunjgr:feature/28968-mo-exact-decimal

Conversation

@aunjgr

@aunjgr aunjgr commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Opt-in embedded MO-reader queries now lower MO exact Decimal64/128/256 values and descriptors to Sirius GPU execution. One immutable capability-scoped profile owns admission, physical types/literals, bound scalar/aggregate overloads, input publication, result reconstruction and public numeric error classes. Terminal evidence records verified descriptors and a schema digest without logging headings or values.

This implements approved PR C from #29690 and the versioned design/validation map. Physical width is preserved at narrow declared precision; MO-bound division metadata and 32-byte Decimal256 coefficients use the existing credit-before-copy and borrowed-result contracts. Embedded fetch defaults and DATE extraction use supported native wire forms.

All prerequisites are merged and inherited:

  • Sirius #29: 908ffc75a58b3be496b2172795e416326416e7ec, with merged importer [#4] Add a new document: CHANGELOG.md #5 at 99c7ca3b6f8f3159239e119ed2982d42f98c4690.
  • Kernel #29775: af3f7232a31879a2d6ed590785c8e9a560361c3f. Prepared SELECT metadata comes from the executed generation; incomplete saved SELECTs clear session state before reuse.
  • Main #29794 supplies the existing MORPC shutdown-test race correction. Main now also includes the histogram-export synchronization fix; C uses main's bounded two-send notification wait unchanged.

Final release/native integration at executable C head ad61b5b5d3, on merged-kernel main af3f7232a3, passes:

  • Default, Sirius-only and combined Sirius/cuVS release builds. The clean merged SDK, all 73 fingerprints, compiler/header/proto identity and runtime package/provenance guards pass.
  • All 22 native preparations without reader/GPU-task admission. The complete public MySQL numeric fixture passes exact values, metadata/NULLs, arithmetic/aggregates/joins/sort, operation-owned errors, masked errors, healthy reuse and prepared division increments 0/4/10/30/4. Native MO supplies the independent oracle.
  • All 31 public terminal events show capability 31, mo-exact-decimal-v1, completed GPU tasks, no fallback, healthy cleanup and zero retained input/result credit.
  • Four C-owning packages plus frontend pass default normal/race checks; native-tag owning checks and the complete combined bridge suite pass. cuVS-before/Sirius/cuVS-after passes in one process.
  • All nine public prepared-metadata/saved-result cases pass normal mode and three race repetitions, including a real persisted-batch failure followed by successful save/replay on the same physical connection.
  • Native data/cancellation passes 68 race repetitions in one process (measured 0.44s; 30s budget). The exact MORPC shutdown test passes 100 race repetitions. Metrics focused/owning normal/race and 100 focused race repetitions pass.

The delivered head is b34111b4c9e94cda034d199a2c362072fe98d808, normally merged with authoritative main e5dd4724f782067e1c76c38c9d2f55cebceda4a9. Review fixes are committed in af4f342066: typed DATE lowering preserves persisted MO zero dates; exact CASE retains the MO-bound descriptor around IfThen; ordinary embedded integer add/subtract/multiply declines before readers because the pinned consumer does not implement MO checked overflow. Flight encoding/admission, reader bytes, native ABI and the merged Sirius pin are unchanged.

The installed and loaded host drivers now both report NVIDIA 615.78.08; final GPU checks run directly with the installed driver. The bounded local native fixture uses 256 MiB GPU and 2 GiB host capacity. An initial 512 MiB host fixture correctly rejected larger progress-window reservations; only the test fixture was corrected. Production windows, limits and assertions remain unchanged.

Review-resolution validation at prior head a4e34ef05d:

  • Both incoming counterexamples have real GPU red/green proof. Zero DATE now returns 0/0/0 alongside year-0001/leap-day/year-9999/NULL controls, with SQL mode restored. Nullable division conditions with two required decimal arms execute with native-MO values and nullable DECIMAL(15,2) metadata.
  • The existing public fixture covers integer +/-/* overflow declines and healthy integral/decimal reuse. No new cluster, production hook, native library dependency, sleep or skip was added.
  • All 22 native preparations pass. Full public GPU normal mode passes (9.596s); measured race mode followed by three in-process repetitions passes (29.571s). Its 34 terminal events verify exact profile/capability31, completed GPU work, no fallback, healthy cleanup and zero retained input/result credits.
  • Fresh combined release/native provenance and full bridge/coexistence pass. Exporter/compiler/CN owning normal/race, focused new wire/domain regressions, config and full mandatory pre-push SCA pass with zero lint findings. Self-review has no unresolved implementation blocker.

Ready for review; merge gates remain required CI and reviewer approval. Both incoming findings were reproduced on actual GPU/public MySQL and fixed with regressions. A separate admitted integer-overflow counterexample returned -2 instead of native MO 1690/22003; it now declines before execution. Fresh CI is pending for the delivered head, and the reviewer has been asked to reassess the fixes. Local passes do not establish required CI success.

C remains opt-in. D begins after C merges and stays one complete public campaign PR for all-22 SF1/SF10, bounded comparison, lifecycle/resource/process evidence and performance gates. E/F retain their separate recovery-readiness design. References #28968 and #28966; neither issue is closed by C.

Conflict resolution at b34111b4c9 (2026-10-10): normally merged authoritative main e5dd4724f7. The single conflict was two equivalent asynchronous histogram-test fixes; main's stopped-timer notification barrier is retained unchanged. All 28 prior C paths were checked: only this test changed, while the DATE/CASE/integer review fixes and merged native pins remain intact.

Validation at this merged head passes: affected owning packages normal/race, 100 focused metrics race repetitions, fresh native generations and combined release/package, all 22 actual preparations (1.116s), full public GPU normal/race (9.762s/11.999s), and full combined bridge/coexistence (1.108s). Both public runs' 34 terminal events verify completed GPU work, exact profile/capability31, healthy cleanup, no fallback and zero retained credits. Mandatory pre-push SCA passes with zero findings; self-review has no unresolved implementation blocker. New-head required CI and human reviewer approval remain the merge gates.

@matrix-meow matrix-meow added the size/XL Denotes a PR that changes [1000, 1999] lines label Oct 9, 2026
…act-decimal

# Conflicts:
#	pkg/util/metric/mometric/metric_exporter_test.go

@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.

Deep re-review of base/merge-base e5dd4724f782067e1c76c38c9d2f55cebceda4a9 → head b34111b4c9e94cda034d199a2c362072fe98d808, including the follow-up to reviewed b39755ff4e and unchanged merged Sirius pin 908ffc75a58b3be496b2172795e416326416e7ec. APPROVE: both previous blocking findings are resolved; no new blocker found. No subagents or production edits.

Comment closure

  • Zero DATE: the typed plan now compares against the same shifted types.ZeroDate representation used by the native reader and produces 0 for that sentinel. Normal/NULL dates retain the component path. The two casts normalize the pinned component kernel's physical INT16 result through INT32 to the declared BIGINT, avoiding the importer eliminating a same-type cast. The existing public fixture now persists zero DATE, compares 0/0/0, retains year-0001/leap-day/year-9999/NULL controls, and restores SQL mode.
  • CASE descriptor: IfThen is wrapped in the existing checked mo_decimal_cast annotation with the complete MO-bound result descriptor. The required-arm/nullable-condition fixture checks both branch values and nullable DECIMAL(15,2) public metadata. The previous nullable-arm and masked-error controls remain. This restores MO as the type authority rather than copying the native arm-only inference rule.
  • The additional ordinary integer +/-/* admission restriction correctly contains a demonstrated unchecked-consumer overflow gap. It applies to embedded post-scan work; MO-owned scan filters keep their normal executor and Flight retains its existing contract. Checked exact arithmetic and integral aggregates remain available. Read both replies and all current PR review threads; none remains substantively unresolved.

Design and complete change map

The approved semantic design (#29449, blob 42a89f09a1d168d02b9583cb3ea7b4de6dbb5634) and C/D delivery split (#29690) remain applicable. Reviewed all current changed hunks, separately accounting for implementation +825/-59 in 12 files, tests +939/-10 in 13 files, documentation +424/-0 in one file, plus one dependency-pin update. The histogram synchronization change from the earlier review is now inherited unchanged from main and is not a remaining PR delta.

The five closures remain coherent: immutable runtime capability → candidate profile; bound types/overloads/literals → exact wire/native preparation; Decimal256 reader publication → borrowed result; native status → public error/terminal evidence; merged SDK pin → supported delivery. Existing MO readers own filters, transactions and snapshots. Input credit, result leases, cancellation and query cleanup retain their established owners. No second allocator, session state machine, execution framework or storage path is introduced. Flight/default execution and the ABI remain separate existing contracts rather than partially mixed exact-decimal paths.

Q1: publication acquires credit before allocation and defers release; result vectors borrow the existing lease and batch cleanup runs after fill; native numeric failure remains primary ahead of cancellation fallout. Q2: native cancellation stays independent of data-path locking, then joins producers before destruction. Q3: existing 64 MiB windows, constant expansion accounting, bounded admission/columns/plan/heading sizes remain effective for 32-byte values; terminal schema evidence is emitted once per execution, without raw headings/values. No new shared mutable state or worker is added.

Tests prove distinct layers: descriptor/wire admission, publication/borrowing, error identity, native preparation, and public differential behavior. The new regression scenarios extend the existing GPU/public fixture; they do not add another cluster framework or weaken previous assertions. Documentation records the current contract and explicitly separates historical evidence and later campaign gates.

Adversarial checks and performance

Challenged the fixes beyond the original reproducers: nullable conditions with required arms, nested CASE, CASE consumed by arithmetic, embedded integer aggregate arithmetic versus MO-owned integer scan filters, DATE versus unsupported quarter, and exact versus legacy profiles. Traced the pinned native normalizer/evaluator, not just the new Go test assertions. Identical decimal cast annotations are removed or return the original operand without copying coefficients.

Unhinted TP and ordinary prepared execution return at the existing offload gate before any new capability/profile work. Embedded DATE extraction now does extra comparison/conditional/cast work, and exact arithmetic, wide aggregates and terminal schema hashing have real costs. This approval does not establish GPU speedup or no performance regression. The approved opt-in C scope remains intact; D's all-22 SF1/SF10, resource/lifecycle and performance campaign is still required before its acceptance or broader rollout. #28968/#28966 remain open.

Validation and limits

  • Locally at this exact head: focused exact/embedded/extract tests passed (3.428s), including all 22 exporter inventory cases and the three new DATE/CASE/integer controls. A private overlay probe using the real parser/optimizer passed seven admission/serialization counterexample/control scenarios (1.123s). No source files were changed.
  • Reused exact-head CI run 38033352247, completed success. Inspected its downloaded ut-summary checkpoint artifact: substrait, compile, siriusbridge and cnservice were all explicitly selected in the light race-test group, which terminated with status 0. Required CI is green. These CPU checks do not execute the GPU-tagged fixture.
  • GPU/public execution evidence is the author's recorded current-head result: all 22 native preparations (1.116s), full public normal/race runs (9.762s/11.999s), and combined bridge/coexistence (1.108s), with 34 terminal events per public run showing completed GPU work, exact profile/capability31, no fallback, healthy cleanup and zero retained credits. The review replies also record actual GPU red/green closure of both original findings. I checked the retained assertions and pinned consumer source; I did not execute CUDA locally or independently reproduce those GPU timings.
  • git diff --check passed; the isolated source worktree is clean. Native CGo inputs and the Sirius pin relevant to reused evidence remain unchanged. The original CPU oracle established the zero-date and nullable-CASE public contract; it is not represented as GPU execution.

@mergify

mergify Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

This branch was successfully deployed

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

Labels

size/XL Denotes a PR that changes [1000, 1999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants