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

Commit f4d4e8f

Browse files
committed
Refactoring + full tree rebuilding (at least 2 levels work)
What still doesn't work/don't care about yet/not done - proper memory management (terrier uses shared_ptr anyways) - other 1-level rewrites, multi-layer rewrites, other expr rewrites - how can we define a grammar to programmatically create these rewrites? (the one we have is way too static...) - in relation to logical equivalence: (1) how do we generate logically equivalent expressions: - multi-pass using generating rules (similar to ApplyRule) OR - from Pattern A, generate logically equivalent set of patterns P OR - transform input expression to match certain specification OR - ??? (2) what operators do we support translating? - probably (a AND b) ====> (b AND a) - probably (a OR b) ====> (b OR a) - probably (a = b) ====> (b = a) - maybe more??? (3) do we want multi level translations? - i.e (a AND b) AND c ====> (a AND (b AND c)) - what order do we do these in? May have to modify these operations: - Some assertions in TopDownRewrite/BottomUpRewrite w.r.t to the iterator - Possibly binding.cpp / optimizer_metadata.h / optimizer_task.cpp Issues still pending: - Comparing Values (Matt email/discussion) - r.rule->Check (terrier issue #332)
1 parent 1be43be commit f4d4e8f

File tree

13 files changed

+216
-133
lines changed

13 files changed

+216
-133
lines changed

src/include/optimizer/absexpr_expression.h

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212

1313
// AbstractExpression Definition
1414
#include "expression/abstract_expression.h"
15+
#include "expression/conjunction_expression.h"
16+
#include "expression/comparison_expression.h"
17+
#include "expression/constant_value_expression.h"
1518

1619
#include <memory>
1720
#include <vector>
@@ -112,6 +115,40 @@ class AbsExpr_Container {
112115
return node != nullptr;
113116
}
114117

118+
//(TODO): fix memory management once go to terrier
119+
expression::AbstractExpression *Rebuild(std::vector<expression::AbstractExpression*> children) {
120+
switch (GetType()) {
121+
case ExpressionType::COMPARE_EQUAL:
122+
case ExpressionType::COMPARE_NOTEQUAL:
123+
case ExpressionType::COMPARE_LESSTHAN:
124+
case ExpressionType::COMPARE_GREATERTHAN:
125+
case ExpressionType::COMPARE_LESSTHANOREQUALTO:
126+
case ExpressionType::COMPARE_GREATERTHANOREQUALTO:
127+
case ExpressionType::COMPARE_LIKE:
128+
case ExpressionType::COMPARE_NOTLIKE:
129+
case ExpressionType::COMPARE_IN:
130+
case ExpressionType::COMPARE_DISTINCT_FROM: {
131+
PELOTON_ASSERT(children.size() == 2);
132+
return new expression::ComparisonExpression(GetType(), children[0], children[1]);
133+
}
134+
case ExpressionType::CONJUNCTION_AND:
135+
case ExpressionType::CONJUNCTION_OR: {
136+
PELOTON_ASSERT(children.size() == 2);
137+
return new expression::ConjunctionExpression(GetType(), children[0], children[1]);
138+
}
139+
case ExpressionType::VALUE_CONSTANT: {
140+
PELOTON_ASSERT(children.size() == 0);
141+
auto cve = static_cast<const expression::ConstantValueExpression*>(node);
142+
return new expression::ConstantValueExpression(cve->GetValue());
143+
}
144+
default: {
145+
int type = static_cast<int>(GetType());
146+
LOG_ERROR("Unimplemented Rebuild() for %d found", type);
147+
return nullptr;
148+
}
149+
}
150+
}
151+
115152
private:
116153
const expression::AbstractExpression *node;
117154
};

src/include/optimizer/binding.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ class GroupBindingIterator : public BindingIterator<Node,OperatorType,OperatorEx
6363
Group<Node,OperatorType,OperatorExpr> *target_group_;
6464
size_t num_group_items_;
6565

66+
// Internal function for HasNext()
67+
bool HasNextBinding();
68+
6669
size_t current_item_index_;
6770
std::unique_ptr<BindingIterator<Node,OperatorType,OperatorExpr>> current_iterator_;
6871
};

