From 4d1289cdfe95af6ae538723b3832a372d97a6bbb Mon Sep 17 00:00:00 2001 From: Seaven Date: Tue, 27 Feb 2024 11:12:04 +0800 Subject: [PATCH] [BugFix] window lose tuple.computeMemory (backport #41442) (#41673) Signed-off-by: Seaven --- be/src/exec/vectorized/aggregator.cpp | 3 ++ .../starrocks/analysis/SlotDescriptor.java | 10 ++++--- .../sql/plan/PlanFragmentBuilder.java | 1 + .../sql/plan/PlanTestNoneDBBase.java | 4 +++ .../com/starrocks/sql/plan/WindowTest.java | 29 +++++++++++++++++++ .../com/starrocks/utframe/UtFrameUtils.java | 5 ++++ 6 files changed, 48 insertions(+), 4 deletions(-) diff --git a/be/src/exec/vectorized/aggregator.cpp b/be/src/exec/vectorized/aggregator.cpp index 4305797e51637..9126e494be663 100644 --- a/be/src/exec/vectorized/aggregator.cpp +++ b/be/src/exec/vectorized/aggregator.cpp @@ -790,6 +790,9 @@ Status Aggregator::_evaluate_exprs(vectorized::Chunk* chunk) { // TODO: optimized the memory usage _group_by_columns[i] = vectorized::NullableColumn::create( _group_by_columns[i], vectorized::NullColumn::create(_group_by_columns[i]->size(), 0)); + } else if (!_group_by_types[i].is_nullable && _group_by_columns[i]->is_nullable()) { + return Status::InternalError(fmt::format("error nullablel column, index: {}, slot: {}", i, + _group_by_expr_ctxs[i]->root()->debug_string())); } } diff --git a/fe/fe-core/src/main/java/com/starrocks/analysis/SlotDescriptor.java b/fe/fe-core/src/main/java/com/starrocks/analysis/SlotDescriptor.java index ec5ae54c0e47c..d62b133d4df94 100644 --- a/fe/fe-core/src/main/java/com/starrocks/analysis/SlotDescriptor.java +++ b/fe/fe-core/src/main/java/com/starrocks/analysis/SlotDescriptor.java @@ -23,14 +23,10 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; -import com.starrocks.catalog.ArrayType; import com.starrocks.catalog.Column; import com.starrocks.catalog.ColumnStats; -import com.starrocks.catalog.MapType; import com.starrocks.catalog.ScalarType; -import com.starrocks.catalog.StructType; import com.starrocks.catalog.Type; import com.starrocks.sql.analyzer.SemanticException; import com.starrocks.thrift.TSlotDescriptor; @@ -176,6 +172,12 @@ public boolean getIsNullable() { public void setIsNullable(boolean value) { isNullable = value; + // NullIndicatorBit is deprecated in BE, we mock bit to avoid BE crash + if (isNullable) { + setNullIndicatorBit(0); + } else { + setNullIndicatorBit(-1); + } } public void setByteSize(int byteSize) { diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/plan/PlanFragmentBuilder.java b/fe/fe-core/src/main/java/com/starrocks/sql/plan/PlanFragmentBuilder.java index ced980c72f381..6ed077a39eee5 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/plan/PlanFragmentBuilder.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/plan/PlanFragmentBuilder.java @@ -2387,6 +2387,7 @@ public PlanFragment visitPhysicalAnalytic(OptExpression optExpr, ExecPlan contex context.getColRefToExpr() .put(analyticCall.getKey(), new SlotRef(analyticCall.getKey().toString(), slotDesc)); } + outputTupleDesc.computeMemLayout(); List partitionExprs = node.getPartitionExpressions().stream().map(e -> ScalarOperatorToExpr.buildExecExpression(e, diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/PlanTestNoneDBBase.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/PlanTestNoneDBBase.java index af598322f45eb..753c19e816165 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/plan/PlanTestNoneDBBase.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/plan/PlanTestNoneDBBase.java @@ -159,6 +159,10 @@ public String getThriftPlan(String sql) throws Exception { return UtFrameUtils.getPlanThriftString(connectContext, sql); } + public String getDescTbl(String sql) throws Exception { + return UtFrameUtils.getThriftDescTbl(connectContext, sql); + } + public static int getPlanCount(String sql) throws Exception { connectContext.getSessionVariable().setUseNthExecPlan(1); int planCount = UtFrameUtils.getPlanAndFragment(connectContext, sql).second.getPlanCount(); diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/WindowTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/WindowTest.java index 596efea6cb6f5..f70f4e8d73aab 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/plan/WindowTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/plan/WindowTest.java @@ -812,4 +812,33 @@ public void testWindowColumnReuse() throws Exception { "args nullable: true; result nullable: true]\n" + " | cardinality: 1"); } + + @Test + public void testWindowOutputColumnNullCheck() throws Exception { + String sql = "select t1a, t1b, t1c, count(t1d) over (partition by t1d) " + + "from test_all_type_not_null"; + String plan = getVerboseExplain(sql); + assertContains(plan, " 2:ANALYTIC\n" + + " | functions: [, count[([4: t1d, BIGINT, false]); args: BIGINT; result: BIGINT; " + + "args nullable: false; result nullable: false], ]\n" + + " | partition by: [4: t1d, BIGINT, false]\n" + + " | cardinality: 1"); + assertContains(plan, " 3:Project\n" + + " | output columns:\n" + + " | 1 <-> [1: t1a, VARCHAR, false]\n" + + " | 2 <-> [2: t1b, SMALLINT, false]\n" + + " | 3 <-> [3: t1c, INT, false]\n" + + " | 11 <-> [11: count(4: t1d), BIGINT, false]\n" + + " | cardinality: 1"); + + plan = getDescTbl(sql); + assertContains(plan, "TSlotDescriptor(id:11, parent:2, " + + "slotType:TTypeDesc(types:[TTypeNode(type:SCALAR, scalar_type:TScalarType(type:BIGINT))]), " + + "columnPos:-1, byteOffset:0, nullIndicatorByte:0, nullIndicatorBit:-1, " + + "colName:, slotIdx:0, isMaterialized:true)"); + assertContains(plan, "TSlotDescriptor(id:11, parent:4, " + + "slotType:TTypeDesc(types:[TTypeNode(type:SCALAR, scalar_type:TScalarType(type:BIGINT))]), " + + "columnPos:-1, byteOffset:8, nullIndicatorByte:0, nullIndicatorBit:-1, " + + "colName:, slotIdx:2, isMaterialized:true)"); + } } diff --git a/fe/fe-core/src/test/java/com/starrocks/utframe/UtFrameUtils.java b/fe/fe-core/src/test/java/com/starrocks/utframe/UtFrameUtils.java index f10855d5e5147..ae9de9751e100 100644 --- a/fe/fe-core/src/test/java/com/starrocks/utframe/UtFrameUtils.java +++ b/fe/fe-core/src/test/java/com/starrocks/utframe/UtFrameUtils.java @@ -654,6 +654,11 @@ public static String getPlanThriftString(ConnectContext ctx, String queryStr) th return UtFrameUtils.getThriftString(UtFrameUtils.getPlanAndFragment(ctx, queryStr).second.getFragments()); } + public static String getThriftDescTbl(ConnectContext ctx, String queryStr) throws Exception { + Pair pair = UtFrameUtils.getPlanAndFragment(ctx, queryStr); + return pair.second.getDescTbl().toThrift().toString(); + } + // Lock all database before analyze private static void lock(Map dbs) { if (dbs == null) {