Skip to content

Fix data branch decimal results on main#24894

Open
gouhongshen wants to merge 7 commits into
matrixorigin:mainfrom
gouhongshen:codex/fix-data-branch-decimal-main
Open

Fix data branch decimal results on main#24894
gouhongshen wants to merge 7 commits into
matrixorigin:mainfrom
gouhongshen:codex/fix-data-branch-decimal-main

Conversation

@gouhongshen

@gouhongshen gouhongshen commented Jun 9, 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:

issue #24847

What this PR does / why we need it:

Port the data branch decimal result conversion fix to main.

  • Preserve decimal precision/scale metadata from MySQL result columns and validate invalid/missing decimal metadata.
  • Materialize decimal64/decimal128/decimal256 row values as typed MatrixOne vectors when building data branch output batches.
  • Decode and compare decimal256 raw tuple values in data branch hashdiff/change replay paths.
  • Add focused unit tests and BVT coverage with diff_14.sql for decimal64/decimal128/decimal256 data branch diff.

Notes:

Checks:

  • make -C thirdparties
  • source $HOME/.zshrc && moenv && go test ./pkg/container/types ./pkg/frontend ./pkg/vm/engine/disttae/logtailreplay
  • git diff --check upstream/main...HEAD

Draft until CI confirms this port on main.

@matrix-meow matrix-meow added the size/M Denotes a PR that changes [100,499] lines label Jun 9, 2026
@gouhongshen gouhongshen changed the title [codex] Fix data branch decimal results on main Fix data branch decimal results on main Jun 9, 2026
@gouhongshen gouhongshen force-pushed the codex/fix-data-branch-decimal-main branch from 53ecf84 to 0903902 Compare June 10, 2026 02:54
@gouhongshen

Copy link
Copy Markdown
Contributor Author

Updated this PR to address the review coverage gaps.

Covered in this push:

  • Added focused appendFromEntry() coverage for decimal256, including null handling.
  • Expanded diff_14 to cover decimal UPDATE / INSERT / DELETE paths under save_query_result=on.
  • Added a product-level pick_12 conflict case using decimal(39,4) as a non-PK value, covering fail / skip / accept behavior. This also covers the decimal256 pick output path without mixing in the separately tracked decimal256 PK issue ([Bug]: data branch pick does not support decimal256 primary keys #24902).

Local validation on main:

  • source $HOME/.zshrc && moenv && go test ./pkg/frontend ./pkg/vm/engine/disttae/logtailreplay
  • BVT diff_14: 15/15 passed
  • BVT pick_12: 69/69 passed
  • git diff --check

Re-requesting review.

@gouhongshen

Copy link
Copy Markdown
Contributor Author

已把 3.0-dev 上处理 change request 的 decimal256 data branch pick 修复 port 到 main。

本次更新 commit: 4259e88572 fix: support decimal256 data branch pick keys

本地验证:

source $HOME/.zshrc && cd /Users/ghs-mo/MOWorkSpace/matrixone-2nd && moenv && cd /private/tmp/matrixone-port-decimal-main && go test ./pkg/container/types ./pkg/container/vector ./pkg/frontend ./pkg/sql/colexec/lockop ./pkg/vm/engine/readutil ./pkg/objectio/mergeutil ./pkg/vm/engine/tae/mergesort -run 'TestSimpleTupleAllTypes|TestSingleTypeTuple|TestDecimal256OrderTuple|TestTupleSQLStrings|TestTupleSQLStringsWithEncodeDecode|TestDecodeTupleWithSchema|TestUnpackNthElement|TestType_DescString|TestFetchDecimal256Rows|TestFetchDecimal256RowsWithFilter|TestFetchDecimal256RowsWithFilterAll|TestNumericGap_AllTypes|TestIsNumericType|TestAppendNumericStringToVec_AllTypes|TestCoercePickKeyVectorToType_Decimal256ReformatsToTargetType|TestExtractPKValDecimal256|TestFormatPickKeyVectorValueAsString_AllSupportedKinds|TestLCAProbeJoinCastType|TestConstructBlockPKFilter|TestMergeBaseFilterInKind|TestMergeSortBatchesDecimal256|TestAObjMergeDecimal256' -count=1

结果:PASS。git diff --check 也已通过。

aunjgr

This comment was marked as low quality.

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 kind/test-ci size/XL Denotes a PR that changes [1000, 1999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants