englefly commented on code in PR #68990: URL: https://github.com/apache/doris/pull/68990#discussion_r4254514362
########## regression-test/suites/nereids_rules_p0/rewrite_simple_agg_to_constant/min_max_after_delete.groovy: ########## @@ -0,0 +1,94 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// A no-filter MIN/MAX is rewritten to a constant from SimpleAggCacheMgr. That cache runs its +// internal SQL on the statistics connect context, which forces MIN/MAX onto the segment zone maps. +// A zone map still covers the rows a delete predicate filters out, so the cache used to store the +// max of a row that had been DELETEd and the rewrite handed that value back to the user. +suite("min_max_after_delete") { + // The rewrite is only reached when the plan is built, so the SQL result cache would answer the + // later queries without ever consulting SimpleAggCacheMgr. + sql "SET enable_sql_cache=false" + + sql "DROP DATABASE IF EXISTS test_min_max_after_delete" + sql "CREATE DATABASE test_min_max_after_delete" + sql "USE test_min_max_after_delete" + + sql """ + CREATE TABLE dup_min_max_deleted ( + k VARCHAR(20) NULL, + d DATETIME NULL, + n BIGINT NULL + ) DUPLICATE KEY(k) + DISTRIBUTED BY HASH(k) BUCKETS 1 + PROPERTIES('replication_num' = '1') Review Comment: Fixed in b92ad1d1797. The test table now sets `disable_auto_compaction = true`, with a comment recording why the test depends on it, and the commit message says the same. Confirmed the premise before applying it: `TableProperty` defaults the property to `false`, so auto compaction is on unless a table opts out, and 789 suites under `regression-test/` already set it this way. Good catch on the assertion too. The `constant exprs` poll only proves the cache loaded, never that the deleted row was still behind its predicate, so it could not have caught this. Re-ran `nereids_rules_p0/rewrite_simple_agg_to_constant` after the change: 4 suites, 0 failures. -- 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]
