[fix](table stream) remove base table cache & use dynamic schema - #67173
Merged
morrySnow merged 2 commits intoAug 28, 2026
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
|
/review |
morrySnow
previously approved these changes
Aug 26, 2026
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Requesting changes for two correctness issues.
Goal and scope
The production change is focused, and the added regression covers live-FE schema reflection across ADD/DROP. However, removing the serialized stream schema breaks mixed-version metadata replay, so the PR does not yet meet its compatibility goal safely.
Critical checkpoints
- Correctness and compatibility: a stream newly persisted by this code has no
fullSchemafield for an older FE reader, which can expose a zero-column stream during rolling upgrade or failover. This is blocking. - Persistence and lifecycle: create-table journal/image serialization and old-reader reconstruction were traced; no compatibility fallback or format gate is present.
- Concurrency: statement collection/binding, stream/base locks, DROP/rename, and allowed row-binlog schema-change paths were checked. No additional PR-introduced lock-order, race, or identity defect was substantiated.
- Parallel paths: local and Cloud stream construction and metadata access paths were checked; no distinct additional defect was found.
- Tests and expected results: the golden output is ordered and consistent with the intended live schema behavior, but the ALTER wait helper does not actually assert completion. Mixed-version journal/image coverage is also missing. I did not run builds or tests in this review-only environment.
- Configuration, FE/BE protocol variables, transactions, and data writes: this PR adds none and introduces no separate checkpoint issue.
- Performance and observability: dynamic schema allocation/catalog lookup was reviewed; planning snapshots the wrapper and no actionable hot-path or observability issue was established.
- User focus: no additional focus was provided; the whole PR was reviewed.
- CI snapshot: BE UT, Cloud UT, and coverage are successful; compile, FE UT, performance, and code-review checks are still pending at submission time.
| public BaseTableStream(long id, String streamName, List<Column> fullSchema, TableIf baseTable) { | ||
| super(id, streamName, TableType.STREAM, fullSchema); | ||
| public BaseTableStream(long id, String streamName, TableIf baseTable) { | ||
| super(id, streamName, TableType.STREAM, null); |
Contributor
There was a problem hiding this comment.
[P1] Preserve a serialized schema for older FE readers
Passing null here leaves fullSchema absent because Doris's Gson configuration does not serialize null fields. A stream created by this FE is therefore written without the only schema representation understood by the pre-change reader: when an older FE replays the CreateTableInfo journal entry or loads an image during rolling upgrade/failover, its no-arg construction leaves an empty schema and its inherited accessors expose zero columns. Please retain a serialized compatibility snapshot (new code can still ignore it at runtime), or explicitly version/gate this metadata format and cover the new-writer/old-reader path.
Contributor
There was a problem hiding this comment.
Confirmed this is still unresolved in the merged head 6268e7a5548. BaseTableStream still passes null as fullSchema, and the Doris Gson builder still omits null fields. A stream created by this version is therefore persisted without the schema snapshot required by pre-change FE readers; replaying the journal or loading the image on an older FE leaves an empty schema. Please track a follow-up that preserves a compatibility snapshot or explicitly gates the metadata format, with new-writer/old-reader coverage for both journal replay and image loading.
Contributor
TPC-H: Total hot run time: 16817 ms |
Contributor
TPC-DS: Total hot run time: 82299 ms |
Contributor
ClickBench: Total hot run time: 14.53 s |
Contributor
Author
|
run cloud_p0 |
2 similar comments
Contributor
Author
|
run cloud_p0 |
Contributor
Author
|
run cloud_p0 |
TsukiokaKogane
force-pushed
the
remove_stream_base_table_cache_and_refactor_dynamic_schema
branch
from
August 27, 2026 06:29
13bb414 to
fa9b9fd
Compare
Contributor
Author
|
run buildall |
1 similar comment
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 16692 ms |
Contributor
TPC-DS: Total hot run time: 81403 ms |
Contributor
ClickBench: Total hot run time: 14.44 s |
fix format remove redundant check remove redundant check
TsukiokaKogane
force-pushed
the
remove_stream_base_table_cache_and_refactor_dynamic_schema
branch
from
August 27, 2026 10:05
86f6ea4 to
6268e7a
Compare
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 16964 ms |
Contributor
TPC-DS: Total hot run time: 81981 ms |
Contributor
ClickBench: Total hot run time: 14.45 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
morrySnow
approved these changes
Aug 28, 2026
luwei16
added a commit
to luwei16/Doris
that referenced
this pull request
Aug 28, 2026
### What problem does this PR solve? Issue Number: close apache#67109 Related PR: apache#67173 Problem Summary: Sync the branch with the latest master and resolve the overlapping table stream dynamic-schema implementation. Keep the upstream base-table ID lookup and stream LSN support while retaining schema generation under the base-table read lock, explicit failure when the base table is unavailable, and an empty persisted stream schema. ### Release note None ### Check List (For Author) - Test: Unit Test - CreateTableStreamTest - ExplainTableStreamPlanTest - TableStreamBaseTableInfoTest - Behavior changed: No (merge conflict resolution preserves the intended PR behavior) - Does this need documentation: No
Contributor
|
PR approved by anyone and no changes requested. |
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
69 tasks
superdiaodiao
added a commit
to superdiaodiao/doris
that referenced
this pull request
Sep 9, 2026
Adds a non-cloud regression test asserting that after the base table of a stream is dropped, information_schema.table_streams reports N/A for BASE_TABLE_NAME / BASE_TABLE_DB / BASE_TABLE_CTL / BASE_TABLE_TYPE instead of the stale identity of the dropped table. The FE-side fix this PR originally carried is obsolete: apache#67173 removed the cached BaseTableStream.baseTable field outright, so getBaseTableNullable() now resolves through baseTableInfo on every call and already returns null for a dropped table. That change shipped without a regression test for this path, which is what remains here. Change-Id: I9b7bfb580afaf5ee67974d748f5e6a847953b957 Signed-off-by: Chester <chesterxu@tencent.com>
superdiaodiao
added a commit
to superdiaodiao/doris
that referenced
this pull request
Sep 9, 2026
Adds a non-cloud regression test asserting that after the base table of a stream is dropped, information_schema.table_streams reports N/A for BASE_TABLE_NAME / BASE_TABLE_DB / BASE_TABLE_CTL / BASE_TABLE_TYPE instead of the stale identity of the dropped table. The FE-side fix this PR originally carried is obsolete: apache#67173 removed the cached BaseTableStream.baseTable field outright, so getBaseTableNullable() now resolves through baseTableInfo on every call and already returns null for a dropped table. That change shipped without a regression test for this path, which is what remains here. Change-Id: I9b7bfb580afaf5ee67974d748f5e6a847953b957 Signed-off-by: Chester <chesterxu@tencent.com> Signed-off-by: Chester <42577861+superdiaodiao@users.noreply.github.com>
12 tasks
luwei16
added a commit
that referenced
this pull request
Sep 18, 2026
### What problem does this PR solve? Issue Number: None Related PR: #67173 Problem Summary: Dropping a base table leaves its table stream object in metadata, but the table stream metadata function previously reported the retained stream as enabled and non-stale even though its original base table ID could no longer be resolved. This change derives the displayed health from the same base-table lookup used for the identity columns, while preserving the stream configured state so recovering the original table restores its health. ### Release note Table streams whose base table is unavailable are now reported as disabled and stale with an explicit reason. ### Check List (For Author) - Test: Unit Test - DropTableStreamTest - TableStreamManagerCleanupTest - TableStreamBaseTableInfoTest - Behavior changed: Yes. Missing-base streams are reported as disabled and stale. - Does this need documentation: No
morningman
added a commit
that referenced
this pull request
Sep 18, 2026
…on PRs from master in merge order (#67783 #68034 #68033 #68057 #68094 #68087) (#68151) Cherry-picked from #67783, #68034, #68033, #68057, #68094, #68087 Batch pick of every merged PR carrying the `incremental-computation` label that `branch-incremental-computation` does not have yet (no `incremental-computation-picked` label), in the order they landed on master (`git log --first-parent`). One commit per PR, each created with `git cherry-pick -x` so the message ends with `(cherry picked from commit <master sha>)`. Follows the same convention as #67830, #68017 and #68073. | # | Master commit | PR | Title | |---|---|---|---| | 1 | e545b13 | #67783 | [fix](policy) Enforce row policies on MOW time travel | | 2 | f7a0842 | #68034 | [fix](binlog) Fix missing DELETE events in row binlog | | 3 | e85575e | #68033 | [fix](binlog) Preserve row binlog compaction policy | | 4 | 21160d7 | #68057 | [fix](binlog) Decouple row binlog compaction from CCR binlog config | | 5 | 04aa5a1 | #68094 | [fix](binlog) Persist row binlog config updates in cloud mode | | 6 | da8feed | #68087 | [fix](binlog) Report streams with missing base tables as stale | Not included on purpose: - The 20 labelled PRs that already carry `incremental-computation-picked` (#62606 in the fork point, #67508 via #67712, the nine of #67830, the six of #68017, the two of #68073, and #68050 whose content this branch got directly through #68012). - #68012 carries the label but is a PR against this branch itself (merged as `6f7c87fa892`); nothing to pick. - #67820 is still open on master; this branch already carries its content via #67861. ### Prerequisite check For every pick I listed the master commits between the fork point (`efedf10c7e3`) and the pick that touch the same files and are not on this branch, and checked whether the pick's behavior depends on them. - **#67783** declares no related PR. It builds on the MOW time-travel rewrite (`BindRelation.buildMowTimeTravelUnion`, #67480) which is already here. Two unlabelled master commits overlap: #66770 (authorization plugin SPI: reworks the data-mask / row-filter API of `LogicalCheckPolicy` and its test) and #67811 (removes the `isPlayNereidsDump()` check in `CheckPolicy`). Neither is a functional prerequisite: the fix consists of `getPolicyTable()` unwrapping `OlapTableWrapper`, `CheckPolicy` collecting the whole filter chain below the policy mask, and `BindRelation` putting a `LogicalCheckPolicy` above each union branch — none of that uses the #66770 API. Not picked; adapted instead (see below). - **#68034**, **#68033**, **#68087** (#68087 relates to #67173, which is before the fork point): no unpicked master commit touches any of their files; the picks applied cleanly and are byte-identical to master. - **#68057**: only `be/test/cloud/cloud_compaction_test.cpp` overlaps with the unrelated #67972 (refresh tablet meta of continuously ingested tablets); auto-merged, the pick only adds new `TEST_F` blocks. Main-code hunks are byte-identical to master. - **#68094**: overlaps with #68090 / #67972 / #66598 (`cloud_tablet.cpp`), #67295 / #67618 / #68090 (`base_tablet.{h,cpp}`), #66598 / #67637 (`meta_service.cpp`), #66598 (`cloud.proto`, `cloud_tablet_test.cpp`) and #67761 / #66598 / #67637 (`meta_service_test.cpp`). All auto-merged. The fix — `BaseTablet::binlog_config()` under the meta lock, `BinlogConfig::operator==`, `CloudTablet::sync_meta()` refreshing the binlog config, `update_tablet` in the meta service accepting `binlog_config`, and `CloudSchemaChangeHandler` routing ROW-binlog property updates through it — does not use anything those commits add. Its hunks are byte-identical to master except one trailing context line in `sync_meta()` (`last_sync_tablet_meta_time_s` comes from #67972). Only #67783 needed adaptation, recorded in its commit message: - `LogicalCheckPolicy.java`: the conflicting context was master's `parsePolicyExpression()` helper (#66770), which does not exist here; the new `getPolicyTable()` is inserted in the same place without it. - `CheckRowPolicyTest.java`: the data-mask mock is written against this branch's per-column `AccessControllerManager.evalDataMaskPolicy(..., column) -> Optional<DataMaskPolicy>` instead of master's `evalDataMaskPolicies(..., Set<String>) -> Map<String, DataMaskSpec>`. The masks are the same (concat for the random-distribution table; the non-movable `k2` mask plus identity masks for the hidden reconstruction columns of the MOW table). The master-only `Or` import (#66770) is not carried; `Collections` / `Locale` imports were added because they arrive with #66770 on master. ### Drift check against master Each pick's `+`/`-` lines are identical to the master commit's, except for the #67783 adaptation above. After the six picks, the touched files still differ from master at `da8feed859d` in: `cloud_tablet.cpp`, `base_tablet.{h,cpp}`, `cloud_compaction_test.cpp`, `cloud_tablet_test.cpp`, `meta_service.cpp`, `meta_service_test.cpp`, `cloud.proto`, `CheckPolicy.java`, `LogicalCheckPolicy.java`, `CheckRowPolicyTest.java`. Replaying the nine unpicked master commits listed above (#67637, #67618, #66770, #66598, #67972, #67811, #67761, #67295, #68090) in a temporary index and removing this branch's own #67861 (`GetTsoRecoveryTransactions` in `cloud.proto` / `meta_service_test.cpp`) brings every file to zero diff against master, except the two `LogicalCheckPolicy.java` / `CheckRowPolicyTest.java` hunks of #66770 that overlap the adaptation. Nothing else is left over. ### Verification - FE: `run-fe-ut.sh --run` on this branch (regenerates thrift/protobuf, compiles fe-core main + test) with the test classes touched by the picks plus `DeleteFromCommandTest` from #68034's checklist: 5 classes, 58 tests, 0 failures, 0 errors, BUILD SUCCESS — `CheckRowPolicyTest` 9 (the three tests #67783 adds included), `ExplainTableStreamPlanTest` 24, `CloudSchemaChangeHandlerTest` 18 (the tests of #68033 and #68094 included), `DropTableStreamTest` 5, `DeleteFromCommandTest` 2. - FE checkstyle on fe-core: 0 violations. - BE: `-fsyntax-only` with the flags of the Release build (`compile_commands.json`, regenerated `gen_cpp` headers incl. the new `TabletMetaInfoPB.binlog_config`) passes for `cloud/cloud_tablet.cpp`, `cloud/cloud_storage_engine.cpp`, `storage/olap_server.cpp`, `storage/tablet/tablet.cpp`, `storage/tablet/base_tablet.cpp`, and with `-DBE_TEST -fno-access-control` for `test/cloud/cloud_compaction_test.cpp`, `test/cloud/cloud_tablet_test.cpp`, `test/storage/compaction/compaction_task_test.cpp`, `test/storage/tablet/tablet_test.cpp`. - Meta service: the two-line `update_tablet` hunk of #68094 only uses the generated `TabletMetaInfoPB::has_binlog_config()/binlog_config()` and `TabletMetaCloudPB::mutable_binlog_config()` accessors, all present in the regenerated headers. - The two new groovy suites (`row_binlog_p0/test_row_binlog_mow_light_delete`, `time_travel_p0/test_mow_time_travel_row_policy`) parse cleanly (groovy parser check). --------- Co-authored-by: morrySnow <zhangwenxin@selectdb.com> Co-authored-by: Luwei <814383175@qq.com>
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
4 participants
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 #67095 #67109 #67101
Related PR: #65418
Problem Summary:
Table Streams currently retain both a cached
TableIfreference and a copied schema from the base table. These snapshots can become stale in two scenarios. First, if the original base table is dropped and another table is created with the same name, relation collection may lock the replacement table by name while stream binding continues to use the cached object of the dropped table, causing the planner lock target and scan target to refer to different table identities. Second, when the base table schema changes through supported ADD/DROP COLUMN operations, especially light schema changes, the stream continues exposing its creation-time schema through DESCRIBE, SHOW COLUMNS, information_schema, and query column resolution. The stale schema can also remain after an FE metadata reload.This change removes the cached base-table object and resolves internal base tables from the persisted catalog, database, and table IDs on each access. It also removes the copied stream schema and derives the current stream schema dynamically from the visible columns of the base table, followed by the hidden stream sequence and change-type columns. As a result, a dropped and recreated table with the same name is not treated as the original base table, while supported base-table schema changes are reflected by stream metadata and query planning. The added regression case covers schema visibility before and after ADD/DROP COLUMN and verifies stream query output after ADD COLUMN.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)