You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@quickstep.apache.org by ji...@apache.org on 2016/06/07 17:25:42 UTC
[12/12] incubator-quickstep git commit: Code review suggestions
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 <hb...@apache.org>
Authored: Mon Jun 6 19:19:19 2016 -0500
Committer: Harshad Deshmukh <hb...@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.