rename operators to match constant naming style. Move the enum out of Expr

src/parser/bison_parser.y

@@ -651,8 +651,8 @@ scalar_expr:
 	;
 
 unary_expr:
-		'-' operand { $$ = Expr::makeOpUnary(Expr::UMINUS, $2); }
+		'-' operand { $$ = Expr::makeOpUnary(kOpMinus, $2); }
-	|	NOT operand { $$ = Expr::makeOpUnary(Expr::NOT, $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 LIKE operand		{ $$ = Expr::makeOpBinary($1, kOpLike, $3); }
-	|	operand NOT LIKE operand	{ $$ = Expr::makeOpBinary($1, Expr::NOT_LIKE, $4); }
+	|	operand NOT LIKE operand	{ $$ = Expr::makeOpBinary($1, kOpNotLike, $4); }
 	;
 
 logic_expr:
-		expr AND expr	{ $$ = Expr::makeOpBinary($1, Expr::AND, $3); }
+		expr AND expr	{ $$ = Expr::makeOpBinary($1, kOpAnd, $3); }
-	|	expr OR expr	{ $$ = Expr::makeOpBinary($1, Expr::OR, $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 LESSEQ operand		{ $$ = Expr::makeOpBinary($1, kOpLessEq, $3); }
-	|	operand GREATEREQ operand	{ $$ = Expr::makeOpBinary($1, Expr::GREATER_EQ, $3); }
+	|	operand GREATEREQ operand	{ $$ = Expr::makeOpBinary($1, kOpGreaterEq, $3); }
 	;
 
 function_expr:

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<Expr*>();
     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<Expr*>();
     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<Expr*>* 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) {

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:

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:

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);

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);