[fix](mtmv) Add null-safety to getBaseViewsOneLevel for backward compatibility#64412
Conversation
…atibility getBaseViewsOneLevel() could return null when an MTMV was created before the baseViewsOneLevel field was introduced, causing NPE when querying information_schema.view_dependency via Stream.concat(). Key changes: - getBaseViewsOneLevel() now falls back to baseViews when baseViewsOneLevel is null, consistent with getBaseTablesOneLevel() Unit Test: - MTMVRelationTest.testMTMVRelation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
| public Set<BaseTableInfo> getBaseViewsOneLevel() { | ||
| return baseViewsOneLevel; | ||
| // For compatibility, previously created MTMV may not have baseViewsOneLevel | ||
| return baseViewsOneLevel == null ? baseViews : baseViewsOneLevel; |
There was a problem hiding this comment.
add a gsonpostprocessor assign empty set to baseViewsOneLevel if it is null?
There was a problem hiding this comment.
这个文件中前面两个函数都是用一样的写法的。 保持一致的风格 ?
TPC-H: Total hot run time: 29273 ms |
TPC-DS: Total hot run time: 168946 ms |
…w-dependency-npe
Move the null-safety fallback from getBaseViewsOneLevel() to gsonPostProcess(), so the compatibility fix is applied once at deserialization time rather than on every getter call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
run buildall |
Avoid shared reference between baseViews and baseViewsOneLevel by creating a new HashSet copy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fall back to empty set instead of null when baseViews is also null. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
run buildall |
TPC-H: Total hot run time: 29165 ms |
TPC-H: Total hot run time: 28985 ms |
TPC-DS: Total hot run time: 169321 ms |
TPC-DS: Total hot run time: 168542 ms |
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
|
/review |
There was a problem hiding this comment.
I found one blocking test-coverage gap in the compatibility path.
Critical checkpoint conclusions:
- Goal: the PR aims to prevent NPE when old MTMV metadata without baseViewsOneLevel is used by information_schema.view_dependency. The code likely initializes the field during Gson deserialization, but the PR does not add a test that proves that old-metadata path.
- Scope: the change is small and focused on MTMVRelation compatibility.
- Concurrency: no new concurrency or locking behavior is introduced.
- Lifecycle/compatibility: this is a metadata deserialization lifecycle change via GsonPostProcessable; that lifecycle needs direct coverage because freshly created MTMVs do not exercise it.
- Config/storage/protocol compatibility: no config or FE-BE protocol change; metadata backward compatibility is the central concern.
- Parallel paths: MetadataGenerator is the relevant getBaseViewsOneLevel consumer; relation-manager paths use baseViews/baseTablesOneLevelAndFromView and did not reveal a separate new issue.
- Tests: missing targeted coverage, noted inline. The existing MTMVRelationTest creates new metadata where baseViewsOneLevel is already populated.
- Observability, transactionality, performance: not applicable for this narrow change.
Existing review context: I read the existing inline thread about baseViewsOneLevel null handling and did not duplicate it. User focus: no additional user-provided focus points.
| return baseViews; | ||
| } | ||
|
|
||
| @Override |
There was a problem hiding this comment.
Please add a test for this exact compatibility path. The existing MTMVRelationTest creates a fresh MTMV, so baseViewsOneLevel is already populated and that test would have passed before this PR. The reported failure only happens after Gson deserializes old MTMV metadata where the bvol field is absent; a focused test should deserialize such JSON through GsonUtils.GSON and assert getBaseViewsOneLevel() is non-null before MetadataGenerator streams it.
|
PR approved by at least one committer and no changes requested. |
…atibility (#64412) Introduced by #56423, the field baseViewsOneLevel may be null for MTMVs created before that PR. This causes NPE when querying information_schema.view_dependency: Cannot invoke java.util.Set.stream() because the return value of org.apache.doris.mtmv.MTMVRelation.getBaseViewsOneLevel() is null baseViewsOneLevel is currently only used for tablefunction/MetadataGenerator, so this change is safe. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…atibility (#64412) Introduced by #56423, the field baseViewsOneLevel may be null for MTMVs created before that PR. This causes NPE when querying information_schema.view_dependency: Cannot invoke java.util.Set.stream() because the return value of org.apache.doris.mtmv.MTMVRelation.getBaseViewsOneLevel() is null baseViewsOneLevel is currently only used for tablefunction/MetadataGenerator, so this change is safe. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…atibility (#64412) Introduced by #56423, the field baseViewsOneLevel may be null for MTMVs created before that PR. This causes NPE when querying information_schema.view_dependency: Cannot invoke java.util.Set.stream() because the return value of org.apache.doris.mtmv.MTMVRelation.getBaseViewsOneLevel() is null baseViewsOneLevel is currently only used for tablefunction/MetadataGenerator, so this change is safe. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Introduced by #56423, the field baseViewsOneLevel may be null for MTMVs created before that PR. This causes NPE when querying information_schema.view_dependency:
Cannot invoke java.util.Set.stream() because the return value of org.apache.doris.mtmv.MTMVRelation.getBaseViewsOneLevel() is null
baseViewsOneLevel is currently only used for tablefunction/MetadataGenerator, so this change is safe.