Skip to content

fix: enforce TINYTEXT byte length limit - #26697

Open
iamlinjunhong wants to merge 2 commits into
matrixorigin:mainfrom
iamlinjunhong:m-26687
Open

fix: enforce TINYTEXT byte length limit#26697
iamlinjunhong wants to merge 2 commits into
matrixorigin:mainfrom
iamlinjunhong:m-26687

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

What this PR does / why we need it:

fix: enforce TINYTEXT byte length limit

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

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

Request changes: provide an upgrade path for existing TINYTEXT columns.

Before this PR, getTypeFromAst persisted every TEXT-family declaration, including TINYTEXT, as T_text with Width=0. This PR recognizes TINYTEXT only while parsing new DDL and all enforcement/formatting helpers key exclusively on T_text + Width=255.

Consequently, a TINYTEXT table created on any pre-fix version still resolves as Width=0 after upgrade. assignmentCastFunctionName then deliberately selects ordinary cast (the new test even asserts this for T_text/Width=0), so the original 1000-byte reproduction continues to store 1000 bytes. SHOW CREATE and CREATE TABLE ... LIKE also treat/copy that legacy column as unbounded TEXT. The fix therefore applies only to tables created or explicitly rebuilt after deployment.

Please add a durable subtype recovery/migration path (for example, versioned catalog metadata/backfill, or a carefully validated recovery from TableDef.Createsql) and define what happens when legacy columns already contain oversized rows. Add an upgrade regression that starts from a pre-fix catalog definition and verifies future strict/non-strict writes plus SHOW/LIKE behavior. The new-table byte-boundary, assignment-mode, DEFAULT, and formatting tests all pass on exact head a9dae6d80b97; the missing piece is existing-schema compatibility.

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

Labels

size/L Denotes a PR that changes [500,999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants