Skip to content

Commit dab5eb9

Browse files
KochetovNicolaiKochetovNicolai
committed
Fix INTERPOLATE by constant. Fix other tests.
1 parent 1f1f052 commit dab5eb9

File tree

9 files changed

+49
-15
lines changed

9 files changed

+49
-15
lines changed

‎src/Analyzer/InterpolateNode.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ void InterpolateNode::dumpTreeImpl(WriteBuffer & buffer, FormatState & format_st
2424
{
2525
buffer << std::string(indent, ' ') << "INTERPOLATE id: " << format_state.getNodeId(this);
2626

27-
buffer << '\n' << std::string(indent + 2, ' ') << "EXPRESSION\n";
27+
buffer << '\n' << std::string(indent + 2, ' ') << "EXPRESSION " << expression_name << " \n";
2828
getExpression()->dumpTreeImpl(buffer, format_state, indent + 4);
2929

3030
buffer << '\n' << std::string(indent + 2, ' ') << "INTERPOLATE_EXPRESSION\n";

‎src/Analyzer/InterpolateNode.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ class InterpolateNode final : public IQueryTreeNode
5050
return QueryTreeNodeType::INTERPOLATE;
5151
}
5252

53+
const std::string & getExpressionName() const { return expression_name; }
54+
5355
void dumpTreeImpl(WriteBuffer & buffer, FormatState & format_state, size_t indent) const override;
5456

5557
protected:

‎src/Analyzer/Resolve/QueryAnalyzer.cpp‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@
6464
#include <Analyzer/Resolve/TableExpressionsAliasVisitor.h>
6565
#include <Analyzer/Resolve/ReplaceColumnsVisitor.h>
6666

67+
#include <Planner/PlannerActionsVisitor.h>
68+
6769
#include <Core/Settings.h>
6870

6971
namespace ProfileEvents
@@ -4122,11 +4124,7 @@ void QueryAnalyzer::resolveInterpolateColumnsNodeList(QueryTreeNodePtr & interpo
41224124
{
41234125
auto & interpolate_node_typed = interpolate_node->as<InterpolateNode &>();
41244126

4125-
auto * column_to_interpolate = interpolate_node_typed.getExpression()->as<IdentifierNode>();
4126-
if (!column_to_interpolate)
4127-
throw Exception(ErrorCodes::LOGICAL_ERROR, "INTERPOLATE can work only for indentifiers, but {} is found",
4128-
interpolate_node_typed.getExpression()->formatASTForErrorMessage());
4129-
auto column_to_interpolate_name = column_to_interpolate->getIdentifier().getFullName();
4127+
auto column_to_interpolate_name = interpolate_node_typed.getExpressionName();
41304128

41314129
resolveExpressionNode(interpolate_node_typed.getExpression(), scope, false /*allow_lambda_expression*/, false /*allow_table_expression*/);
41324130

@@ -4135,14 +4133,11 @@ void QueryAnalyzer::resolveInterpolateColumnsNodeList(QueryTreeNodePtr & interpo
41354133
auto & interpolation_to_resolve = interpolate_node_typed.getInterpolateExpression();
41364134
IdentifierResolveScope interpolate_scope(interpolation_to_resolve, &scope /*parent_scope*/);
41374135

4138-
auto fake_column_node = std::make_shared<ColumnNode>(NameAndTypePair(column_to_interpolate_name, interpolate_node_typed.getExpression()->getResultType()), interpolate_node_typed.getExpression());
4136+
auto fake_column_node = std::make_shared<ColumnNode>(NameAndTypePair(column_to_interpolate_name, interpolate_node_typed.getExpression()->getResultType()), interpolate_node);
41394137
if (is_column_constant)
41404138
interpolate_scope.expression_argument_name_to_node.emplace(column_to_interpolate_name, fake_column_node);
41414139

41424140
resolveExpressionNode(interpolation_to_resolve, interpolate_scope, false /*allow_lambda_expression*/, false /*allow_table_expression*/);
4143-
4144-
if (is_column_constant)
4145-
interpolation_to_resolve = interpolation_to_resolve->cloneAndReplace(fake_column_node, interpolate_node_typed.getExpression());
41464141
}
41474142
}
41484143

‎src/Planner/CollectTableExpressionData.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ class CollectSourceColumnsVisitor : public InDepthQueryTreeVisitor<CollectSource
4646
auto column_source_node = column_node->getColumnSource();
4747
auto column_source_node_type = column_source_node->getNodeType();
4848

49-
if (column_source_node_type == QueryTreeNodeType::LAMBDA)
49+
if (column_source_node_type == QueryTreeNodeType::LAMBDA || column_source_node_type == QueryTreeNodeType::INTERPOLATE)
5050
return;
5151

5252
/// JOIN using expression

‎src/Planner/Planner.cpp‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,8 @@ void addWithFillStepIfNeeded(QueryPlan & query_plan,
744744
}
745745
else
746746
{
747+
ActionsDAG rename_dag;
748+
747749
for (auto & interpolate_node : interpolate_list_nodes)
748750
{
749751
auto & interpolate_node_typed = interpolate_node->as<InterpolateNode &>();
@@ -772,8 +774,28 @@ void addWithFillStepIfNeeded(QueryPlan & query_plan,
772774

773775
const auto * alias_node = &interpolate_actions_dag.addAlias(*interpolate_expression, expression_to_interpolate_name);
774776
interpolate_actions_dag.getOutputs().push_back(alias_node);
777+
778+
/// Here we fix INTERPOLATE by constant expression.
779+
/// Example from 02336_sort_optimization_with_fill:
780+
///
781+
/// SELECT 5 AS x, 'Hello' AS s ORDER BY x WITH FILL FROM 1 TO 10 INTERPOLATE (s AS s||'A')
782+
///
783+
/// For this query, INTERPOLATE_EXPRESSION would be : s AS concat(s, 'A'),
784+
/// so that interpolate_actions_dag would have INPUT `s`.
785+
///
786+
/// However, INPUT `s` does not exist. Instead, we have a constant with execution name 'Hello'_String.
787+
/// To fix this, we prepend a rename : 'Hello'_String -> s
788+
if (const auto * constant_node = interpolate_node_typed.getExpression()->as<const ConstantNode>())
789+
{
790+
const auto * node = &rename_dag.addInput(alias_node->result_name, alias_node->result_type);
791+
node = &rename_dag.addAlias(*node, interpolate_node_typed.getExpressionName());
792+
rename_dag.getOutputs().push_back(node);
793+
}
775794
}
776795

796+
if (!rename_dag.getOutputs().empty())
797+
interpolate_actions_dag = ActionsDAG::merge(std::move(rename_dag), std::move(interpolate_actions_dag));
798+
777799
interpolate_actions_dag.removeUnusedActions();
778800
}
779801

‎src/Planner/PlannerExpressionAnalysis.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,9 @@ SortAnalysisResult analyzeSort(const QueryNode & query_node,
462462
for (auto & interpolate_node : interpolate_list_node.getNodes())
463463
{
464464
auto & interpolate_node_typed = interpolate_node->as<InterpolateNode &>();
465+
if (interpolate_node_typed.getExpression()->getNodeType() == QueryTreeNodeType::CONSTANT)
466+
continue;
467+
465468
interpolate_actions_visitor.visit(interpolate_actions_dag, interpolate_node_typed.getInterpolateExpression());
466469
}
467470

‎src/Processors/QueryPlan/FillingStep.cpp‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#include <Processors/Transforms/FillingTransform.h>
33
#include <QueryPipeline/QueryPipelineBuilder.h>
44
#include <IO/Operators.h>
5+
#include <Interpreters/ExpressionActions.h>
56
#include <Common/JSONBuilder.h>
67

78
namespace DB
@@ -58,14 +59,25 @@ void FillingStep::transformPipeline(QueryPipelineBuilder & pipeline, const Build
5859

5960
void FillingStep::describeActions(FormatSettings & settings) const
6061
{
61-
settings.out << String(settings.offset, ' ');
62+
String prefix(settings.offset, settings.indent_char);
63+
settings.out << prefix;
6264
dumpSortDescription(sort_description, settings.out);
6365
settings.out << '\n';
66+
if (interpolate_description)
67+
{
68+
auto expression = std::make_shared<ExpressionActions>(interpolate_description->actions.clone());
69+
expression->describeActions(settings.out, prefix);
70+
}
6471
}
6572

6673
void FillingStep::describeActions(JSONBuilder::JSONMap & map) const
6774
{
6875
map.add("Sort Description", explainSortDescription(sort_description));
76+
if (interpolate_description)
77+
{
78+
auto expression = std::make_shared<ExpressionActions>(interpolate_description->actions.clone());
79+
map.add("Expression", expression->toTree());
80+
}
6981
}
7082

7183
void FillingStep::updateOutputStream()

‎tests/queries/0_stateless/00257_shard_no_aggregates_and_constant_keys.reference‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@
88
40
99
41
1010

11-
0
11+
41
1212
2 42
1313

1414
2 42
1515
43
1616

17-
0
17+
43
1818
11
1919

2020
11

‎tests/queries/0_stateless/03215_analyzer_materialized_constants_bug.sql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ WITH (
2020
SELECT
2121
coalesce(materialize(toLowCardinality(toNullable(1))), 10, NULL),
2222
max(v)
23-
FROM remote('127.0.0.{1,2}', default, test__fuzz_21)
23+
FROM remote('127.0.0.{1,2}', currentDatabase(), test__fuzz_21)
2424
GROUP BY
2525
coalesce(NULL),
2626
coalesce(1, 10, 10, materialize(NULL));

0 commit comments

Comments
 (0)