github-actions[bot] commented on code in PR #65453:
URL: https://github.com/apache/doris/pull/65453#discussion_r3602095895
##########
regression-test/suites/inverted_index_p0/index_change/test_add_drop_index_with_delete_data.groovy:
##########
@@ -230,8 +208,9 @@ suite("test_add_drop_index_with_delete_data",
"inverted_index"){
wait_for_last_col_change_finish(indexTbName1, timeout)
if (!isCloudMode()) {
- sql "build index idx_desc on ${indexTbName1}"
- wait_for_build_index_on_partition_finish(indexTbName1, timeout)
+ run_index_change_job_and_wait(indexTbName1, timeout) {
Review Comment:
This stricter wait proves the rebuild job finished, but the suite still does
not prove that the rebuilt `idx_desc` is readable. Every later MATCH leaves
both downgrade paths enabled, so a missing file or absent iterator can scan the
base rows and then apply the delete predicates, producing exactly the expected
results. Please add a selective post-rebuild MATCH with both
`enable_fallback_on_missing_inverted_index=false` and
`enable_match_without_inverted_index=false` (or an equivalent positive
index-profile assertion) before the ordinary result checks.
##########
regression-test/suites/inverted_index_p0/index_change/test_drop_index_on_partition.groovy:
##########
@@ -76,27 +44,37 @@ suite("test_drop_index_on_partition", "inverted_index") {
// create inverted index
sql "CREATE INDEX idx_v2 ON ${tableName1}(v2) USING INVERTED"
- wait_for_last_build_index_finish(tableName1, timeout)
+ wait_for_last_col_change_finish(tableName1, timeout)
+ if (!isCloudMode()) {
+ run_index_change_job_and_wait(tableName1, timeout) {
+ build_index_on_table("idx_v2", tableName1)
+ }
+ def physical_index_result = sql """SELECT /*+
SET_VAR(enable_fallback_on_missing_inverted_index=false) */
Review Comment:
The new probes establish only the pre-DROP state. Once the jobs finish,
cases 1, 2, and 10 either enable missing-index fallback or perform a full scan,
so both a no-op partition delete and an overbroad delete that strips an
untouched partition produce the same asserted rows. Please add partition-pruned
postconditions with both `enable_fallback_on_missing_inverted_index=false` and
`enable_match_without_inverted_index=false`: the dropped partition should raise
the expected no-index error, while an untouched partition must still succeed.
This is distinct from the earlier fix that made the files exist before DROP; it
verifies the DROP result and partition selectivity.
##########
regression-test/suites/inverted_index_p0/index_change/test_index_change_with_cumulative_compaction.groovy:
##########
@@ -154,13 +130,22 @@ suite("test_index_change_with_cumulative_compaction",
"nonConcurrent") {
sql """ CREATE INDEX idx_date ON ${tableName}(`date`) USING INVERTED
"""
sql """ CREATE INDEX idx_city ON ${tableName}(`city`) USING INVERTED
"""
- // build index
- build_index_on_table("idx_user_id", tableName)
- wait_for_build_index_on_partition_finish(tableName, timeout)
- build_index_on_table("idx_date", tableName)
- wait_for_build_index_on_partition_finish(tableName, timeout)
- build_index_on_table("idx_city", tableName)
- wait_for_build_index_on_partition_finish(tableName, timeout)
+ // Cloud BUILD INDEX is table-scoped; local BUILD INDEX is
index-scoped.
+ if (isCloudMode()) {
+ run_index_change_job_and_wait(tableName, timeout) {
+ build_index_on_table("idx_user_id", tableName)
+ }
+ } else {
+ run_index_change_job_and_wait(tableName, timeout) {
+ build_index_on_table("idx_user_id", tableName)
+ }
+ run_index_change_job_and_wait(tableName, timeout) {
+ build_index_on_table("idx_date", tableName)
+ }
+ run_index_change_job_and_wait(tableName, timeout) {
+ build_index_on_table("idx_city", tableName)
+ }
+ }
// trigger compactions for all tablets in ${tableName}
trigger_and_wait_compaction(tableName, "cumulative")
Review Comment:
The new waits prove that the three indexes exist before compaction, but
nothing exercises an index after compaction: the remaining checks are only
rowset counts and an unfiltered SELECT. A cumulative compaction that drops the
rebuilt index metadata/files therefore still passes; the full-compaction twin
has the same gap. Please add a selective post-compaction predicate in both
suites—for example `city MATCH ...` with both
`enable_match_without_inverted_index=false` and
`enable_fallback_on_missing_inverted_index=false`—or inspect the output
tablets' index files.
##########
regression-test/suites/inverted_index_p0/index_change/test_add_drop_index_ignore_case_column.groovy:
##########
@@ -113,8 +89,9 @@ suite("test_add_drop_index_with_ignore_case_column",
"inverted_index"){
sql "create index idx_desc on ${indexTbName1}(description) USING INVERTED
PROPERTIES(\"parser\"=\"standard\");"
wait_for_last_col_change_finish(indexTbName1, timeout)
if (!isCloudMode()) {
- sql "build index idx_desc on ${indexTbName1}"
- wait_for_build_index_on_partition_finish(indexTbName1, timeout)
+ run_index_change_job_and_wait(indexTbName1, timeout) {
Review Comment:
Both the lower-case and upper-case CREATE/BUILD variants are followed only
by fallback-enabled MATCH queries. A case-normalization bug that leaves
`idx_desc` undiscoverable or unreadable therefore scans the rows and returns
the same results even though the build job is `FINISHED`. Please add a
selective probe after each spelling variant with both
`enable_fallback_on_missing_inverted_index=false` and
`enable_match_without_inverted_index=false` (or a positive index-profile
assertion) so this suite actually exercises the physical case-insensitive
binding.
--
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]
