Skip to content
This repository was archived by the owner on Sep 27, 2019. It is now read-only.

Commit 5ed21f1

Browse files
chenboychenboy
committed
Convert aggregate EXPR to TVE, pass all tests
1 parent 55cadb1 commit 5ed21f1

File tree

8 files changed

+34
-34
lines changed

8 files changed

+34
-34
lines changed

‎src/binder/bind_node_visitor.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,7 @@ void BindNodeVisitor::Visit(parser::SelectStatement *node) {
7676
select_element->DeriveSubqueryFlag();
7777

7878
// Recursively deduce expression value type
79-
expression::ExpressionUtil::EvaluateExpression({ExprMap()},
80-
select_element.get());
79+
select_element->DeduceExpressionType();
8180
// Recursively deduce expression name
8281
select_element->DeduceExpressionName();
8382
new_select_list.push_back(std::move(select_element));

‎src/expression/tuple_value_expression.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ void TupleValueExpression::PerformBinding(
3636
const auto &context = binding_contexts[GetTupleId()];
3737
ai_ = context->Find(GetColumnId());
3838
PL_ASSERT(ai_ != nullptr);
39-
LOG_DEBUG("TVE Column ID %u.%u binds to AI %p (%s)", GetTupleId(),
39+
LOG_TRACE("TVE Column ID %u.%u binds to AI %p (%s)", GetTupleId(),
4040
GetColumnId(), ai_, ai_->name.c_str());
4141
}
4242

‎src/include/expression/aggregate_expression.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ class AggregateExpression : public AbstractExpression {
8484
const auto &context = binding_contexts[0];
8585
ai_ = context->Find(value_idx_);
8686
PL_ASSERT(ai_ != nullptr);
87-
LOG_DEBUG("AggregateOutput Column ID %u.%u binds to AI %p (%s)", 0,
87+
LOG_TRACE("AggregateOutput Column ID %u.%u binds to AI %p (%s)", 0,
8888
value_idx_, ai_, ai_->name.c_str());
8989
}
9090

‎src/include/expression/expression_util.h

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -418,18 +418,24 @@ class ExpressionUtil {
418418
*original expression since it may be referenced in other places
419419
*/
420420
static void ConvertToTvExpr(AbstractExpression *expr,
421-
ExprMap &child_expr_map) {
421+
std::vector<ExprMap> child_expr_maps) {
422+
if (expr == nullptr) {
423+
return;
424+
};
422425
for (size_t i = 0; i < expr->GetChildrenSize(); i++) {
423426
auto child_expr = expr->GetModifiableChild(i);
424-
if (child_expr->GetExpressionType() != ExpressionType::VALUE_TUPLE &&
425-
child_expr_map.count(child_expr)) {
426-
// EvaluateExpression({child_expr_map}, child_expr);
427-
expr->SetChild(i,
428-
new TupleValueExpression(child_expr->GetValueType(), 0,
429-
child_expr_map[child_expr]));
430-
} else {
431-
ConvertToTvExpr(child_expr, child_expr_map);
427+
for (size_t tuple_idx = 0; tuple_idx < child_expr_maps.size();
428+
++tuple_idx) {
429+
if (child_expr->GetExpressionType() != ExpressionType::VALUE_TUPLE &&
430+
child_expr_maps[tuple_idx].count(child_expr)) {
431+
// EvaluateExpression({child_expr_map}, child_expr);
432+
expr->SetChild(i, new TupleValueExpression(
433+
child_expr->GetValueType(), tuple_idx,
434+
child_expr_maps[tuple_idx][child_expr]));
435+
break;
436+
}
432437
}
438+
ConvertToTvExpr(expr->GetModifiableChild(i), child_expr_maps);
433439
}
434440
}
435441

‎src/optimizer/plan_generator.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,8 @@ void PlanGenerator::Visit(const PhysicalInnerNLJoin *op) {
186186
expression::ExpressionUtil::JoinAnnotatedExprs(op->join_predicates);
187187
expression::ExpressionUtil::EvaluateExpression(children_expr_map_,
188188
join_predicate.get());
189+
expression::ExpressionUtil::ConvertToTvExpr(join_predicate.get(),
190+
children_expr_map_);
189191

190192
vector<oid_t> left_keys;
191193
vector<oid_t> right_keys;
@@ -225,6 +227,8 @@ void PlanGenerator::Visit(const PhysicalInnerHashJoin *op) {
225227
expression::ExpressionUtil::JoinAnnotatedExprs(op->join_predicates);
226228
expression::ExpressionUtil::EvaluateExpression(children_expr_map_,
227229
join_predicate.get());
230+
expression::ExpressionUtil::ConvertToTvExpr(join_predicate.get(),
231+
children_expr_map_);
228232

229233
vector<unique_ptr<const expression::AbstractExpression>> left_keys;
230234
vector<unique_ptr<const expression::AbstractExpression>> right_keys;
@@ -423,7 +427,7 @@ void PlanGenerator::BuildProjectionPlan() {
423427
// Copy then evaluate the expression and add to target list
424428
auto col_copy = col->Copy();
425429
// TODO(boweic) : integrate the following two functions
426-
expression::ExpressionUtil::ConvertToTvExpr(col_copy, child_expr_map);
430+
expression::ExpressionUtil::ConvertToTvExpr(col_copy, {child_expr_map});
427431
expression::ExpressionUtil::EvaluateExpression(output_expr_maps,
428432
col_copy);
429433
planner::DerivedAttribute attribute{col_copy};
@@ -470,7 +474,6 @@ void PlanGenerator::BuildAggregatePlan(
470474
expression::ExpressionUtil::EvaluateExpression(children_expr_map_, expr);
471475
if (expression::ExpressionUtil::IsAggregateExpression(
472476
expr->GetExpressionType())) {
473-
LOG_DEBUG("Output Column for Agg %lu : %s", idx, expr->GetInfo().c_str());
474477
auto agg_expr = reinterpret_cast<expression::AggregateExpression *>(expr);
475478
auto agg_col = expr->GetModifiableChild(0);
476479
// Maps the aggregate value in th right tuple to the output
@@ -500,12 +503,10 @@ void PlanGenerator::BuildAggregatePlan(
500503
// Generate the Aggregate Plan
501504
unique_ptr<const planner::ProjectInfo> proj_info(
502505
new planner::ProjectInfo(move(tl), move(dml)));
503-
// LOG_DEBUG(
504-
// "Having predicate : %s ",
505-
// having_predicate == nullptr ? "" :
506-
// having_predicate->GetInfo().c_str());
507506
expression::ExpressionUtil::EvaluateExpression({output_expr_map},
508507
having_predicate.get());
508+
expression::ExpressionUtil::ConvertToTvExpr(having_predicate.get(),
509+
{output_expr_map});
509510
unique_ptr<const expression::AbstractExpression> predicate(
510511
having_predicate.release());
511512
// TODO(boweic): Ditto, since the aggregate plan will own the schema, we may

‎src/optimizer/rule_impls.cpp

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,6 @@ void GetToIndexScan::Transform(
199199
}
200200
// Add transformed plan if found
201201
if (index_matched) {
202-
LOG_DEBUG("Index id :%u", index_id);
203202
auto index_scan_op = PhysicalIndexScan::make(
204203
get->get_id, get->table, get->table_alias, get->predicates,
205204
get->is_for_update, index_id, {}, {}, {});
@@ -726,7 +725,7 @@ void PushFilterThroughJoin::Transform(
726725
std::shared_ptr<OperatorExpression> input,
727726
std::vector<std::shared_ptr<OperatorExpression>> &transformed,
728727
UNUSED_ATTRIBUTE OptimizeContext *context) const {
729-
LOG_DEBUG("PushFilterThroughJoin::Transform");
728+
LOG_TRACE("PushFilterThroughJoin::Transform");
730729
auto &memo = context->metadata->memo;
731730
auto join_op_expr = input->Children().at(0);
732731
auto &join_children = join_op_expr->Children();
@@ -818,7 +817,7 @@ void PushFilterThroughAggregation::Transform(
818817
std::shared_ptr<OperatorExpression> input,
819818
std::vector<std::shared_ptr<OperatorExpression>> &transformed,
820819
UNUSED_ATTRIBUTE OptimizeContext *context) const {
821-
LOG_DEBUG("PushFilterThroughAggregation::Transform");
820+
LOG_TRACE("PushFilterThroughAggregation::Transform");
822821
auto aggregation_op =
823822
input->Children().at(0)->Op().As<LogicalAggregateAndGroupBy>();
824823

@@ -979,7 +978,7 @@ void MarkJoinToInnerJoin::Transform(
979978
std::shared_ptr<OperatorExpression> input,
980979
std::vector<std::shared_ptr<OperatorExpression>> &transformed,
981980
UNUSED_ATTRIBUTE OptimizeContext *context) const {
982-
LOG_DEBUG("MarkJoinToInnerJoin::Transform");
981+
LOG_TRACE("MarkJoinToInnerJoin::Transform");
983982
UNUSED_ATTRIBUTE auto mark_join = input->Op().As<LogicalMarkJoin>();
984983
auto &join_children = input->Children();
985984

@@ -1030,7 +1029,7 @@ void SingleJoinToInnerJoin::Transform(
10301029
std::shared_ptr<OperatorExpression> input,
10311030
std::vector<std::shared_ptr<OperatorExpression>> &transformed,
10321031
UNUSED_ATTRIBUTE OptimizeContext *context) const {
1033-
LOG_DEBUG("SingleJoinToInnerJoin::Transform");
1032+
LOG_TRACE("SingleJoinToInnerJoin::Transform");
10341033
UNUSED_ATTRIBUTE auto single_join = input->Op().As<LogicalSingleJoin>();
10351034
auto &join_children = input->Children();
10361035

@@ -1085,7 +1084,7 @@ void PullFilterThroughMarkJoin::Transform(
10851084
std::shared_ptr<OperatorExpression> input,
10861085
std::vector<std::shared_ptr<OperatorExpression>> &transformed,
10871086
UNUSED_ATTRIBUTE OptimizeContext *context) const {
1088-
LOG_DEBUG("PullFilterThroughMarkJoin::Transform");
1087+
LOG_TRACE("PullFilterThroughMarkJoin::Transform");
10891088
UNUSED_ATTRIBUTE auto mark_join = input->Op().As<LogicalMarkJoin>();
10901089
auto &join_children = input->Children();
10911090
auto filter = join_children[1]->Op();
@@ -1146,7 +1145,7 @@ void PullFilterThroughAggregation::Transform(
11461145
std::shared_ptr<OperatorExpression> input,
11471146
std::vector<std::shared_ptr<OperatorExpression>> &transformed,
11481147
UNUSED_ATTRIBUTE OptimizeContext *context) const {
1149-
LOG_DEBUG("PullFilterThroughAggregation::Transform");
1148+
LOG_TRACE("PullFilterThroughAggregation::Transform");
11501149
auto &memo = context->metadata->memo;
11511150
auto &filter_expr = input->Children()[0];
11521151
auto child_group_id =
@@ -1167,14 +1166,9 @@ void PullFilterThroughAggregation::Transform(
11671166
// (outer_relation.a = (expr))
11681167
correlated_predicates.emplace_back(predicate);
11691168
auto &root_expr = predicate.expr;
1170-
LOG_DEBUG("Correlated predicate : %s", root_expr->GetInfo().c_str());
11711169
if (root_expr->GetChild(0)->GetDepth() < root_expr->GetDepth()) {
1172-
LOG_DEBUG("New groupby col : %s",
1173-
root_expr->GetChild(1)->GetInfo().c_str());
11741170
new_groupby_cols.emplace_back(root_expr->GetChild(1)->Copy());
11751171
} else {
1176-
LOG_DEBUG("New groupby col : %s",
1177-
root_expr->GetChild(0)->GetInfo().c_str());
11781172
new_groupby_cols.emplace_back(root_expr->GetChild(0)->Copy());
11791173
}
11801174
}

‎src/planner/project_info.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ void ProjectInfo::PerformRebinding(
121121

122122
const BindingContext *src_context = input_contexts[dm.second.first];
123123
const auto *dest_ai = src_context->Find(src_col_id);
124-
LOG_DEBUG("Direct: Dest col %u is bound to AI %p", dest_col_id, dest_ai);
124+
LOG_TRACE("Direct: Dest col %u is bound to AI %p", dest_col_id, dest_ai);
125125
output_context.BindNew(dest_col_id, dest_ai);
126126
}
127127

@@ -144,7 +144,7 @@ void ProjectInfo::PerformRebinding(
144144
derived_attribute.attribute_info.attribute_id = dest_col_id;
145145

146146
const auto *dest_ai = &derived_attribute.attribute_info;
147-
LOG_DEBUG("Target: Dest col %u is bound to AI %p", dest_col_id, dest_ai);
147+
LOG_TRACE("Target: Dest col %u is bound to AI %p", dest_col_id, dest_ai);
148148
output_context.BindNew(dest_col_id, dest_ai);
149149
}
150150
}

‎test/sql/optimizer_sql_test.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ TEST_F(OptimizerSQLTests, GroupByTest) {
393393
false);
394394
// Test group by with having
395395
TestUtil("SELECT AVG(a), b FROM test GROUP BY b having AVG(a)=3.5",
396-
{"3.5", "22"}, false);
396+
{"3.5", "11", "3.5", "22"}, false);
397397

398398
// Test group by combined with ORDER BY
399399
TestUtil("SELECT b FROM test GROUP BY b ORDER BY b", {"0", "11", "22", "33"},

0 commit comments

Comments
 (0)