-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[BugFix] fix json length check #66628
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BugFix] fix json length check #66628
Conversation
Signed-off-by: Murphy <mofei@starrocks.com>
🧪 CI InsightsHere's what we observed from your CI run for b81f3cd. 🟢 All jobs passed!But CI Insights is watching 👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request fixes an off-by-one error in JSON to VARCHAR schema change validation. The issue was that when converting a JSON column to VARCHAR, the code incorrectly rejected VARCHAR with exactly the minimum required length of 1024 bytes.
Key Change:
- Fixed the comparison operator in
Column.checkSchemaChangeAllowed()from<=to<to allow VARCHAR(1024) when converting from JSON type, which has a minimum type size of 1024 bytes.
|
|
@cursor review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no bugs!
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 1 / 1 (100.00%) file detail
|
|
@Mergifyio backport branch-3.5 |
|
@Mergifyio backport branch-4.0 |
✅ Backports have been createdDetails
|
✅ Backports have been createdDetails
|
(cherry picked from commit ed8b59b)
(cherry picked from commit ed8b59b)
(cherry picked from commit ed8b59b)
(cherry picked from commit ed8b59b)
Why I'm doing:
What I'm doing:
Minimal length is 1024, which should be allowed.
Fixes #https://github.com/StarRocks/StarRocksTest/issues/10673
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check:
Note
Permits schema change from JSON to CHAR/VARCHAR when target length equals the JSON minimum (e.g., 1024).
Column.checkSchemaChangeAllowed, relax JSON length check: change<=to<so converting JSON toCHAR/VARCHARis allowed when targetstrLenequalsPrimitiveType.JSON.getTypeSize().Written by Cursor Bugbot for commit b81f3cd. This will update automatically on new commits. Configure here.