Conversation
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: `ALTER TABLE ... ADD ROLLUP r (v, id) DUPLICATE KEY(v)` is accepted by FE when `v` is a VARIANT column, but the asynchronous rollup job then fails in BE and ends up CANCELLED: ``` rollup tasks failed on same tablet reach threshold 1, reason=task type: ALTER, status_code: NOT_IMPLEMENTED_ERROR, status_message: [NOT_IMPLEMENTED_ERROR] not implemented ``` Root cause: when the user lists the duplicate keys explicitly, `MaterializedViewHandler.checkAndPrepareMaterializedView(AddRollupOp ...)` never checks the key column type. The implicit branch stops at the first column that `couldBeShortKey()` rejects, and CREATE MATERIALIZED VIEW rejects these types too, but the explicit branch lets them through. BE sorts the rollup rows by the duplicate keys and cannot compare these types. Checked on a cluster before the fix: | duplicate key type | rollup job result | |---|---| | VARIANT | CANCELLED, NOT_IMPLEMENTED_ERROR | | ARRAY / MAP / STRUCT | CANCELLED, `OlapColumnDataConvertor* not support get_data_at` | | JSON | CANCELLED, INTERNAL_ERROR `value 65537 cast to type t out of range` | | STRING / DOUBLE / IPV6 | FINISHED | Fix: reject a duplicate key whose type `isOnlyMetricType()` (hll, bitmap, quantile_state, array, map, struct, jsonb, variant) when the statement is submitted, with the same `Type.OnlyMetricTypeErrorMsg` that CREATE MATERIALIZED VIEW uses. Types that BE can sort, such as STRING and DOUBLE, keep working as before. The same function also serves the ROLLUP clause of CREATE TABLE, which now reports this reason too. ### Release note ADD ROLLUP with an explicit DUPLICATE KEY on a VARIANT, ARRAY, MAP, STRUCT, JSON, HLL, BITMAP or QUANTILE_STATE column is now rejected immediately instead of creating a job that fails later. ### Check List (For Author) - Test: Regression test (rollup_p0/test_rollup_dup_key_type) - Behavior changed: Yes. These rollups are rejected when submitted instead of being cancelled asynchronously. - Does this need documentation: No Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 27787 ms |
Contributor
TPC-DS: Total hot run time: 153899 ms |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 27504 ms |
Contributor
TPC-DS: Total hot run time: 153039 ms |
Contributor
ClickBench: Total hot run time: 23.93 s |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
ALTER TABLE ... ADD ROLLUP r (v, id) DUPLICATE KEY(v)is accepted by FE whenvis a VARIANT column, but the asynchronous rollup job then fails in BE and ends up CANCELLED:Root cause: when the user lists the duplicate keys explicitly,
MaterializedViewHandler.checkAndPrepareMaterializedView(AddRollupOp ...)never checks the key column type. The implicit branch stops at the first column thatcouldBeShortKey()rejects, and CREATE MATERIALIZED VIEW rejects these types too, but the explicit branch lets them through. BE sorts the rollup rows by the duplicate keys and cannot compare these types. Checked on a cluster before the fix:OlapColumnDataConvertor* not support get_data_atvalue 65537 cast to type t out of rangeFix: reject a duplicate key whose type
isOnlyMetricType()(hll, bitmap, quantile_state, array, map, struct, jsonb, variant) when the statement is submitted, with the sameType.OnlyMetricTypeErrorMsgthat CREATE MATERIALIZED VIEW uses. Types that BE can sort, such as STRING and DOUBLE, keep working as before. The same function also serves the ROLLUP clause of CREATE TABLE, which now reports this reason too.Release note
ADD ROLLUP with an explicit DUPLICATE KEY on a VARIANT, ARRAY, MAP, STRUCT, JSON, HLL, BITMAP or QUANTILE_STATE column is now rejected immediately instead of creating a job that fails later.
Check List (For Author)
🤖 Generated with Claude Code