Repository navigation
[fix](ivm) Rename leftover row binlog to changelog in IVM tests - #69372
Merged
hello-stephen merged 1 commit intoOct 21, 2026
Merged
Conversation
### What problem does this PR solve? Issue Number: close #xxx Related PR: apache#69311, apache#69268, apache#63639 Problem Summary: apache#69311 renamed the row binlog (the change stream IVM reads) to changelog and rejected the legacy keys `binlog.format` / `binlog.need_historical_value` in every DDL. Two test files merged just before it were outside that rename sweep and now fail: the regression suite `test_ivm_datetime_sequence` (apache#69268) and the FE unit test `RelationImpliedPredicateTest#testIvmOutputPredicatesSkipHiddenColumns` (apache#63639) still created IVM base tables with `binlog.enable = true` plus `binlog.format = ROW`, which CREATE TABLE now rejects, and the FE test also gated on `enable_feature_binlog` instead of the new `enable_feature_changelog`. Fix: switch both to `changelog.enable = true` (plus `changelog.need_historical_value` where needed) and `enable_feature_changelog`; the gate in the regression cluster conf is covered by the existing CI configuration. Verified locally: the two FE UT classes pass (35 tests, 0 failures) and the regression suite passes against a cluster rebuilt from current master ("All suites success", results unchanged so the .out is untouched). ### Release note None ### Check List (For Author) - Test: Regression test / Unit Test - fe: RelationImpliedPredicateTest and IvmUtilTest pass via run-fe-ut.sh - regression: mtmv_p0 suite test_ivm_datetime_sequence passes via run-regression-test.sh - Behavior changed: No - Does this need documentation: No
yujun777
requested review from
924060929,
englefly,
morrySnow and
starocean999
as code owners
October 21, 2026 07:10
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
|
skip buildall |
morningman
approved these changes
Oct 21, 2026
starocean999
approved these changes
Oct 21, 2026
morrySnow
approved these changes
Oct 21, 2026
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
No new actionable findings in the two changed files at 134f35e. The test fixtures now use the supported changelog properties and the actual CREATE feature gate. Both MoW regression inputs retain before images; the duplicate-key unit fixture correctly omits them. No existing inline findings require carry-forward, and no additional user focus was supplied.
Critical checkpoint conclusions:
- Goal and evidence: the DDL matches PropertyAnalyzer and InternalCatalog; existing unit and regression assertions still cover the intended behavior. This review used static inspection only; no builds or tests were run.
- Scope: the change is small and confined to two test files and all four diff hunks.
- Concurrency and locking: no new threads, locks, or shared collection operations. Existing scoped global configuration handling and separate JVM test forks introduce no demonstrated new race or lock-order issue.
- Lifecycle and initialization: the unit test restores the original feature, stream, and session settings in finally. Cleanup order is preserved; no static initialization or ownership change.
- Configuration: no new option is introduced. enable_feature_changelog is the gate checked by CREATE; normal regression CI configurations already enable it.
- Compatibility: these tests adapt to an existing DDL rename; production symbols and storage formats are unchanged.
- Thrift/protobuf definitions: untouched; no field-ID or enum compatibility change.
- Parallel paths: both regression input tables and the duplicate-key unit fixture are migrated. Aggregate and plain-join IVM paths remain covered, and ivm_force_mow preserves the hidden-column unit subject.
- Conditional checks and errors: no new conditional or error path. Legacy-property rejection is avoided using supported keys; refresh waits still require SUCCESS.
- Test coverage: both showHiddenColumns values retain hidden-row-ID, nonempty-predicate, visible-slot, and score=1 assertions. The regression retains complete initialization, dimension updates, DATETIME-sequenced base updates, and a new matching key. Query and DML transparent rewrites stay disabled. No new invalid-input behavior requires additional negative cases.
- Expected results: all eight ordered query labels match the unchanged output file. The dimension update yields a2, the base value changes to 22, and key 4 appears as expected; property spelling requires no result update.
- Observability: no runtime logging or metrics path changes; existing assertion diagnostics and refresh success checks suffice for this fixture correction.
- Transactions and persistence: no production journal, replay, failover, or transaction path changes.
- Data writes and atomicity: test DML and refresh ordering are unchanged; no introduced crash or atomicity issue was found.
- FE/BE variable transport: no new variable or transport path.
- Performance and memory: no new hot-path work, allocation, retained resource, or BE memory ownership change.
- Core invariants and other issues: no changed production path affects committed-data visibility, versions, delete bitmaps, or memory tracking. Test failures remain assertions/exceptions; no other substantiated issue was found.
Converged after round 1: the complete-scope reviewer and the independent risk reviewer both returned NO_NEW_VALUABLE_FINDINGS on the same unchanged empty finding set. Main-agent verification resolved both property/stream semantics and feature-gate lifecycle risks, followed by a full changed-file and candidate sweep. No unresolved static candidates or uncovered assigned scope remain. Runtime behavior was not verified in this invocation, as required by the static-only contract.
Review converged after 1 round(s).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #69311, #69268, #63639
Problem Summary: #69311 renamed the row binlog (the change stream IVM reads) to changelog and rejected the legacy keys
binlog.format/binlog.need_historical_valuein every DDL. Two test files merged just before it fell outside that rename sweep and now fail:regression-test/suites/mtmv_p0/ivm/test_ivm_datetime_sequence.groovy(from [fix](ivm) Use type zero values for missing hidden slots #69268) — creates its IVM base tables with"binlog.enable" = "true", "binlog.format" = "ROW", "binlog.need_historical_value" = "true", which CREATE TABLE now rejects.RelationImpliedPredicateTest#testIvmOutputPredicatesSkipHiddenColumns(from [fix](mtmv) Eliminate redundant filters in nested MV candidate definitions #63639) — same legacy properties in the base-table DDL, and it gates the creation onenable_feature_binloginstead of the newenable_feature_changelog.Fix: switch both to
"changelog.enable" = "true"(plus"changelog.need_historical_value" = "true"where the before-image is needed), drop the rejectedbinlog.format, and useenable_feature_changelogas the feature gate in the FE unit test. Related IVM trace issue: #65418 .Release note
None
Check List (For Author)
run-fe-ut.sh --run org.apache.doris.nereids.mv.RelationImpliedPredicateTest,org.apache.doris.mtmv.ivm.IvmUtilTeston top of current master — 35 tests, 0 failures.run-regression-test.sh --run -d mtmv_p0 -s test_ivm_datetime_sequence— "All suites success"; query results are unchanged by the property rename, so the.outis untouched.