Skip to content

fix: preserve SIGN numeric type in prepared statements - #28636

Merged
XuPeng-SH merged 2 commits into
matrixorigin:mainfrom
iamlinjunhong:m-28583
Sep 12, 2026
Merged

fix: preserve SIGN numeric type in prepared statements#28636
XuPeng-SH merged 2 commits into
matrixorigin:mainfrom
iamlinjunhong:m-28583

Conversation

@iamlinjunhong

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 #28583

What this PR does / why we need it:

fix: preserve SIGN numeric type in prepared statements

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

@matrix-meow matrix-meow added the size/M Denotes a PR that changes [100,499] lines label Sep 10, 2026

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

Reviewed the complete current diff at 0323d82. Extending the existing ABS deferred-numeric mechanism to SIGN is a reasonable focused fix: PREPARE avoids premature integer coercion, while EXECUTE rebinds the input overload from the runtime numeric category and retains the stable result contract. Traced marker creation, cached parameter-position eligibility, replacement/rebinding, explicit numeric CAST boundaries, and the scalar-subquery reference-preserving branch. Ordinary column expressions without a prepared parameter retain the existing binding path. The tests cover exact overload selection plus real SQL PREPARE/EXECUTE and binary COM_STMT execution, repeated fractional/integer domain changes, explicit casts, and NULL. Additional decoding/rebinding is limited to statements carrying the deferred metadata; ordinary executions do not gain a new expression-tree walk from this change. No new shared-state owner or persistent format is introduced. Reused successful current-head SCA, UT, coverage, and BVT checks; no concrete correctness, unhappy-path, or performance blocker found.

@mergify mergify Bot added the queued label Sep 12, 2026
@mergify

mergify Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-09-12 07:56 UTC · Rule: main · triggered by rule Automatic queue on approval for main
  • 🟠 Checks running · in-place
  • 🚫 Left the queue2026-09-12 09:03 UTC · at b9c3ea27f81ebd8ec4e0672b663b59c3393b9eec

This pull request spent 1 hour 7 minutes in the queue, with no time running CI.

Waiting for any of
  • check-neutral = CI Required
  • check-skipped = CI Required
  • check-success = CI Required
All conditions

Reason

Pull request #28636 has been dequeued

Pull request from fork cannot be queued. This pull request comes from a fork, and Mergify needs the author's permission to update its branch.

The author needs to enable "Allow edits from maintainers" on this pull request.

Hint

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.
If you do update this pull request, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

Tick the box to put this pull request back in the merge queue (same as @mergifyio queue).

  • Requeue this pull request

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

Labels

dequeued size/M Denotes a PR that changes [100,499] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants