From eddd799c26660b7f811ad1cbc78b35adff922183 Mon Sep 17 00:00:00 2001 From: Pedro Flemming Date: Thu, 6 Apr 2017 17:25:47 +0200 Subject: [PATCH] rename operators to match constant naming style. Move the enum out of Expr --- src/parser/bison_parser.y | 24 +++++++-------- src/sql/Expr.cpp | 14 ++++----- src/sql/Expr.h | 63 ++++++++++++++++++++------------------- src/sqlhelper.cpp | 8 ++--- test/select_tests.cpp | 8 ++--- test/tpc_h_tests.cpp | 6 ++-- 6 files changed, 62 insertions(+), 61 deletions(-) diff --git a/src/parser/bison_parser.y b/src/parser/bison_parser.y index 7593212..4ecdb93 100644 --- a/src/parser/bison_parser.y +++ b/src/parser/bison_parser.y @@ -651,8 +651,8 @@ scalar_expr: ; unary_expr: - '-' operand { $$ = Expr::makeOpUnary(Expr::UMINUS, $2); } - | NOT operand { $$ = Expr::makeOpUnary(Expr::NOT, $2); } + '-' operand { $$ = Expr::makeOpUnary(kOpMinus, $2); } + | NOT operand { $$ = Expr::makeOpUnary(kOpNot, $2); } ; binary_expr: @@ -663,20 +663,20 @@ binary_expr: | operand '*' operand { $$ = Expr::makeOpBinary($1, '*', $3); } | operand '%' operand { $$ = Expr::makeOpBinary($1, '%', $3); } | operand '^' operand { $$ = Expr::makeOpBinary($1, '^', $3); } - | operand LIKE operand { $$ = Expr::makeOpBinary($1, Expr::LIKE, $3); } - | operand NOT LIKE operand { $$ = Expr::makeOpBinary($1, Expr::NOT_LIKE, $4); } + | operand LIKE operand { $$ = Expr::makeOpBinary($1, kOpLike, $3); } + | operand NOT LIKE operand { $$ = Expr::makeOpBinary($1, kOpNotLike, $4); } ; logic_expr: - expr AND expr { $$ = Expr::makeOpBinary($1, Expr::AND, $3); } - | expr OR expr { $$ = Expr::makeOpBinary($1, Expr::OR, $3); } + expr AND expr { $$ = Expr::makeOpBinary($1, kOpAnd, $3); } + | expr OR expr { $$ = Expr::makeOpBinary($1, kOpOr, $3); } ; in_expr: operand IN '(' expr_list ')' { $$ = Expr::makeInOperator($1, $4); } - | operand NOT IN '(' expr_list ')' { $$ = Expr::makeOpUnary(Expr::NOT, Expr::makeInOperator($1, $5)); } + | operand NOT IN '(' expr_list ')' { $$ = Expr::makeOpUnary(kOpNot, Expr::makeInOperator($1, $5)); } | operand IN '(' select_no_paren ')' { $$ = Expr::makeInOperator($1, $4); } - | operand NOT IN '(' select_no_paren ')' { $$ = Expr::makeOpUnary(Expr::NOT, Expr::makeInOperator($1, $5)); } + | operand NOT IN '(' select_no_paren ')' { $$ = Expr::makeOpUnary(kOpNot, Expr::makeInOperator($1, $5)); } ; // TODO: allow no else specified @@ -686,16 +686,16 @@ case_expr: exists_expr: EXISTS '(' select_no_paren ')' { $$ = Expr::makeExists($3); } - | NOT EXISTS '(' select_no_paren ')' { $$ = Expr::makeOpUnary(Expr::NOT, Expr::makeExists($4)); } + | NOT EXISTS '(' select_no_paren ')' { $$ = Expr::makeOpUnary(kOpNot, Expr::makeExists($4)); } ; comp_expr: operand '=' operand { $$ = Expr::makeOpBinary($1, '=', $3); } - | operand NOTEQUALS operand { $$ = Expr::makeOpBinary($1, Expr::NOT_EQUALS, $3); } + | operand NOTEQUALS operand { $$ = Expr::makeOpBinary($1, kOpNotEquals, $3); } | operand '<' operand { $$ = Expr::makeOpBinary($1, '<', $3); } | operand '>' operand { $$ = Expr::makeOpBinary($1, '>', $3); } - | operand LESSEQ operand { $$ = Expr::makeOpBinary($1, Expr::LESS_EQ, $3); } - | operand GREATEREQ operand { $$ = Expr::makeOpBinary($1, Expr::GREATER_EQ, $3); } + | operand LESSEQ operand { $$ = Expr::makeOpBinary($1, kOpLessEq, $3); } + | operand GREATEREQ operand { $$ = Expr::makeOpBinary($1, kOpGreaterEq, $3); } ; function_expr: diff --git a/src/sql/Expr.cpp b/src/sql/Expr.cpp index f1ebe9b..40209ab 100644 --- a/src/sql/Expr.cpp +++ b/src/sql/Expr.cpp @@ -51,7 +51,7 @@ namespace hsql { Expr* Expr::makeOpBinary(Expr* expr1, char op, Expr* expr2) { Expr* e = new Expr(kExprOperator); - e->opType = SIMPLE_OP; + e->opType = kOpSimple; e->opChar = op; e->expr = expr1; e->expr2 = expr2; @@ -61,7 +61,7 @@ namespace hsql { Expr* Expr::makeBetween(Expr* expr, Expr* left, Expr* right) { Expr* e = new Expr(kExprOperator); e->expr = expr; - e->opType = BETWEEN; + e->opType = kOpBetween; e->exprList = new std::vector(); e->exprList->push_back(left); e->exprList->push_back(right); @@ -71,7 +71,7 @@ namespace hsql { Expr* Expr::makeCase(Expr* expr, Expr* then, Expr* other) { Expr* e = new Expr(kExprOperator); e->expr = expr; - e->opType = CASE; + e->opType = kOpCase; e->exprList = new std::vector(); e->exprList->push_back(then); e->exprList->push_back(other); @@ -132,14 +132,14 @@ namespace hsql { Expr* Expr::makeExists(SelectStatement* select) { Expr* e = new Expr(kExprOperator); - e->opType = EXISTS; + e->opType = kOpExists; e->select = select; return e; } Expr* Expr::makeInOperator(Expr* expr, std::vector* exprList) { Expr* e = new Expr(kExprOperator); - e->opType = IN; + e->opType = kOpIn; e->expr = expr; e->exprList = exprList; @@ -148,7 +148,7 @@ namespace hsql { Expr* Expr::makeInOperator(Expr* expr, SelectStatement* select) { Expr* e = new Expr(kExprOperator); - e->opType = IN; + e->opType = kOpIn; e->expr = expr; e->select = select; @@ -177,7 +177,7 @@ namespace hsql { } bool Expr::isSimpleOp() { - return opType == SIMPLE_OP; + return opType == kOpSimple; } bool Expr::isSimpleOp(char op) { diff --git a/src/sql/Expr.h b/src/sql/Expr.h index 3284802..810895b 100644 --- a/src/sql/Expr.h +++ b/src/sql/Expr.h @@ -24,42 +24,43 @@ namespace hsql { kExprSelect }; + // Operator types. These are important for expressions of type kExprOperator. + // Trivial types are those that can be described by a single character e.g: + // + - * / < > = % + // Non-trivial are: <> <= >= LIKE ISNULL NOT + enum OperatorType { + kOpNone, + + // Ternary operators + kOpBetween, + kOpCase, + + // Binary operators. + // Simple operators are identified by the opChar field (e.g. =, >, <). + kOpSimple, + + kOpNotEquals, + kOpLessEq, + kOpGreaterEq, + kOpLike, + kOpNotLike, + kOpAnd, + kOpOr, + kOpIn, + + // Unary operators. + kOpNot, + kOpMinus, + kOpIsNull, + kOpExists + }; + typedef struct Expr Expr; // Represents SQL expressions (i.e. literals, operators, column_refs). // TODO: When destructing a placeholder expression, we might need to alter the placeholder_list. struct Expr { - // Operator types. These are important for expressions of type kExprOperator. - // Trivial types are those that can be described by a single character e.g: - // + - * / < > = % - // Non-trivial are: <> <= >= LIKE ISNULL NOT - enum OperatorType { - NONE, - - // Ternary operators - BETWEEN, - CASE, - - // Binary operators. - SIMPLE_OP, - NOT_EQUALS, - LESS_EQ, - GREATER_EQ, - LIKE, - NOT_LIKE, - AND, - OR, - IN, - - // Unary operators. - NOT, - UMINUS, - ISNULL, - EXISTS - }; - - - + Expr(ExprType type); // Interesting side-effect: diff --git a/src/sqlhelper.cpp b/src/sqlhelper.cpp index 529cead..36aefc8 100644 --- a/src/sqlhelper.cpp +++ b/src/sqlhelper.cpp @@ -63,16 +63,16 @@ namespace hsql { } switch (expr->opType) { - case Expr::SIMPLE_OP: + case kOpSimple: inprintC(expr->opChar, numIndent); break; - case Expr::AND: + case kOpAnd: inprint("AND", numIndent); break; - case Expr::OR: + case kOpOr: inprint("OR", numIndent); break; - case Expr::NOT: + case kOpNot: inprint("NOT", numIndent); break; default: diff --git a/test/select_tests.cpp b/test/select_tests.cpp index 7a0ddd3..1e034f3 100644 --- a/test/select_tests.cpp +++ b/test/select_tests.cpp @@ -144,7 +144,7 @@ TEST(SelectBetweenTest) { Expr* where = stmt->whereClause; ASSERT_NOTNULL(where); ASSERT(where->isType(kExprOperator)); - ASSERT_EQ(where->opType, Expr::BETWEEN); + ASSERT_EQ(where->opType, kOpBetween); ASSERT_STREQ(where->expr->getName(), "grade"); ASSERT(where->expr->isType(kExprColumnRef)); @@ -169,7 +169,7 @@ TEST(SelectConditionalSelectTest) { Expr* where = stmt->whereClause; ASSERT_NOTNULL(where); ASSERT(where->isType(kExprOperator)); - ASSERT_EQ(where->opType, Expr::AND); + ASSERT_EQ(where->opType, kOpAnd); // a = (SELECT ...) Expr* cond1 = where->expr; @@ -189,7 +189,7 @@ TEST(SelectConditionalSelectTest) { // EXISTS (SELECT ...) Expr* cond2 = where->expr2; - ASSERT_EQ(cond2->opType, Expr::EXISTS); + ASSERT_EQ(cond2->opType, kOpExists); ASSERT_NOTNULL(cond2->select); SelectStatement* ex_select = cond2->select; @@ -216,7 +216,7 @@ TEST(SelectCaseWhen) { Expr* caseExpr = func->exprList->at(0); ASSERT_NOTNULL(caseExpr); ASSERT(caseExpr->isType(kExprOperator)); - ASSERT_EQ(caseExpr->opType, Expr::CASE); + ASSERT_EQ(caseExpr->opType, kOpCase); ASSERT(caseExpr->expr->isType(kExprOperator)); ASSERT(caseExpr->expr->isSimpleOp('=')); ASSERT_EQ(caseExpr->exprList->size(), 2); diff --git a/test/tpc_h_tests.cpp b/test/tpc_h_tests.cpp index 9da1b13..6b3387a 100644 --- a/test/tpc_h_tests.cpp +++ b/test/tpc_h_tests.cpp @@ -69,18 +69,18 @@ TEST(TPCHQueryDetailTest) { Expr* where = select20->whereClause; ASSERT_NOTNULL(where); ASSERT(where->isType(kExprOperator)); - ASSERT_EQ(where->opType, Expr::AND); + ASSERT_EQ(where->opType, kOpAnd); Expr* andExpr2 = where->expr; ASSERT_NOTNULL(andExpr2); ASSERT(andExpr2->isType(kExprOperator)); - ASSERT_EQ(andExpr2->opType, Expr::AND); + ASSERT_EQ(andExpr2->opType, kOpAnd); // Test IN expression. Expr* inExpr = andExpr2->expr; ASSERT_NOTNULL(inExpr); ASSERT(inExpr->isType(kExprOperator)); - ASSERT_EQ(inExpr->opType, Expr::IN); + ASSERT_EQ(inExpr->opType, kOpIn); ASSERT_STREQ(inExpr->expr->getName(), "S_SUPPKEY"); ASSERT_NOTNULL(inExpr->select);