airborne12 opened a new pull request, #69444:
URL: https://github.com/apache/doris/pull/69444
### What problem does this PR solve?
Issue Number: None
Related PR: #68674
Problem Summary:
When an ALTER TABLE statement combines an index change that can run on the
light index change path with a clause that needs a heavy schema change, the
light path wins and the heavy work is never done:
```sql
-- v was INT, idx is an INVERTED index: the shadow column __doris_shadow_v
BIGINT is published and the
-- schema change job is recorded as FINISHED, but no BE conversion job runs
ALTER TABLE t MODIFY COLUMN v BIGINT NULL, DROP INDEX idx;
ALTER TABLE t MODIFY COLUMN v BIGINT NULL, ADD INDEX i2 (x) USING INVERTED;
-- enable_add_index_for_new_data = false: bf_v2 must be built for the
existing data, but it is only
-- published for new data
ALTER TABLE t ADD INDEX bf_v2 (v2) USING BLOOMFILTER, DROP INDEX idx;
```
The same holds for every column or property clause that cannot run as a
light schema change (dropping a key column, reordering columns, sequence
mapping tables, table properties). On a changelog table such a statement also
bypassed the check that only light changes are allowed, because that check
looked at the candidate flag `lightIndexChange`.
Root cause: in `SchemaChangeHandler#process` every clause that is not light
sets `lightSchemaChange = false`, index clauses included, so nothing records
that a clause needs a heavy schema change. The dispatch then takes the light
index change branch whenever an index clause can be light.
Fix: the clause loop also records `requiresHeavyChange` for column and
property clauses that cannot be light and for an `ADD INDEX` that cannot be
light. After the loop, the light index change path is only used when no clause
needs heavy work; otherwise the statement runs as a heavy schema change job
(`createJob`), which applies the index changes as well, exactly as it does when
`enable_light_index_change` is off. The changelog check uses the path that is
actually taken. No statement is newly rejected (other than on changelog tables,
where the heavy path was never allowed).
Notes:
- A `DROP INDEX` without light support still needs no heavy work: in a light
index change only its metadata is dropped (same-statement NGRAM_BF to
BLOOMFILTER replacement in local mode keeps working).
- The light DROP INDEX pre-check from #68674 still runs before the clause
loop, because whether a clause needs heavy work is only known after it is
processed. In cloud mode it therefore still requires a compute group for such a
statement, as before this change.
### Release note
ALTER TABLE statements that combine a light index change with a column,
property or index change that needs a heavy schema change now run as a heavy
schema change job, instead of publishing the new schema without converting the
data or building the index.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
`SchemaChangeHandlerTest#testLightIndexChangeWithHeavyWorkRunsHeavySchemaChange`
runs `MODIFY COLUMN vi BIGINT NULL, DROP INDEX idx_v1` and, with
`enable_add_index_for_new_data = false`, `ADD INDEX bf_v2 (v2) USING
BLOOMFILTER, DROP INDEX idx_v3`. It checks each creates one schema change job
and no index change job, and after the jobs finish that `vi` is BIGINT, no
shadow column is left and the index list is as expected. The test fails without
the change.
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
Such statements run as a heavy schema change job instead of the light
index change path.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
