Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BugFix] skip add the alias into fieldMapping when order by expr not referenced this alias (backport #50546) #50565

Closed
wants to merge 1 commit into from
Closed

[BugFix] skip add the alias into fieldMapping when order by expr not referenced this alias (backport #50546) #50565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 33 additions & 9 deletions fe/fe-core/src/main/java/com/starrocks/sql/optimizer/transformer/QueryTransformer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import com.starrocks.analysis.LimitElement;
import com.starrocks.analysis.OrderByElement;
import com.starrocks.analysis.SlotRef;
import com.starrocks.analysis.TableName;
import com.starrocks.catalog.Type;
import com.starrocks.common.Pair;
import com.starrocks.common.TreeNode;
Expand Down Expand Up @@ -89,21 +90,21 @@ public LogicalPlan plan(SelectRelation queryBlock, ExpressionMapping outer) {
Iterables.concat(queryBlock.getOutputExpr(),
queryBlock.getOrderSourceExpressions(),
queryBlock.getOrderByAnalytic()),
queryBlock.getOutputExprInOrderByScope(),
queryBlock,
outputNames,
builder.getFieldMappings(),
queryBlock.getOrderScope(), true);
true);

} else {
// requires both output and source fields to be visible if there is no aggregation or distinct
// if there is a distinct, we still no need to project the orderSourceExpressions because it must
// in the outputExpression.
builder = projectForOrder(builder,
Iterables.concat(queryBlock.getOutputExpression(), queryBlock.getOrderByAnalytic()),
queryBlock.getOutputExprInOrderByScope(),
queryBlock,
queryBlock.getColumnOutputNames(),
builder.getFieldMappings(),
queryBlock.getOrderScope(), queryBlock.isDistinct());
queryBlock.isDistinct());
}
}

Expand Down Expand Up @@ -146,10 +147,12 @@ private OptExprBuilder planFrom(Relation node, CTETransformerContext cteContext)

private OptExprBuilder projectForOrder(OptExprBuilder subOpt,
Iterable<Expr> outputExpression,
List<Integer> outputExprInOrderByScope,
SelectRelation queryBlock,
List<String> outputNames,
List<ColumnRefOperator> sourceExpression, Scope scope,
List<ColumnRefOperator> sourceExpression,
boolean withAggregation) {
List<Integer> outputExprInOrderByScope = queryBlock.getOutputExprInOrderByScope();
Scope scope = queryBlock.getOrderScope();
ExpressionMapping outputTranslations = new ExpressionMapping(scope);
Map<ColumnRefOperator, ScalarOperator> projections = Maps.newHashMap();

Expand All @@ -167,10 +170,31 @@ private OptExprBuilder projectForOrder(OptExprBuilder subOpt,
projections.put(columnRefOperator, scalarOperator);

if (outputExprInOrderByScope.contains(outputExprIdx)) {
outputTranslations.putWithSymbol(expression,
new SlotRef(null, outputNames.get(outputExprIdx)), columnRefOperator);
outputTranslations.put(expression, columnRefOperator);
TableName resolveTableName = queryBlock.getResolveTableName();
if (expression instanceof SlotRef) {
resolveTableName = queryBlock.getRelation().getResolveTableName();
}
SlotRef alias = new SlotRef(resolveTableName, outputNames.get(outputExprIdx));
// order by expr may reference the alias. We need put the alias into the fieldMappings or the
// expressionToColumns.
// if the alias not be used in the order by expr like:
// select v1 cc, v2 cc from tbl order by v3. We just add the slotRef(cc) to columnRefOperator in the
// expressionToColumns to ensure this projection can output v1 with alias cc, v2 with alias cc and v3.
// We don't need to ensure the uniqueness of the alias in the fieldMappings.
// if the alias used in the order by expr like:
// select v1 v3, v2 cc from (select abs(v1) v1, abs(v2) v2, v3 from t0) t1 order by t1.v3.
// order by expr t1.v3 referenced the v1 with alias v3, we need set the fieldMappings to ensure order by
// expr can be resolved.
if (scope.getRelationFields().resolveFields(alias).size() > 1) {
outputTranslations.getExpressionToColumns()
.put(new SlotRef(resolveTableName, outputNames.get(outputExprIdx)), columnRefOperator);
} else {
outputTranslations.put(alias, columnRefOperator);
}

} else {
outputTranslations.putWithSymbol(expression, expression, columnRefOperator);
outputTranslations.put(expression, columnRefOperator);
}
outputExprIdx++;
}
Expand Down
51 changes: 47 additions & 4 deletions fe/fe-core/src/test/java/com/starrocks/sql/plan/OrderByTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,12 +519,22 @@ void testNotStrictOrderByExpression(String sql, String expectedPlan) throws Exce
assertContains(plan, expectedPlan);
}

@ParameterizedTest
@MethodSource("duplicateAliasSql")
void testDuplicateAlias(String sql, String expectedPlan) throws Exception {
String plan = getFragmentPlan(sql);
assertContains(plan, expectedPlan);
}

private static Stream<Arguments> allOrderBySql() {
return Stream.concat(successToStrictSql(), failToStrictSql());
}

