This is an automated email from the ASF dual-hosted git repository.
morrysnow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 18bc354c06 [fix](Nereids) use correct column unique id when read data
from non-base index (#15534)
18bc354c06 is described below
commit 18bc354c06c1505e15dfadfd1ab71f0c2d2d4ff6
Author: Shuo Wang <wangshuo...@gmail.com>
AuthorDate: Wed Jan 4 01:41:25 2023 +0800
[fix](Nereids) use correct column unique id when read data from non-base
index (#15534)
When light schema change is enabled by default, a column in OLAP scan is
retrieved by column unique id instead of the column name. Columns with the same
name would use different unique IDs among materialized indexes.
This PR ensures that the column in the OLAP scan node could use the correct
column unique id.
---
.../org/apache/doris/analysis/SlotDescriptor.java | 16 ++++++-----
.../glue/translator/PhysicalPlanTranslator.java | 19 ++++++++++++-
.../glue/translator/PlanTranslatorContext.java | 17 ++++++++----
.../translator/PhysicalPlanTranslatorTest.java | 2 +-
.../doris/planner/TableFunctionPlanTest.java | 32 +++++++++++-----------
regression-test/data/nereids_syntax_p0/rollup.out | 9 ++++++
.../suites/nereids_syntax_p0/rollup.groovy | 6 ++--
7 files changed, 67 insertions(+), 34 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotDescriptor.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotDescriptor.java
index f4a67fe167..e4682a4586 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotDescriptor.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotDescriptor.java
@@ -325,13 +325,15 @@ public class SlotDescriptor {
}
public String getExplainString(String prefix) {
- StringBuilder builder = new StringBuilder();
- String colStr = (column == null ? "null" : column.getName());
- String typeStr = (type == null ? "null" : type.toString());
- builder.append(prefix).append("SlotDescriptor{")
- .append("id=").append(id).append(",
col=").append(colStr).append(", type=").append(typeStr)
- .append(", nullable=").append(isNullable).append("}");
- return builder.toString();
+ return new StringBuilder()
+ .append(prefix).append("SlotDescriptor{")
+ .append("id=").append(id)
+ .append(", col=").append(column == null ? "null" :
column.getName())
+ .append(", colUniqueId=").append(column == null ? "null" :
column.getUniqueId())
+ .append(", type=").append(type == null ? "null" :
type.toString())
+ .append(", nullable=").append(isNullable)
+ .append("}")
+ .toString();
}
public boolean isScanSlot() {
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 dda2ee547d..dc3643cb01 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
@@ -32,6 +32,7 @@ import org.apache.doris.analysis.TableName;
import org.apache.doris.analysis.TableRef;
import org.apache.doris.analysis.TupleDescriptor;
import org.apache.doris.analysis.TupleId;
+import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.OlapTable;
import org.apache.doris.catalog.Table;
import org.apache.doris.catalog.TableIf;
@@ -140,6 +141,7 @@ import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
+import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@@ -447,6 +449,21 @@ public class PhysicalPlanTranslator extends
DefaultPlanVisitor<PlanFragment, Pla
.build();
OlapTable olapTable = olapScan.getTable();
TupleDescriptor tupleDescriptor = generateTupleDesc(slotList,
olapTable, context);
+
+ // Use column with the same name in selected materialized index meta
for slot desc,
+ // to get the correct col unique id.
+ if (olapScan.getSelectedIndexId() != olapTable.getBaseIndexId()) {
+ Map<String, Column> indexCols =
olapTable.getSchemaByIndexId(olapScan.getSelectedIndexId())
+ .stream()
+ .collect(Collectors.toMap(Column::getName,
Function.identity()));
+ tupleDescriptor.getSlots().forEach(slotDesc -> {
+ Column column = slotDesc.getColumn();
+ if (column != null && indexCols.containsKey(column.getName()))
{
+ slotDesc.setColumn(indexCols.get(column.getName()));
+ }
+ });
+ }
+
tupleDescriptor.setTable(olapTable);
OlapScanNode olapScanNode = new OlapScanNode(context.nextPlanNodeId(),
tupleDescriptor, "OlapScanNode");
@@ -1422,7 +1439,7 @@ public class PhysicalPlanTranslator extends
DefaultPlanVisitor<PlanFragment, Pla
TupleDescriptor tupleDescriptor = context.generateTupleDesc();
tupleDescriptor.setTable(table);
for (Slot slot : slotList) {
- context.createSlotDesc(tupleDescriptor, (SlotReference) slot);
+ context.createSlotDesc(tupleDescriptor, (SlotReference) slot,
table);
}
return tupleDescriptor;
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PlanTranslatorContext.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PlanTranslatorContext.java
index 13fadc09a1..ab5a181646 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PlanTranslatorContext.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PlanTranslatorContext.java
@@ -25,6 +25,7 @@ import org.apache.doris.analysis.TupleDescriptor;
import org.apache.doris.analysis.TupleId;
import org.apache.doris.analysis.VirtualSlotRef;
import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.TableIf;
import org.apache.doris.common.IdGenerator;
import org.apache.doris.nereids.CascadesContext;
import org.apache.doris.nereids.trees.expressions.ExprId;
@@ -46,6 +47,7 @@ import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;
+import javax.annotation.Nullable;
/**
* Context of physical plan.
@@ -147,14 +149,15 @@ public class PlanTranslatorContext {
}
/**
- * Create SlotDesc and add it to the mappings from expression to the
stales epxr
+ * Create SlotDesc and add it to the mappings from expression to the
stales expr.
*/
- public SlotDescriptor createSlotDesc(TupleDescriptor tupleDesc,
SlotReference slotReference) {
+ public SlotDescriptor createSlotDesc(TupleDescriptor tupleDesc,
SlotReference slotReference,
+ @Nullable TableIf table) {
SlotDescriptor slotDescriptor = this.addSlotDesc(tupleDesc);
- Optional<Column> column = slotReference.getColumn();
// Only the SlotDesc that in the tuple generated for scan node would
have corresponding column.
- if (column.isPresent()) {
- slotDescriptor.setColumn(column.get());
+ if (table != null) {
+ Optional<Column> column = slotReference.getColumn();
+ column.ifPresent(slotDescriptor::setColumn);
}
slotDescriptor.setType(slotReference.getDataType().toCatalogDataType());
slotDescriptor.setIsMaterialized(true);
@@ -174,6 +177,10 @@ public class PlanTranslatorContext {
return slotDescriptor;
}
+ public SlotDescriptor createSlotDesc(TupleDescriptor tupleDesc,
SlotReference slotReference) {
+ return createSlotDesc(tupleDesc, slotReference, null);
+ }
+
public List<TupleDescriptor> getTupleDesc(PlanNode planNode) {
if (planNode.getOutputTupleDesc() != null) {
return Lists.newArrayList(planNode.getOutputTupleDesc());
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslatorTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslatorTest.java
index a836b92b6a..2da342ff26 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslatorTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslatorTest.java
@@ -63,7 +63,7 @@ public class PhysicalPlanTranslatorTest {
t1Output.add(col2);
t1Output.add(col3);
LogicalProperties t1Properties = new LogicalProperties(() -> t1Output);
- PhysicalOlapScan scan = new
PhysicalOlapScan(RelationUtil.newRelationId(), t1, qualifier, 0L,
+ PhysicalOlapScan scan = new
PhysicalOlapScan(RelationUtil.newRelationId(), t1, qualifier,
t1.getBaseIndexId(),
Collections.emptyList(), Collections.emptyList(), null,
PreAggStatus.on(),
Optional.empty(), t1Properties);
Literal t1FilterRight = new IntegerLiteral(1);
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/planner/TableFunctionPlanTest.java
b/fe/fe-core/src/test/java/org/apache/doris/planner/TableFunctionPlanTest.java
index b29778a076..580bf4cc0a 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/planner/TableFunctionPlanTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/planner/TableFunctionPlanTest.java
@@ -79,7 +79,7 @@ public class TableFunctionPlanTest {
explainString.contains("table function:
explode_split(`default_cluster:db1`.`tbl1`.`k2`, ',')"));
Assert.assertTrue(explainString.contains("tuple ids: 0 1"));
Assert.assertTrue(explainString.contains("TupleDescriptor{id=1,
tbl=tmp, byteSize=32}"));
- Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=e1,
type=VARCHAR(*)"));
+ Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=e1,
colUniqueId=-1, type=VARCHAR(*)"));
}
/* Case2 without output explode column
@@ -95,7 +95,7 @@ public class TableFunctionPlanTest {
explainString.contains("table function:
explode_split(`default_cluster:db1`.`tbl1`.`k2`, ',')"));
Assert.assertTrue(explainString.contains("tuple ids: 0 1"));
Assert.assertTrue(explainString.contains("TupleDescriptor{id=1,
tbl=tmp, byteSize=32}"));
- Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=e1,
type=VARCHAR(*)"));
+ Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=e1,
colUniqueId=-1, type=VARCHAR(*)"));
}
/* Case3 group by explode column
@@ -116,7 +116,7 @@ public class TableFunctionPlanTest {
explainString.contains("table function:
explode_split(`default_cluster:db1`.`tbl1`.`k2`, ',')"));
Assert.assertTrue(explainString.contains("tuple ids: 0 1"));
Assert.assertTrue(explainString.contains("TupleDescriptor{id=1,
tbl=tmp, byteSize=32}"));
- Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=e1,
type=VARCHAR(*)"));
+ Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=e1,
colUniqueId=-1, type=VARCHAR(*)"));
// group by tuple
Assert.assertTrue(explainString.contains("TupleDescriptor{id=2,
tbl=null, byteSize=32}"));
}
@@ -135,7 +135,7 @@ public class TableFunctionPlanTest {
Assert.assertTrue(explainString.contains("PREDICATES: `e1` = '1'"));
Assert.assertTrue(explainString.contains("tuple ids: 0 1"));
Assert.assertTrue(explainString.contains("TupleDescriptor{id=1,
tbl=tmp, byteSize=32}"));
- Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=e1,
type=VARCHAR(*)"));
+ Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=e1,
colUniqueId=-1, type=VARCHAR(*)"));
}
/* Case5 where normal column
@@ -151,7 +151,7 @@ public class TableFunctionPlanTest {
explainString.contains("table function:
explode_split(`default_cluster:db1`.`tbl1`.`k2`, ',')"));
Assert.assertTrue(explainString.contains("tuple ids: 0 1"));
Assert.assertTrue(explainString.contains("TupleDescriptor{id=1,
tbl=tmp, byteSize=32}"));
- Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=e1,
type=VARCHAR(*)"));
+ Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=e1,
colUniqueId=-1, type=VARCHAR(*)"));
Assert.assertTrue(UtFrameUtils.checkPlanResultContainsNode(explainString, 0,
"OlapScanNode"));
Assert.assertTrue(explainString.contains("PREDICATES: `k1` = 1"));
}
@@ -171,10 +171,10 @@ public class TableFunctionPlanTest {
Assert.assertTrue(explainString.contains("lateral view tuple id: 1
2"));
// lateral view 2 tuple
Assert.assertTrue(explainString.contains("TupleDescriptor{id=1,
tbl=tmp2, byteSize=32}"));
- Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=e2,
type=VARCHAR(*)"));
+ Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=e2,
colUniqueId=-1, type=VARCHAR(*)"));
// lateral view 1 tuple
Assert.assertTrue(explainString.contains("TupleDescriptor{id=2,
tbl=tmp1, byteSize=32}"));
- Assert.assertTrue(explainString.contains("SlotDescriptor{id=2, col=e1,
type=VARCHAR(*)"));
+ Assert.assertTrue(explainString.contains("SlotDescriptor{id=2, col=e1,
colUniqueId=-1, type=VARCHAR(*)"));
}
// test explode_split function
@@ -350,8 +350,8 @@ public class TableFunctionPlanTest {
explainString.contains("table function:
explode_split(concat(`a`.`k2`, ',', `a`.`k3`), ',')"));
Assert.assertTrue(explainString.contains("lateral view tuple id: 1"));
Assert.assertTrue(explainString.contains("output slot id: 3"));
- Assert.assertTrue(explainString.contains("SlotDescriptor{id=0, col=k2,
type=VARCHAR(1)"));
- Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=k3,
type=VARCHAR(1)"));
+ Assert.assertTrue(explainString.contains("SlotDescriptor{id=0, col=k2,
colUniqueId=1, type=VARCHAR(1)"));
+ Assert.assertTrue(explainString.contains("SlotDescriptor{id=1, col=k3,
colUniqueId=2, type=VARCHAR(1)"));
}
// lateral view of subquery
@@ -368,7 +368,7 @@ public class TableFunctionPlanTest {
Assert.assertTrue(explainString.contains("lateral view tuple id: 2"));
Assert.assertTrue(explainString.contains("output slot id: 2"));
Assert.assertTrue(explainString.contains("tuple ids: 0 2"));
- Assert.assertTrue(explainString.contains("SlotDescriptor{id=2, col=e1,
type=VARCHAR(*)"));
+ Assert.assertTrue(explainString.contains("SlotDescriptor{id=2, col=e1,
colUniqueId=-1, type=VARCHAR(*)"));
}
/*
@@ -384,7 +384,7 @@ public class TableFunctionPlanTest {
Assert.assertTrue(explainString.contains("lateral view tuple id: 3"));
Assert.assertTrue(explainString.contains("output slot id: 3"));
Assert.assertTrue(explainString.contains("tuple ids: 1 3"));
- Assert.assertTrue(explainString.contains("SlotDescriptor{id=3, col=e1,
type=VARCHAR(*)"));
+ Assert.assertTrue(explainString.contains("SlotDescriptor{id=3, col=e1,
colUniqueId=-1, type=VARCHAR(*)"));
}
/*
@@ -403,19 +403,19 @@ public class TableFunctionPlanTest {
Assert.assertTrue(explainString.contains("tuple ids: 1 3"));
String formatString = explainString.replaceAll(" ", "");
Assert.assertTrue(formatString.contains(
- "SlotDescriptor{id=0,col=k1,type=INT"
+ "SlotDescriptor{id=0,col=k1,colUniqueId=0,type=INT"
));
Assert.assertTrue(formatString.contains(
- "SlotDescriptor{id=1,col=k2,type=VARCHAR(1)"
+ "SlotDescriptor{id=1,col=k2,colUniqueId=1,type=VARCHAR(1)"
));
Assert.assertTrue(formatString.contains(
- "SlotDescriptor{id=2,col=null,type=INT"
+ "SlotDescriptor{id=2,col=null,colUniqueId=null,type=INT"
));
Assert.assertTrue(formatString.contains(
- "SlotDescriptor{id=3,col=null,type=VARCHAR(*)"
+ "SlotDescriptor{id=3,col=null,colUniqueId=null,type=VARCHAR(*)"
));
Assert.assertTrue(formatString.contains(
- "SlotDescriptor{id=6,col=e1,type=VARCHAR(*)"
+ "SlotDescriptor{id=6,col=e1,colUniqueId=-1,type=VARCHAR(*)"
));
}
diff --git a/regression-test/data/nereids_syntax_p0/rollup.out
b/regression-test/data/nereids_syntax_p0/rollup.out
new file mode 100644
index 0000000000..71d96f5919
--- /dev/null
+++ b/regression-test/data/nereids_syntax_p0/rollup.out
@@ -0,0 +1,9 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !rollup1 --
+2 6
+3 4
+
+-- !rollup2 --
+1 6
+2 4
+
diff --git a/regression-test/suites/nereids_syntax_p0/rollup.groovy
b/regression-test/suites/nereids_syntax_p0/rollup.groovy
index ae86ad5dc4..b3b761d693 100644
--- a/regression-test/suites/nereids_syntax_p0/rollup.groovy
+++ b/regression-test/suites/nereids_syntax_p0/rollup.groovy
@@ -82,11 +82,9 @@ suite("rollup") {
contains("PREAGGREGATION: ON")
}
- // TODO: add these qt tests back when nereids planner could get correct
plan when
- // light schema change is enabled.
- // order_qt_rollup1 "select k2, sum(v1) from rollup_t1 group by k2"
+ order_qt_rollup1 "select k2, sum(v1) from rollup_t1 group by k2"
- // order_qt_rollup2 "select k1, sum(v1) from rollup_t1 group by k1"
+ order_qt_rollup2 "select k1, sum(v1) from rollup_t1 group by k1"
explain {
sql("select k1, max(v1) from rollup_t1 group by k1")
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
