quickstep-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jign...@apache.org
Subject [12/12] incubator-quickstep git commit: Code review suggestions
Date Tue, 07 Jun 2016 17:25:42 GMT
Code review suggestions

- Made query ID as uint64 in the WorkOrder proto.
- Removed the default value set for query ID in the base WorkOrder
  constructor.


Project: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/commit/eb3e73f7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/tree/eb3e73f7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/diff/eb3e73f7

Branch: refs/heads/master
Commit: eb3e73f7fcbb169f4403b4eceb39182221b43962
Parents: 9f1008a
Author: Harshad Deshmukh <hbdeshmukh@apache.org>
Authored: Mon Jun 6 19:19:19 2016 -0500
Committer: Harshad Deshmukh <hbdeshmukh@apache.org>
Committed: Mon Jun 6 19:19:19 2016 -0500

----------------------------------------------------------------------
 query_execution/tests/Foreman_unittest.cpp             | 2 +-
 query_execution/tests/QueryManager_unittest.cpp        | 2 +-
 query_execution/tests/WorkOrdersContainer_unittest.cpp | 2 +-
 relational_operators/HashJoinOperator.hpp              | 9 ++++++---
 relational_operators/RebuildWorkOrder.hpp              | 3 ++-
 relational_operators/WorkOrder.hpp                     | 2 +-
 relational_operators/WorkOrder.proto                   | 2 +-
 7 files changed, 13 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/eb3e73f7/query_execution/tests/Foreman_unittest.cpp
