You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2024/09/26 02:35:08 UTC
(doris) branch branch-2.1 updated: [improvement](nereids) Simplify ScanNode projection handling by removing redundant conditions (#40801) (#41315)
This is an automated email from the ASF dual-hosted git repository.
morrysnow pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.1 by this push:
new 4deda2fce73 [improvement](nereids) Simplify ScanNode projection handling by removing redundant conditions (#40801) (#41315)
4deda2fce73 is described below
commit 4deda2fce738db0a6c0db0a0f9b430e61a267d41
Author: zy-kkk <zh...@gmail.com>
AuthorDate: Thu Sep 26 10:35:01 2024 +0800
[improvement](nereids) Simplify ScanNode projection handling by removing redundant conditions (#40801) (#41315)
pick from master #40801
This PR simplifies the handling of `ScanNode` projection logic.
Previously, the code included multiple conditional checks to determine
whether a `projectionTuple` should be generated. These conditions have
been removed, and now `projectionTuple `is always generated for
`ScanNode`, ensuring a consistent projection setup. Additionally,
redundant handling of `SlotId` and `SlotRef` has been eliminated, making
the code cleaner and easier to maintain. The behavior for `OlapScanNode`
remains unchanged.
---
.../glue/translator/PhysicalPlanTranslator.java | 49 +++++-----------------
.../jdbc/test_doris_jdbc_catalog.out | 8 ++++
.../jdbc/test_doris_jdbc_catalog.groovy | 4 ++
3 files changed, 23 insertions(+), 38 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
index 7cfeb3dbaff..a3d3a1885f3 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
@@ -1951,21 +1951,10 @@ public class PhysicalPlanTranslator extends DefaultPlanVisitor<PlanFragment, Pla
// slotIdsByOrder is used to ensure the ScanNode's output order is same with current Project
// if we change the output order in translate project, the upper node will receive wrong order
// tuple, since they get the order from project.getOutput() not scan.getOutput()./
- List<SlotId> slotIdsByOrder = Lists.newArrayList();
- if (requiredByProjectSlotIdSet.size() != requiredSlotIdSet.size()
- || new HashSet<>(projectionExprs).size() != projectionExprs.size()
- || projectionExprs.stream().anyMatch(expr -> !(expr instanceof SlotRef))) {
- projectionTuple = generateTupleDesc(slots,
- ((ScanNode) inputPlanNode).getTupleDesc().getTable(), context);
- inputPlanNode.setProjectList(projectionExprs);
- inputPlanNode.setOutputTupleDesc(projectionTuple);
- } else {
- for (int i = 0; i < slots.size(); ++i) {
- context.addExprIdSlotRefPair(slots.get(i).getExprId(),
- (SlotRef) projectionExprs.get(i));
- slotIdsByOrder.add(((SlotRef) projectionExprs.get(i)).getSlotId());
- }
- }
+ projectionTuple = generateTupleDesc(slots,
+ ((ScanNode) inputPlanNode).getTupleDesc().getTable(), context);
+ inputPlanNode.setProjectList(projectionExprs);
+ inputPlanNode.setOutputTupleDesc(projectionTuple);
// TODO: this is a temporary scheme to support two phase read when has project.
// we need to refactor all topn opt into rbo stage.
@@ -1975,20 +1964,16 @@ public class PhysicalPlanTranslator extends DefaultPlanVisitor<PlanFragment, Pla
SlotDescriptor lastSlot = olapScanSlots.get(olapScanSlots.size() - 1);
if (lastSlot.getColumn() != null
&& lastSlot.getColumn().getName().equals(Column.ROWID_COL)) {
- if (projectionTuple != null) {
- injectRowIdColumnSlot(projectionTuple);
- SlotRef slotRef = new SlotRef(lastSlot);
- inputPlanNode.getProjectList().add(slotRef);
- requiredByProjectSlotIdSet.add(lastSlot.getId());
- } else {
- slotIdsByOrder.add(lastSlot.getId());
- }
+ injectRowIdColumnSlot(projectionTuple);
+ SlotRef slotRef = new SlotRef(lastSlot);
+ inputPlanNode.getProjectList().add(slotRef);
+ requiredByProjectSlotIdSet.add(lastSlot.getId());
requiredSlotIdSet.add(lastSlot.getId());
}
((OlapScanNode) inputPlanNode).updateRequiredSlots(context, requiredByProjectSlotIdSet);
}
updateScanSlotsMaterialization((ScanNode) inputPlanNode, requiredSlotIdSet,
- requiredByProjectSlotIdSet, slotIdsByOrder, context);
+ requiredByProjectSlotIdSet, context);
} else {
TupleDescriptor tupleDescriptor = generateTupleDesc(slots, null, context);
inputPlanNode.setProjectList(projectionExprs);
@@ -2434,22 +2419,10 @@ public class PhysicalPlanTranslator extends DefaultPlanVisitor<PlanFragment, Pla
private void updateScanSlotsMaterialization(ScanNode scanNode,
Set<SlotId> requiredSlotIdSet, Set<SlotId> requiredByProjectSlotIdSet,
- List<SlotId> slotIdsByOrder, PlanTranslatorContext context) {
+ PlanTranslatorContext context) {
// TODO: use smallest slot if do not need any slot in upper node
SlotDescriptor smallest = scanNode.getTupleDesc().getSlots().get(0);
- if (CollectionUtils.isNotEmpty(slotIdsByOrder)) {
- // if we eliminate project above scan, we should ensure the slot order of scan's output is same with
- // the projection's output. So, we need to reorder the output slot in scan's tuple.
- Map<SlotId, SlotDescriptor> idToSlotDescMap = scanNode.getTupleDesc().getSlots().stream()
- .filter(s -> requiredSlotIdSet.contains(s.getId()))
- .collect(Collectors.toMap(SlotDescriptor::getId, s -> s));
- scanNode.getTupleDesc().getSlots().clear();
- for (SlotId slotId : slotIdsByOrder) {
- scanNode.getTupleDesc().getSlots().add(idToSlotDescMap.get(slotId));
- }
- } else {
- scanNode.getTupleDesc().getSlots().removeIf(s -> !requiredSlotIdSet.contains(s.getId()));
- }
+ scanNode.getTupleDesc().getSlots().removeIf(s -> !requiredSlotIdSet.contains(s.getId()));
if (scanNode.getTupleDesc().getSlots().isEmpty()) {
scanNode.getTupleDesc().getSlots().add(smallest);
}
diff --git a/regression-test/data/external_table_p0/jdbc/test_doris_jdbc_catalog.out b/regression-test/data/external_table_p0/jdbc/test_doris_jdbc_catalog.out
index 3fec3f8a574..9695f628fee 100644
--- a/regression-test/data/external_table_p0/jdbc/test_doris_jdbc_catalog.out
+++ b/regression-test/data/external_table_p0/jdbc/test_doris_jdbc_catalog.out
@@ -141,6 +141,14 @@ char_col char(85) Yes true \N NONE
varchar_col char(85) Yes true \N NONE
json_col text Yes true \N NONE
+-- !sql --
+\N \N
+a 1
+
+-- !sql --
+\N \N
+1 a
+
-- !sql --
doris_jdbc_catalog
diff --git a/regression-test/suites/external_table_p0/jdbc/test_doris_jdbc_catalog.groovy b/regression-test/suites/external_table_p0/jdbc/test_doris_jdbc_catalog.groovy
index 2051a4ad54d..c4fce17c62c 100644
--- a/regression-test/suites/external_table_p0/jdbc/test_doris_jdbc_catalog.groovy
+++ b/regression-test/suites/external_table_p0/jdbc/test_doris_jdbc_catalog.groovy
@@ -231,6 +231,10 @@ suite("test_doris_jdbc_catalog", "p0,external,doris,external_docker,external_doc
// test query tvf
qt_sql """desc function query("catalog" = "doris_jdbc_catalog", "query" = "select * from regression_test_jdbc_catalog_p0.base");"""
+ order_qt_sql """ select varchar_col,tinyint_col from query("catalog" = "doris_jdbc_catalog", "query" = "select varchar_col,tinyint_col from regression_test_jdbc_catalog_p0.base");"""
+
+ order_qt_sql """ select tinyint_col,varchar_col from query("catalog" = "doris_jdbc_catalog", "query" = "select varchar_col,tinyint_col from regression_test_jdbc_catalog_p0.base");"""
+
//clean
qt_sql """select current_catalog()"""
sql "switch internal"
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org