src/include/optimizer/memo.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ class Memo {
9191
private:
9292
GroupID AddNewGroup(std::shared_ptr<GroupExpression<Node,OperatorType,OperatorExpr>> gexpr);
9393

94+
// Internal InsertExpression function
95+
GroupExpression<Node,OperatorType,OperatorExpr>* InsertExpr(
96+
std::shared_ptr<GroupExpression<Node,OperatorType,OperatorExpr>> gexpr,
97+
GroupID target_group, bool enforced);
98+
9499
// The group owns the group expressions, not the memo
95100
std::unordered_set<GroupExpression<Node,OperatorType,OperatorExpr>*,
96101
GExprPtrHash<Node,OperatorType,OperatorExpr>,

src/include/optimizer/rewriter.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class Rewriter {
4040
std::shared_ptr<AbsExpr_Expression> ConvertToAbsExpr(const expression::AbstractExpression *expr);
4141

4242
private:
43+
expression::AbstractExpression* RebuildExpression(int root_group);
4344
void ExecuteTaskStack(OptimizerTaskStack<AbsExpr_Container,ExpressionType,AbsExpr_Expression> &task_stack);
4445
void RewriteLoop(int root_group_id);
4546
std::shared_ptr<GroupExpression<AbsExpr_Container,ExpressionType,AbsExpr_Expression>> ConvertTree(const expression::AbstractExpression *expr);

src/include/optimizer/rule_rewrite.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@
2020
namespace peloton {
2121
namespace optimizer {
2222

23+
/* Rules are applied from high to low priority */
24+
enum class RulePriority : int {
25+
HIGH = 3,
26+
MEDIUM = 2,
27+
LOW = 1
28+
};
29+
2330
class ComparatorElimination: public Rule<AbsExpr_Container,ExpressionType,AbsExpr_Expression> {
2431
public:
2532
ComparatorElimination();

src/optimizer/binding.cpp

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ GroupBindingIterator<Node,OperatorType,OperatorExpr>::GroupBindingIterator(
3838
}
3939

4040
template <class Node, class OperatorType, class OperatorExpr>
41-
bool GroupBindingIterator<Node,OperatorType,OperatorExpr>::HasNext() {
42-
//(TODO): refactor this and specialization to reduce duplicated code
41+
bool GroupBindingIterator<Node,OperatorType,OperatorExpr>::HasNextBinding() {
4342
if (current_iterator_) {
4443
// Check if still have bindings in current item
4544
if (!current_iterator_->HasNext()) {
@@ -65,10 +64,14 @@ bool GroupBindingIterator<Node,OperatorType,OperatorExpr>::HasNext() {
6564
}
6665
}
6766

68-
std::cout << "Is there a group bind: " << (current_iterator_ != nullptr) << "\n";
6967
return current_iterator_ != nullptr;
7068
}
7169

70+
template <class Node, class OperatorType, class OperatorExpr>
71+
bool GroupBindingIterator<Node,OperatorType,OperatorExpr>::HasNext() {
72+
return HasNextBinding();
73+
}
74+
7275
// Specialization
7376
template <>
7477
bool GroupBindingIterator<Operator,OpType,OperatorExpression>::HasNext() {
@@ -78,37 +81,11 @@ bool GroupBindingIterator<Operator,OpType,OperatorExpression>::HasNext() {
7881
return current_item_index_ == 0;
7982
}
8083

81-
if (current_iterator_) {
82-
// Check if still have bindings in current item
83-
if (!current_iterator_->HasNext()) {
84-
current_iterator_.reset(nullptr);
85-
current_item_index_++;
86-
}
87-
}
88-
89-
if (current_iterator_ == nullptr) {
90-
// Keep checking item iterators until we find a match
91-
while (current_item_index_ < num_group_items_) {
92-
current_iterator_.reset(new GroupExprBindingIterator<Operator,OpType,OperatorExpression>(
93-
this->memo_,
94-
target_group_->GetLogicalExpressions()[current_item_index_].get(),
95-
pattern_));
96-
97-
if (current_iterator_->HasNext()) {
98-
break;
99-
}
100-
101-
current_iterator_.reset(nullptr);
102-
current_item_index_++;
103-
}
104-
}
105-
106-
return current_iterator_ != nullptr;
84+
return HasNextBinding();
10785
}
10886

10987
template <class Node, class OperatorType, class OperatorExpr>
11088
std::shared_ptr<OperatorExpr> GroupBindingIterator<Node,OperatorType,OperatorExpr>::Next() {
111-
std::cout << "Fetching next iterator\n";
11289
return current_iterator_->Next();
11390
}
11491

src/optimizer/group.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,12 @@ void Group<Node, OperatorType, OperatorExpr>::AddExpression(
3333
std::shared_ptr<GroupExpression<Node,OperatorType,OperatorExpr>> expr,
3434
bool enforced) {
3535

36-
//(TODO): rethink how separation works with AbstractExpressions
36+
// Additional assertion checks for AddExpression() with AST rewriting
37+
if (std::is_same<Node, AbsExpr_Container>::value) {
38+
PELOTON_ASSERT(!enforced);
39+
PELOTON_ASSERT(!expr->Op().IsPhysical());
40+
}
41+
3742
// Do duplicate detection
3843
expr->SetGroupID(id_);
3944
if (enforced)

src/optimizer/memo.cpp

Lines changed: 22 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -74,27 +74,13 @@ GroupID Memo<Operator,OpType,OperatorExpression>::AddNewGroup(std::shared_ptr<Gr
7474
// Memo remaining interface functions
7575
//===--------------------------------------------------------------------===//
7676
template <class Node, class OperatorType, class OperatorExpr>
77-
GroupExpression<Node,OperatorType,OperatorExpr> *Memo<Node,OperatorType,OperatorExpr>::InsertExpression(
78-
std::shared_ptr<GroupExpression<Node,OperatorType,OperatorExpr>> gexpr,
79-
bool enforced) {
77+
GroupExpression<Node,OperatorType,OperatorExpr>* Memo<Node,OperatorType,OperatorExpr>::InsertExpr(
78+
std::shared_ptr<GroupExpression<Node,OperatorType,OperatorExpr>> gexpr,
79+
GroupID target_group, bool enforced) {
8080

81-
return InsertExpression(gexpr, UNDEFINED_GROUP, enforced);
82-
}
83-
84-
template <class Node, class OperatorType, class OperatorExpr>
85-
GroupExpression<Node,OperatorType,OperatorExpr> *Memo<Node,OperatorType,OperatorExpr>::InsertExpression(
86-
std::shared_ptr<GroupExpression<Node,OperatorType,OperatorExpr>> gexpr,
87-
GroupID target_group,
88-
bool enforced) {
89-
90-
//(TODO): refactor this with the specialization
9181
auto it = group_expressions_.find(gexpr.get());
92-
std::cout << "group_expressions_.size(): " << group_expressions_.size() << "\n";
93-
std::cout << "InsertExpression (" << gexpr << "," << target_group << ")\n";
94-
9582
if (it != group_expressions_.end()) {
9683
gexpr->SetGroupID((*it)->GetGroupID());
97-
std::cout << "Same Group discovered..\n";
9884
return *it;
9985
} else {
10086
group_expressions_.insert(gexpr.get());
@@ -109,18 +95,34 @@ GroupExpression<Node,OperatorType,OperatorExpr> *Memo<Node,OperatorType,Operator
10995

11096
Group<Node,OperatorType,OperatorExpr> *group = GetGroupByID(group_id);
11197
group->AddExpression(gexpr, enforced);
112-
113-
std::cout << "Inserted into new group...size(): " << group_expressions_.size() << "\n";
11498
return gexpr.get();
11599
}
116100
}
117101

102+
template <class Node, class OperatorType, class OperatorExpr>
103+
GroupExpression<Node,OperatorType,OperatorExpr> *Memo<Node,OperatorType,OperatorExpr>::InsertExpression(
104+
std::shared_ptr<GroupExpression<Node,OperatorType,OperatorExpr>> gexpr,
105+
bool enforced) {
106+
107+
return InsertExpression(gexpr, UNDEFINED_GROUP, enforced);
108+
}
109+
110+
template <class Node, class OperatorType, class OperatorExpr>
111+
GroupExpression<Node,OperatorType,OperatorExpr> *Memo<Node,OperatorType,OperatorExpr>::InsertExpression(
112+
std::shared_ptr<GroupExpression<Node,OperatorType,OperatorExpr>> gexpr,
113+
GroupID target_group,
114+
bool enforced) {
115+
116+
return InsertExpr(gexpr, target_group, enforced);
117+
}
118+
118119
// Specialization for Memo::InsertExpression due to OpType
119120
template <>
120121
GroupExpression<Operator,OpType,OperatorExpression> *Memo<Operator,OpType,OperatorExpression>::InsertExpression(
121122
std::shared_ptr<GroupExpression<Operator,OpType,OperatorExpression>> gexpr,
122123
GroupID target_group,
123124
bool enforced) {
125+
124126
// If leaf, then just return
125127
if (gexpr->Op().GetType() == OpType::Leaf) {
126128
const LeafOperator *leaf = gexpr->Op().As<LeafOperator>();
@@ -130,26 +132,7 @@ GroupExpression<Operator,OpType,OperatorExpression> *Memo<Operator,OpType,Operat
130132
return nullptr;
131133
}
132134

133-
// Lookup in hash table
134-
auto it = group_expressions_.find(gexpr.get());
135-
136-
if (it != group_expressions_.end()) {
137-
gexpr->SetGroupID((*it)->GetGroupID());
138-
return *it;
139-
} else {
140-
group_expressions_.insert(gexpr.get());
141-
// New expression, so try to insert into an existing group or
142-
// create a new group if none specified
143-
GroupID group_id;
144-
if (target_group == UNDEFINED_GROUP) {
145-
group_id = AddNewGroup(gexpr);
146-
} else {
147-
group_id = target_group;
148-
}
149-
Group<Operator,OpType,OperatorExpression> *group = GetGroupByID(group_id);
150-
group->AddExpression(gexpr, enforced);
151-
return gexpr.get();
152-
}
135+
return InsertExpr(gexpr, target_group, enforced);
153136
}
154137

155138
template <class Node, class OperatorType, class OperatorExpr>

src/optimizer/optimizer_task.cpp

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ void TopDownRewrite<Node,OperatorType,OperatorExpr>::execute() {
431431
auto before = iterator.Next();
432432
PELOTON_ASSERT(!iterator.HasNext());
433433

434-
// (TODO): verify correctness
434+
// (TODO): pending terrier issue #332
435435
// Check whether rule actually can be applied
436436
// as opposed to a structural level test
437437
if (!r.rule->Check(before, this->context_.get())) {
@@ -494,27 +494,22 @@ void BottomUpRewrite<Node,OperatorType,OperatorExpr>::execute() {
494494
std::sort(valid_rules.begin(), valid_rules.end(),
495495
std::greater<RuleWithPromise<Node,OperatorType,OperatorExpr>>());
496496

497-
std::cout << "Rule pass starting\n";
498497
for (auto &r : valid_rules) {
499498
GroupExprBindingIterator<Node,OperatorType,OperatorExpr> iterator(this->GetMemo(), cur_group_expr,
500499
r.rule->GetMatchPattern());
501500
if (iterator.HasNext()) {
502501
auto before = iterator.Next();
503502
PELOTON_ASSERT(!iterator.HasNext());
504503

505-
std::cout << "Structural match found\n";
506-
507-
// (TODO): verify correctness
504+
// (TODO): pending terrier issue #332
508505
// Check whether rule actually can be applied
509506
// as opposed to a structural level test
510507
if (!r.rule->Check(before, this->context_.get())) {
511508
continue;
512509
}
513510

514-
std::cout << "Rule integrity check passed\n";
515511
std::vector<std::shared_ptr<OperatorExpr>> after;
516512
r.rule->Transform(before, after, this->context_.get());
517-
std::cout << "Rule Transformation conducted\n";
518513

519514
// Rewrite rule should provide at most 1 expression
520515
PELOTON_ASSERT(after.size() <= 1);
@@ -527,15 +522,11 @@ void BottomUpRewrite<Node,OperatorType,OperatorExpr>::execute() {
527522
this->PushTask(
528523
new BottomUpRewrite<Node,OperatorType,OperatorExpr>(group_id_, this->context_, rule_set_name_, false));
529524

530-
std::cout << "Rewrote expression overwrote!\n";
531-
std::cout << "Rule Pass ended, starting again\n";
532525
return;
533526
}
534527
}
535528
cur_group_expr->SetRuleExplored(r.rule);
536529
}
537-
538-
std::cout << "Rule Pass ended\n";
539530
}
540531

541532

0 commit comments

Comments
 (0)