Skip to content

fix(storage): handle MatrixOne aggregate and nullable parameter compatibility - #239

Merged
aptend merged 5 commits into
matrixorigin:mainfrom
loveRhythm1990:lr90/fix-matrixone-sum-decimal
Aug 26, 2026
Merged

fix(storage): handle MatrixOne aggregate and nullable parameter compatibility#239
aptend merged 5 commits into
matrixorigin:mainfrom
loveRhythm1990:lr90/fix-matrixone-sum-decimal

Conversation

@loveRhythm1990

@loveRhythm1990 loveRhythm1990 commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • replace count-like SUM expressions with conditional COUNT so MatrixOne does not return DECIMAL128 for integer counts
  • explicitly cast aggregate results that Memoria decodes as Rust numeric types
  • preserve NULL confidence averages instead of coercing unknown values to zero
  • log metrics aggregate decoding failures instead of silently reporting zero
  • avoid MatrixOne 4.2 prepared-statement NULL-state leakage by using distinct INSERT SQL shapes for nullable values
  • add aggregate and same-connection Some -> None -> Some regression coverage

Background

MatrixOne widened SUM(INT64/UINT64) to DECIMAL128(38,0), while affected Memoria queries still decoded those values as i64.

MatrixOne 4.2 also retains a prepared parameter NULL state across executions, so a non-NULL value following NULL can be persisted as NULL. The upstream fix is merged to MatrixOne main but is not present in the current 4.2 latest image.

Aggregate change: matrixorigin/matrixone@14c4e4d

Prepared parameter fix: matrixorigin/matrixone#26915

Validation

Validated against the current matrixorigin/matrixone:latest image with a 4 GB container:

  • cargo check -p memoria-storage --tests
  • storage CRUD tests: 24 passed
  • session-scope API tests: 5 passed
  • full workspace: 834 passed, 5 ignored across 53 suites
  • git diff --check

Fixes #238

Replace count-like SUM expressions with conditional COUNT and explicitly cast aggregate results before decoding them into Rust numeric types. Preserve nullable confidence averages and surface metrics decoding failures through warnings.

Refs matrixorigin#238
@aptend
aptend enabled auto-merge (squash) August 26, 2026 08:20
@loveRhythm1990 loveRhythm1990 changed the title fix(storage): handle widened MatrixOne aggregates fix(storage): handle MatrixOne aggregate and nullable parameter compatibility Aug 26, 2026
@aptend

aptend commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Blocking: the new NULL-parameter workaround changes the existing empty-string contract for session_id and superseded_by.

nullable_str() deliberately normalizes both None and Some("") to SQL NULL, and the previous single/batch insert paths used it. The new SQL-shape selection uses .is_some() and binds the original string, so Some("") now produces a bound empty string rather than a literal NULL in both paths.

Please derive the shape and bind value from nullable_str(...) (not .is_some()), then add regression coverage for empty session_id / superseded_by in both single and batch inserts.

@loveRhythm1990

Copy link
Copy Markdown
Collaborator Author

Fixed in 84415b4. Both single and batch INSERT paths now derive the SQL shape and bind values from nullable_str() for session_id and superseded_by, preserving Some("") -> SQL NULL. Added raw-database regression coverage for both fields in single and batch inserts. Validation: Rust 1.85 cargo check/clippy passed; storage CRUD 26/26; session-scope API tests 5/5.

@aptend aptend left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed 84415b4. The NULL SQL-shape selection and bound values now consistently derive from nullable_str() for both single and batch INSERTs, and the new raw-database regression tests cover the empty-string contract. No remaining blocking issues.

@aptend
aptend disabled auto-merge August 26, 2026 10:09
@aptend
aptend merged commit 54566bc into matrixorigin:main Aug 26, 2026
5 checks passed
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.

fix(storage): handle MatrixOne DECIMAL result from SUM(BIGINT)

2 participants