Skip to content

[BugFix] Preserve key column keyness on MODIFY COLUMN for DUPLICATE/UNIQUE tables#75329

Open
richerduong wants to merge 1 commit into
StarRocks:mainfrom
richerduong:keyness-fix-74553
Open

[BugFix] Preserve key column keyness on MODIFY COLUMN for DUPLICATE/UNIQUE tables#75329
richerduong wants to merge 1 commit into
StarRocks:mainfrom
richerduong:keyness-fix-74553

Conversation

@richerduong

Copy link
Copy Markdown

Why I'm doing:

ALTER TABLE ... MODIFY COLUMN on a key column of a DUPLICATE or UNIQUE table, without restating the KEY keyword, fails with a misleading Can not change aggregation type error, even when only the column comment or type is changed. Omitting KEY is a very natural thing to do, and the error gives no hint about the real cause.

CREATE TABLE t (id INT NOT NULL, name VARCHAR(12) NOT NULL, v FLOAT NOT NULL)
DUPLICATE KEY(id, name) DISTRIBUTED BY HASH(id) BUCKETS 1 PROPERTIES ("replication_num" = "1");

-- Change only the comment of the key column `name`, keeping the exact same type:
ALTER TABLE t MODIFY COLUMN name VARCHAR(12) COMMENT 'full name';
-- ERROR 1064 (HY000): Can not change aggregation type

-- Workaround today: restate KEY explicitly
ALTER TABLE t MODIFY COLUMN name VARCHAR(12) KEY COMMENT 'full name';  -- succeeds

The same problem reproduces on UNIQUE KEY tables. It does not happen on PRIMARY KEY or AGGREGATE tables (see root cause).

What I'm doing:

Root cause: in SchemaChangeHandler.processModifyColumn, the column rebuilt from the MODIFY COLUMN clause is non-key when KEY is omitted. The UNIQUE and DUPLICATE branches then assign it an aggregation type (REPLACE or NONE), silently demoting the existing key column to a value column. That implicitly assigned aggregation type later clashes with the original key column's null aggregation type in Column.checkSchemaChangeAllowed, which throws Can not change aggregation type.

A keyness flip is not a supported MODIFY COLUMN operation. The range-distribution guard in the same method already rejects it explicitly. This change mirrors the existing PRIMARY KEY backward-compatibility behavior: when the base column is a key and KEY is omitted, preserve its keyness instead of demoting it. PRIMARY KEY and AGGREGATE tables already behave this way.

Adds ModifyColumnKeynessTest covering DUPLICATE and UNIQUE key-column modifications without the KEY keyword, plus a value-column case to guard against an over-broad fix. The two key-column tests fail with Can not change aggregation type on the unpatched code and pass with the fix; the value-column test passes in both cases.

Fixes #74553

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
    • This pr needs auto generate documentation
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 4.1
    • 4.0
    • 3.5

…NIQUE tables

MODIFY COLUMN on a key column of a DUPLICATE or UNIQUE table, without
restating the KEY keyword, failed with a misleading "Can not change
aggregation type" error (e.g. changing only a key column's comment).

Root cause: in SchemaChangeHandler.processModifyColumn, the rebuilt column
defaults to non-key when KEY is omitted. The UNIQUE and DUPLICATE branches
then assigned it an aggregation type (REPLACE/NONE), silently demoting the
key column to a value column. That fabricated aggregation type later clashed
with the original key column's null aggregation type in
Column.checkSchemaChangeAllowed, surfacing the misleading error.

A keyness flip is not a supported MODIFY COLUMN operation (the
range-distribution path already rejects it explicitly). Mirror the existing
PRIMARY KEY behavior: when the base column is a key and KEY is omitted,
preserve its keyness instead of demoting it. PRIMARY KEY and AGGREGATE tables
already behaved this way.

Adds ModifyColumnKeynessTest covering DUPLICATE and UNIQUE key-column modify
without the KEY keyword, plus a value-column guard against an over-broad fix.

Signed-off-by: Richer Duong <duongricher@gmail.com>
@richerduong

Copy link
Copy Markdown
Author

Hi @gengjun-git and @meegoo, could you please review this?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MODIFY key column without keyness on DUPLICATE tables, failing with misleading "Can not change aggregation type"

1 participant