github-actions[bot] commented on code in PR #65453:
URL: https://github.com/apache/doris/pull/65453#discussion_r3587600089
##########
regression-test/suites/inverted_index_p0/index_change/test_index_change_on_renamed_column.groovy:
##########
@@ -101,7 +78,7 @@ suite("test_index_change_on_renamed_column") {
// drop inverted index on renamed column
sql """ alter table ${tableName} drop index idx_s; """
Review Comment:
Keeping only the column wait is right for the cloud false-mode case from the
earlier thread, but this suite also runs the drop in non-cloud mode after
building `idx_s`. In FE, full-table `INVERTED` drops in non-cloud still take
the light-index path: `SchemaChangeHandler` sets `supportLightIndexChange` for
`INVERTED`/`ANN` when `Config.isNotCloudMode()`, and
`modifyTableLightSchemaChange` calls `buildOrDeleteTableInvertedIndices` for
drops. That enqueues delete `IndexChangeJob`s after the metadata/column job, so
the non-cloud run can finish this suite without waiting for the physical delete
job it just submitted. Please keep this column-only for cloud false-mode, but
also snapshot and wait for the new build-index/delete rows when the full-table
drop is a non-cloud `INVERTED` drop.
##########
regression-test/suites/inverted_index_p0/index_change/test_index_change_4.groovy:
##########
@@ -36,19 +36,33 @@ suite("test_index_change_4") {
++finished_num;
}
}
- if (finished_num == expected_finished_num) {
Review Comment:
The V1 branch still does not snapshot before its explicit `BUILD INDEX`.
Earlier on the same `test_index_change_4_v1` table, the test drops two inverted
indexes and waits for those delete jobs to finish. This helper then succeeds
whenever `SHOW BUILD INDEX WHERE TableName = ...` is nonempty and every
returned row is `FINISHED`, so if the later `build_index_on_table("idx_note",
tableName)` path ever fails to create a new job row, the already-finished drop
rows are enough for the wait to pass. Please mirror the V2 branch by capturing
`get_build_index_job_ids(tableName)` immediately before the build and using the
snapshot-filtered shared wait.
##########
regression-test/suites/inverted_index_p0/index_change/test_index_change_on_renamed_column.groovy:
##########
@@ -101,7 +78,7 @@ suite("test_index_change_on_renamed_column") {
// drop inverted index on renamed column
sql """ alter table ${tableName} drop index idx_s; """
- wait_for_last_build_index_finish(tableName, timeout)
+ wait_for_last_col_change_finish(tableName, timeout)
Review Comment:
Keeping only the column wait is right for the cloud false-mode case from the
earlier thread, but this suite also runs the drop in non-cloud mode after
building `idx_s`. In FE, full-table `INVERTED` drops in non-cloud still take
the light-index path: `SchemaChangeHandler` sets `supportLightIndexChange` for
`INVERTED`/`ANN` when `Config.isNotCloudMode()`, and
`modifyTableLightSchemaChange` calls `buildOrDeleteTableInvertedIndices` for
drops. That enqueues delete `IndexChangeJob`s after the metadata/column job, so
the non-cloud run can finish this suite without waiting for the physical delete
job it just submitted. Please keep this column-only for cloud false-mode, but
also snapshot and wait for the new build-index/delete rows when the full-table
drop is a non-cloud `INVERTED` drop.
##########
regression-test/suites/inverted_index_p0/index_change/test_index_change_4.groovy:
##########
@@ -36,19 +36,33 @@ suite("test_index_change_4") {
++finished_num;
}
}
- if (finished_num == expected_finished_num) {
+ if (!alter_res.isEmpty() && finished_num == expected_finished_num)
{
Review Comment:
The V1 branch still does not snapshot before its explicit `BUILD INDEX`.
Earlier on the same `test_index_change_4_v1` table, the test drops two inverted
indexes and waits for those delete jobs to finish. This helper then succeeds
whenever `SHOW BUILD INDEX WHERE TableName = ...` is nonempty and every
returned row is `FINISHED`, so if the later `build_index_on_table("idx_note",
tableName)` path ever fails to create a new job row, the already-finished drop
rows are enough for the wait to pass. Please mirror the V2 branch by capturing
`get_build_index_job_ids(tableName)` immediately before the build and using the
snapshot-filtered shared wait.
--
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]