private static Stream<Arguments> successToStrictSql() {
List<Arguments> list = Lists.newArrayList();
list.add(Arguments.of("select *, v1, abs(v1) v1 from t0 order by 1", "order by: <slot 1> 1: v1 ASC"));
list.add(Arguments.of("select distinct *, v1, abs(v1) v1 from t0 order by 1", "order by: <slot 1> 1: v1 ASC"));
list.add(Arguments.of("select distinct abs(v1) v1, * from t0 order by 1", "order by: <slot 4> 4: abs ASC"));
list.add(Arguments.of("select * from t0 order by 1", "order by: <slot 1> 1: v1 ASC"));
list.add(Arguments.of("select abs(v1) v1, * from t0 order by 1", "order by: <slot 4> 4: abs ASC"));
list.add(Arguments.of("select distinct * from t0 order by 1", "order by: <slot 1> 1: v1 ASC"));
Expand All @@ -546,20 +556,53 @@ private static Stream<Arguments> successToStrictSql() {

private static Stream<Arguments> failToStrictSql() {
List<Arguments> list = Lists.newArrayList();
list.add(Arguments.of("select *, v1, abs(v1) v1 from t0 order by 1", "order by: <slot 1> 1: v1 ASC"));
list.add(Arguments.of("select distinct *, v1, abs(v1) v1 from t0 order by 1", "order by: <slot 1> 1: v1 ASC"));
list.add(Arguments.of("select distinct abs(v1) v1, * from t0 order by 1", "order by: <slot 4> 4: abs ASC"));
list.add(Arguments.of("select distinct *, v1, abs(v1) v1 from t0 order by v1", "order by: <slot 1> 1: v1 ASC"));
list.add(Arguments.of("select v1, max(v2) v1 from t0 group by v1 order by abs(v1)", "order by: <slot 5> 5: abs ASC"));
list.add(Arguments.of("select max(v2) v1, v1 from t0 group by v1 order by abs(v1)", "order by: <slot 5> 5: abs ASC"));
list.add(Arguments.of("select v2, max(v2) v2 from t0 group by v2 order by max(v2)", "order by: <slot 4> 4: max ASC"));
list.add(Arguments.of("select max(v2) v2, v2 from t0 group by v2 order by max(v2)", "order by: <slot 4> 4: max ASC"));
list.add(Arguments.of("select upper(v1) v1, *, v1 from t0 order by v1", "order by: <slot 4> 4: upper ASC"));
list.add(Arguments.of("select *, v1, upper(v1) v1 from t0 order by v1", "order by: <slot 1> 1: v1 ASC"));
list.add(Arguments.of("select distinct upper(v1) v1, *, v1 from t0 order by v1", "order by: <slot 1> 1: v1 ASC"));
list.add(Arguments.of("select distinct v1, *, upper(v1) v1 from t0 order by v1", "order by: <slot 1> 1: v1 ASC"));
list.add(Arguments.of("select distinct upper(v1) v1, *, v1 from t0 order by v1", "order by: <slot 4> 4: upper ASC"));
list.add(Arguments.of("select distinct *, v1, upper(v1) v1 from t0 order by v1", "order by: <slot 1> 1: v1 ASC"));
list.add(Arguments.of("select distinct abs(v1) v1, v1 from t0 order by v1", "order by: <slot 4> 4: abs ASC"));

return list.stream();
}
<<<<<<< HEAD
}
=======

private static Stream<Arguments> duplicateAliasSql() {
List<Arguments> list = Lists.newArrayList();
list.add(Arguments.of("select *, v1 cc, v2 cc from t0 order by v1", "order by: <slot 1> 1: v1 ASC"));
list.add(Arguments.of("select v1 cc, abs(v2) cc from t0 order by v1 ", "order by: <slot 1> 1: v1 ASC"));
list.add(Arguments.of("select distinct v1 cc, v2 cc from t0 order by v1", "order by: <slot 1> 1: v1 ASC"));
list.add(Arguments.of("select v1 cc, v2 cc, abs(v3) ccc from t0 order by abs(ccc)", "order by: <slot 5> 5: abs ASC"));
list.add(Arguments.of("select count(v1) cnt, count(v2) cnt from t0 group by v3 order by abs(v3)",
"3:SORT\n" +
" | order by: <slot 6> 6: abs ASC\n" +
" | offset: 0\n" +
" | \n" +
" 2:Project\n" +
" | <slot 4> : 4: count\n" +
" | <slot 5> : 5: count\n" +
" | <slot 6> : abs(3: v3)"));
list.add(Arguments.of("select v1 cc, v2 cc from (select abs(v1) v1, abs(v2) v2, v3 from t0) t1 order by v3",
"2:SORT\n" +
" | order by: <slot 3> 3: v3 ASC\n" +
" | offset: 0\n" +
" | \n" +
" 1:Project\n" +
" | <slot 3> : 3: v3\n" +
" | <slot 4> : abs(1: v1)\n" +
" | <slot 5> : abs(2: v2)"));
list.add(Arguments.of("select v1 v3, v2 cc from (select abs(v1) v1, abs(v2) v2, v3 from t0) t1 order by t1.v3",
"order by: <slot 4> 4: abs ASC"));
list.add(Arguments.of("select v1, v2 cc from (select abs(v1) v1, abs(v2) v2, v3 from t0) t1 order by t1.v3",
"order by: <slot 3> 3: v3 ASC"));
return list.stream();
}
}
>>>>>>> 885c5009be ([BugFix] skip add the alias into fieldMapping when order by expr not referenced this alias (#50546))
Loading