----------------------------------------------------------------------
diff --git a/query_execution/tests/Foreman_unittest.cpp b/query_execution/tests/Foreman_unittest.cpp
index 47cc641..d2f43a4 100644
--- a/query_execution/tests/Foreman_unittest.cpp
+++ b/query_execution/tests/Foreman_unittest.cpp
@@ -61,7 +61,7 @@ namespace quickstep {
 class MockWorkOrder : public WorkOrder {
  public:
   explicit MockWorkOrder(const int op_index)
-      : op_index_(op_index) {}
+      : WorkOrder(0), op_index_(op_index) {}
 
   void execute() override {
     VLOG(3) << "WorkOrder[" << op_index_ << "] executing.";

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/eb3e73f7/query_execution/tests/QueryManager_unittest.cpp
----------------------------------------------------------------------
diff --git a/query_execution/tests/QueryManager_unittest.cpp b/query_execution/tests/QueryManager_unittest.cpp
index 1b9be48..80876f2 100644
--- a/query_execution/tests/QueryManager_unittest.cpp
+++ b/query_execution/tests/QueryManager_unittest.cpp
@@ -62,7 +62,7 @@ namespace quickstep {
 class MockWorkOrder : public WorkOrder {
  public:
   explicit MockWorkOrder(const int op_index)
-      : op_index_(op_index) {}
+      : WorkOrder(0), op_index_(op_index) {}
 
   void execute() override {
     VLOG(3) << "WorkOrder[" << op_index_ << "] executing.";

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/eb3e73f7/query_execution/tests/WorkOrdersContainer_unittest.cpp
----------------------------------------------------------------------
diff --git a/query_execution/tests/WorkOrdersContainer_unittest.cpp b/query_execution/tests/WorkOrdersContainer_unittest.cpp
index d7db9a6..cf133c4 100644
--- a/query_execution/tests/WorkOrdersContainer_unittest.cpp
+++ b/query_execution/tests/WorkOrdersContainer_unittest.cpp
@@ -30,7 +30,7 @@ namespace quickstep {
 class MockNUMAWorkOrder : public WorkOrder {
  public:
   MockNUMAWorkOrder(const int id, const std::vector<int> &numa_nodes)
-      : id_(id) {
+      : WorkOrder(0), id_(id) {
     for (int numa_node : numa_nodes) {
       preferred_numa_nodes_.push_back(numa_node);
     }

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/eb3e73f7/relational_operators/HashJoinOperator.hpp
----------------------------------------------------------------------
diff --git a/relational_operators/HashJoinOperator.hpp b/relational_operators/HashJoinOperator.hpp
index 37c23b8..825f360 100644
--- a/relational_operators/HashJoinOperator.hpp
+++ b/relational_operators/HashJoinOperator.hpp
@@ -660,14 +660,15 @@ class HashOuterJoinWorkOrder : public WorkOrder {
    * @param join_key_attributes The IDs of equijoin attributes in \c
    *        probe_relation.
    * @param any_join_key_attributes_nullable If any attribute is nullable.
-   * @param hash_table The JoinHashTable to use.
+   * @param lookup_block_id The block id of the probe_relation.
    * @param selection A list of Scalars corresponding to the relation attributes
    *        in \c output_destination. Each Scalar is evaluated for the joined
    *        tuples, and the resulting value is inserted into the join result.
    * @param is_selection_on_build Whether each Scalar in the \p selection vector
    *        is using attributes from the build relation as input. Note that the
    *        length of this vector should equal the length of \p selection.
-   * @param lookup_block_id The block id of the probe_relation.
+   * @param hash_table The JoinHashTable to use.
+   * @param query_id The ID of the query to which this WorkOrder belongs.
    * @param output_destination The InsertDestination to insert the join results.
    * @param storage_manager The StorageManager to use.
    **/
@@ -679,9 +680,11 @@ class HashOuterJoinWorkOrder : public WorkOrder {
                          const std::vector<std::unique_ptr<const Scalar>> &selection,
                          std::vector<bool> &&is_selection_on_build,
                          const JoinHashTable &hash_table,
+                         const std::size_t query_id,
                          InsertDestination *output_destination,
                          StorageManager *storage_manager)
-      : build_relation_(build_relation),
+      : WorkOrder(query_id),
+        build_relation_(build_relation),
         probe_relation_(probe_relation),
         join_key_attributes_(std::move(join_key_attributes)),
         any_join_key_attributes_nullable_(any_join_key_attributes_nullable),

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/eb3e73f7/relational_operators/RebuildWorkOrder.hpp
----------------------------------------------------------------------
diff --git a/relational_operators/RebuildWorkOrder.hpp b/relational_operators/RebuildWorkOrder.hpp
index 2695a62..ae876ba 100644
--- a/relational_operators/RebuildWorkOrder.hpp
+++ b/relational_operators/RebuildWorkOrder.hpp
@@ -64,7 +64,8 @@ class RebuildWorkOrder : public WorkOrder {
                    const client_id scheduler_client_id,
                    const std::size_t query_id,
                    MessageBus *bus)
-      : block_ref_(std::move(block_ref)),
+      : WorkOrder(query_id),
+        block_ref_(std::move(block_ref)),
         input_operator_index_(input_operator_index),
         input_relation_id_(input_relation_id),
         scheduler_client_id_(scheduler_client_id),

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/eb3e73f7/relational_operators/WorkOrder.hpp
----------------------------------------------------------------------
diff --git a/relational_operators/WorkOrder.hpp b/relational_operators/WorkOrder.hpp
index f159cc2..059865d 100644
--- a/relational_operators/WorkOrder.hpp
+++ b/relational_operators/WorkOrder.hpp
@@ -291,7 +291,7 @@ class WorkOrder {
    *
    * @param query_id The ID of the query to which this WorkOrder belongs.
    **/
-  explicit WorkOrder(const std::size_t query_id = 0)
+  explicit WorkOrder(const std::size_t query_id)
       : query_id_(query_id) {}
 
   const std::size_t query_id_;

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/eb3e73f7/relational_operators/WorkOrder.proto
----------------------------------------------------------------------
diff --git a/relational_operators/WorkOrder.proto b/relational_operators/WorkOrder.proto
index 697c09c..fd731f7 100644
--- a/relational_operators/WorkOrder.proto
+++ b/relational_operators/WorkOrder.proto
@@ -45,7 +45,7 @@ enum WorkOrderType {
 
 message WorkOrder {
   required WorkOrderType work_order_type = 1;
-  required uint32 query_id = 2;
+  required uint64 query_id = 2;
 
   // The convention for extension numbering is that extensions for a particular
   // WorkOrderID should begin from (operator_type + 1) * 16.


Mime
View raw message