Implement parser and resolver for UNION and INTERSECT operators #229

Terranlee · 2017-04-06T22:50:11Z

This pull request implements the parser and resolver parts of set operations (UNION and INTERSECT).

It also supports composite set operations, e.g.

SELECT * FROM a
UNION ALL
SELECT * FROM b
UNION ALL
SELECT * FROM c
INTERSECT
SELECT * FROM d;

Note: will add parser and resolver tests, so do not merge yet.

zuyu · 2017-04-07T00:13:59Z

parser/ParseSetOperation.hpp

+   * @brief The possible types of set operations.
+   */
+  enum SetOperationType {
+    kIntersect,


Please change to kIntersect = 0,.

zuyu · 2017-04-07T00:16:46Z

parser/ParseSetOperation.hpp

+      case kSingle:
+        return "Single";
+      default:
+        return "Unknown";


Suggest to change to LOG(FATAL) << "Unknown set operation type.";.

zuyu · 2017-04-07T00:18:06Z

parser/ParseSetOperation.hpp

+
+ private:
+  PtrList<ParseTreeNode> operands_;
+  SetOperationType set_operation_type_;


Please mark as const.

zuyu · 2017-04-07T00:20:04Z

parser/ParseSetOperation.hpp

+   */
+  enum SetOperationType {
+    kIntersect,
+    kSingle,


Question: does kSingle has an extra meaning beyond representing a SELECT query?

zuyu · 2017-04-07T00:23:19Z

parser/ParseStatement.hpp

@@ -479,49 +480,49 @@ class ParseStatementDropTable : public ParseStatement {
 };

 /**
- * @brief The parsed representation of a SELECT statement.
+ * @brief The parsed representation of a UNION/INTERSECT/SELECT statement.


Typo: a to an.

zuyu · 2017-04-07T02:03:01Z

query_optimizer/resolver/Resolver.cpp

+                << "and " << possible_type.getName();
+          }
+        } else {
+            THROW_SQL_ERROR_AT(&parse_set_operations)


Reduce to two-whitespace-indentation.

zuyu · 2017-04-07T02:04:01Z

query_optimizer/resolver/Resolver.cpp

+  // Get the possible output attributes that the attributes of all operands can cast to.
+  std::vector<E::AttributeReferencePtr> possible_attributes;
+  for (std::size_t aid = 0; aid < operation_attributes.size(); ++aid) {
+    E::AttributeReferencePtr possible_attribute = attribute_matrix[0][aid];


Mark const?

zuyu · 2017-04-07T02:08:24Z

query_optimizer/resolver/Resolver.cpp

+        E::AttributeReference::Create(context_->nextExprId(),
+                                      attr->attribute_name(),
+                                      attr->attribute_alias(),
+                                      ""/* relation_name*/,


Change to "" /* relation_name */,

zuyu · 2017-04-07T02:13:50Z

query_optimizer/resolver/Resolver.hpp

+  /**
+   * @brief Resolves a set operation and returns a logical plan.
+   */
+  logical::LogicalPtr resolveSetOperationDispatcher(


How about renaming to resolveSetOperation?

zuyu · 2017-04-07T02:17:04Z

query_optimizer/resolver/Resolver.cpp

+          L::SetOperation::kUnionAll, resolved_operations, output_attributes);
+    default:
+      LOG(FATAL) << "Unknown operation: " << parse_set_operations.toString();
+  }


Add return nullptr; to fix the error in Travis CI.

zuyu · 2017-04-08T06:00:53Z

parser/SqlParser.ypp

@@ -676,7 +686,7 @@ sql_statement:
  | quit_statement {
    $$ = $1;
  }
-  | select_statement {
+  | set_operation_statement {


What if we add set_operation_statement, while still keeping select_statement so that we don't need to modify test files with SELECT-only queries?

That will lead to a conflict -- the parser cannot decide whether a SELECT query should be reduced to a select statement or a set operation statement.

jianqiao · 2017-04-08T17:24:03Z

LGTM! Merging.

zuyu · 2017-04-08T17:50:31Z

@jianqiao We need to wait for squashing commits into one before merge.

zuyu · 2017-04-11T05:15:53Z

parser/SqlParser.ypp

+    $$ = new quickstep::ParseSetOperation(@1.first_line, @1.first_column, quickstep::ParseSetOperation::kSelect);
+    $$->addOperand($1);
+  }
+
 select_query:
  TOKEN_SELECT opt_all_distinct selection from_clause opt_where_clause opt_group_by_clause opt_having_clause


I observed the following warning:

SqlParser.ypp:1243.16-31: warning: unused value: $2 [-Wother] TOKEN_SELECT opt_all_distinct selection from_clause opt_where_clause opt_group_by_clause opt_having_clause ^^^^^^^^^^^^^^^^

This is from the original ypp file, since we do not implement the select query like 'SELECT ALL ...' or 'SELECT DISTINCT ...'

zuyu suggested changes Apr 7, 2017

View reviewed changes

zuyu reviewed Apr 8, 2017

View reviewed changes

Terranlee force-pushed the union-operator-parser branch from 6750cae to ec877c6 Compare April 8, 2017 23:51

Implement parser and resolver for UNION and INTERSECT.

5b7b5cb

Terranlee force-pushed the union-operator-parser branch from ec877c6 to 5b7b5cb Compare April 9, 2017 00:28

asfgit merged commit 5b7b5cb into apache:master Apr 9, 2017

zuyu reviewed Apr 11, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement parser and resolver for UNION and INTERSECT operators #229

Implement parser and resolver for UNION and INTERSECT operators #229

Terranlee commented Apr 6, 2017

zuyu Apr 7, 2017

zuyu Apr 7, 2017

zuyu Apr 7, 2017

zuyu Apr 7, 2017

zuyu Apr 7, 2017

zuyu Apr 7, 2017

zuyu Apr 7, 2017

zuyu Apr 7, 2017

zuyu Apr 7, 2017

zuyu Apr 7, 2017

zuyu Apr 8, 2017 •

edited

jianqiao Apr 8, 2017 •

edited

jianqiao commented Apr 8, 2017

zuyu commented Apr 8, 2017

zuyu Apr 11, 2017

Terranlee Apr 11, 2017

Implement parser and resolver for UNION and INTERSECT operators #229

Implement parser and resolver for UNION and INTERSECT operators #229

Conversation

Terranlee commented Apr 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zuyu Apr 8, 2017 • edited

Choose a reason for hiding this comment

jianqiao Apr 8, 2017 • edited

Choose a reason for hiding this comment

jianqiao commented Apr 8, 2017

zuyu commented Apr 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zuyu Apr 8, 2017 •

edited

jianqiao Apr 8, 2017 •

edited