yujun777 commented on PR #68180: URL: https://github.com/apache/doris/pull/68180#issuecomment-5772141969
**Re: the changes requested in this review** — the remaining P1 is fixed in 9214167dba2; the inline thread has the detail. You were right about the two reads, and my earlier claim that they bracket a concurrent window change was too strong: a window set and cleared *inside* the mapping construction is seen by neither read. Rather than adding a third conservative branch, the window now gets the answer this class already gives the other two properties that trade correctness for refresh volume (`excluded_trigger_tables`, `ivm_partition_window_limit`): an effective change to it forces a complete baseline rebuild, compared before and after inside the same MV write lock that applies the ALTER. The ALTER records the barrier itself, so no ordering of it against a partition change can leave a windowed mapping trusted; the selector's two reads stay, covering the case of a window that is set and stays set. A mapping built under a window that both reads answer "no limit" for requires an active-to-inactive change inside that interval, which is what now forces the rebuild. Two things stated rather than left to be inferred: 1. This is a deliberate behaviour change: altering `partition_sync_limit` / `partition_sync_time_unit` / `partition_sync_date_format` on an IVM MV now costs one complete baseline rebuild on the next refresh. Restating the same window compares equal and forces nothing. It follows the rule the two neighbouring properties already follow. 2. The three tests that set the window now record that barrier themselves, so they release it before the change they are actually testing; otherwise their assertions would hold because of the ALTER rather than because of the selection. Verification on this head: `IvmBaselineRebuildTest` 37/37, `MTMVPropertyUtilTest` 13/13, `MTMVTest` 23/23, `checkstyle:check` clean, and the `mtmv_p0/ivm` suites test_ivm_partition_sync_limit, test_ivm_partition_sync_limit_with_window, test_ivm_baseline_marker_scope, test_ivm_partition_baseline_rebuild, test_ivm_partition_baseline_rebuild_dup_keys, test_ivm_drop_referenced_column_baseline_rebuild, test_ivm_partition_drop_live_delta, test_ivm_replace_table_reconcile, test_ivm_replace_stream_cleanup and test_ivm_drop_column_fallback_reason green on a local cluster built from it. -- 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]
