[refactor](Descriptor) remove TupleDescriptor's TableRefInfo#62290
[refactor](Descriptor) remove TupleDescriptor's TableRefInfo#62290morrySnow wants to merge 1 commit intoapache:masterfrom
Conversation
|
run buildall |
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
There was a problem hiding this comment.
Found 1 issue that should be fixed before merge.\n\n1. : removing handling from is not behavior-preserving. intentionally seeds from for SQL like (), but still recomputes partitions via . After this change that recomputation always starts from , so the manual partition subset is dropped and the scan can touch partitions outside the user-specified list.\n\nCritical checkpoint conclusions:\n- Goal of current task: not met safely; the refactor removes stored , but it also changes OLAP partition-pruning behavior, and there is no test proving equivalence.\n- Modification size/focus: small, but not behavior-preserving because planner state was removed from a path that still depended on it.\n- Concurrency: no new concurrency concerns in the changed code.\n- Lifecycle/static initialization: not involved.\n- Configuration: no config changes.\n- Compatibility: no FE/BE protocol or storage-format incompatibility identified.\n- Parallel code paths: not all relevant paths were updated; the Nereids explicit-partition/lazy range-evaluation path still needs the preserved partition subset.\n- Special conditional checks: the removed partition-name restriction was semantically necessary and now has no replacement.\n- Test coverage: missing targeted coverage for explicit queries on this path.\n- Observability: no additional observability needed for this refactor.\n- Transaction/persistence: not involved.\n- Data writes/modifications: not involved.\n- FE/BE variable passing: not involved.\n- Performance: neutral/slightly simpler, but correctness regression outweighs it.\n- Other issues: none beyond the partition-selection regression were confirmed.
| // Step1: compute partition ids | ||
| PartitionNamesInfo partitionNames = desc.getRef().getPartitionNamesInfo(); | ||
| PartitionInfo partitionInfo = olapTable.getPartitionInfo(); | ||
| if (partitionInfo.getType() == PartitionType.RANGE || partitionInfo.getType() == PartitionType.LIST) { |
There was a problem hiding this comment.
This drops the only FE-side restriction that preserves explicitly specified partitions when recomputes partition ids. Nereids still builds with for queries like (), and later calls . After this change, that method always starts from all partitions, so the manual partition subset is lost on this path. Please keep the explicit-partition filter here, or preserve the preselected partition set when recomputing.
There was a problem hiding this comment.
Found 1 issue that should be fixed before merge.
- fe/fe-core/src/main/java/org/apache/doris/planner/OlapScanNode.java: removing explicit partition filtering from computePartitionInfo is not behavior preserving. LogicalOlapScan intentionally seeds selectedPartitionIds from specifiedPartitions for SQL like SELECT * FROM t PARTITION p1, but OlapScanNode.lazyEvaluateRangeLocations still recomputes partitions via computePartitionInfo. After this change that recomputation always starts from all partitions, so the manual partition subset is dropped and the scan can touch partitions outside the user specified list.
Critical checkpoint conclusions:
- Goal of current task: not met safely; the refactor removes stored TableRefInfo, but it also changes OLAP partition pruning behavior, and there is no test proving equivalence.
- Modification size and focus: small, but not behavior preserving because planner state was removed from a path that still depended on it.
- Concurrency: no new concurrency concerns in the changed code.
- Lifecycle or static initialization: not involved.
- Configuration: no config changes.
- Compatibility: no FE or BE protocol or storage format incompatibility identified.
- Parallel code paths: not all relevant paths were updated; the Nereids explicit partition lazy range evaluation path still needs the preserved partition subset.
- Special conditional checks: the removed partition name restriction was semantically necessary and now has no replacement.
- Test coverage: missing targeted coverage for explicit PARTITION queries on this path.
- Observability: no additional observability needed for this refactor.
- Transaction and persistence: not involved.
- Data writes and modifications: not involved.
- FE to BE variable passing: not involved.
- Performance: neutral to slightly simpler, but correctness regression outweighs it.
- Other issues: none beyond the partition selection regression were confirmed.
| // Step1: compute partition ids | ||
| PartitionNamesInfo partitionNames = desc.getRef().getPartitionNamesInfo(); | ||
| PartitionInfo partitionInfo = olapTable.getPartitionInfo(); | ||
| if (partitionInfo.getType() == PartitionType.RANGE || partitionInfo.getType() == PartitionType.LIST) { |
There was a problem hiding this comment.
This drops the only FE side restriction that preserves explicitly specified partitions when OlapScanNode recomputes partition ids. Nereids still builds LogicalOlapScan with selectedPartitionIds equal to specifiedPartitions for queries like SELECT * FROM t PARTITION p1, and lazyEvaluateRangeLocations later calls computePartitionInfo. After this change, that method always starts from all partitions, so the manual partition subset is lost on this path. Please keep the explicit partition filter here, or preserve the preselected partition set when recomputing.
FE UT Coverage ReportIncrement line coverage |
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)