You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@quickstep.apache.org by shixuan-fan <gi...@git.apache.org> on 2016/07/01 20:33:49 UTC
[GitHub] incubator-quickstep pull request #49: QUICKSTEP-20: Added ExecutionGenerator...
GitHub user shixuan-fan opened a pull request:
https://github.com/apache/incubator-quickstep/pull/49
QUICKSTEP-20: Added ExecutionGenerator support for Window Aggregation
- Created `WindowAggregationOperationState` and `WindowAggregationOperator`.
- The execution generator could form an execution plan for window aggregation. Window aggregation operator takes the result of sorting as input, and output a relation with one extra attribute which is the result of window aggregation.
- `WindowAggregationOperator::execute()` is a dummy method, it will report that window aggregation is not supported, and return an empty table based on the expected output relation.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-quickstep SQL-window-aggregation
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-quickstep/pull/49.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #49
----
commit c6ed1c3063289f61094366c178cd76c09157ceaf
Author: shixuan-fan <sh...@apache.org>
Date: 2016-06-29T20:25:18Z
Created WindowAggregationOperator
commit 678d94dc306e9a85dc9b32f9d5a8f630a08ce78d
Author: shixuan-fan <sh...@apache.org>
Date: 2016-06-30T15:59:21Z
Created WindowAggregationOperationState
commit 3a4eeb81056b4d7f01a6f8fe2dc815e75794b52c
Author: shixuan-fan <sh...@apache.org>
Date: 2016-06-30T16:33:27Z
Added WindowAggregationOperationState in QueryContext
commit 1a83deecb866e977c9d0e3984e9a41139fa5fb12
Author: shixuan-fan <sh...@apache.org>
Date: 2016-06-30T21:52:39Z
Added convertWindowAggregate() in ExecutionGenerator
commit f9d618a83fca6b97d5756b67d606597bcea9de3a
Author: shixuan-fan <sh...@apache.org>
Date: 2016-07-01T18:50:50Z
Added dummy execute() method, window aggregate function will return an empty table with correct attributes
commit e17b35477277c41ef3e3dd431c47168e92c1eaba
Author: shixuan-fan <sh...@apache.org>
Date: 2016-07-01T19:19:23Z
Minor comment modification
commit 7242806e753e262bc5016c8ff05040f5dbd1c053
Author: shixuan-fan <sh...@apache.org>
Date: 2016-07-01T20:12:13Z
Added unittest
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quickstep pull request #49: QUICKSTEP-20: Added ExecutionGenerator...
Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/49#discussion_r69986183
--- Diff: storage/tests/WindowAggregationOperationState_unittest.cpp ---
@@ -0,0 +1,91 @@
+/**
+ * Copyright 2011-2015 Quickstep Technologies LLC.
+ * Copyright 2015-2016 Pivotal Software, Inc.
+ * Copyright 2016, Quickstep Research Group, Computer Sciences Department,
+ * University of Wisconsin\u2014Madison.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ **/
+
+#include <cstddef>
+#include <memory>
+
+#include "catalog/CatalogDatabase.hpp"
+#include "catalog/CatalogRelation.hpp"
+#include "catalog/CatalogTypedefs.hpp"
+#include "expressions/aggregation/AggregateFunction.pb.h"
+#include "storage/WindowAggregationOperationState.hpp"
+#include "storage/WindowAggregationOperationState.pb.h"
+
+#include "gtest/gtest.h"
+
+using std::unique_ptr;
+
+namespace quickstep {
+
+class WindowAggregationOperationStateProtoTest : public ::testing::Test {
+ protected:
+ static const relation_id kInvalidTableId = 100;
+ static const std::int64_t kInvalidNum = -10;
+ static const std::int64_t kValidNum = 10;
+
+ virtual void SetUp() {
+ database_.reset(new CatalogDatabase(nullptr, "db"));
+ rel_id_ = database_->addRelation(new CatalogRelation(nullptr, "rel"));
+ }
+
+ unique_ptr<CatalogDatabase> database_;
+ relation_id rel_id_;
+};
+
+TEST_F(WindowAggregationOperationStateProtoTest, UninitializationTest) {
+ serialization::WindowAggregationOperationState proto;
+ EXPECT_FALSE(WindowAggregationOperationState::ProtoIsValid(proto, *database_.get()));
+}
+
+TEST_F(WindowAggregationOperationStateProtoTest, InvalidRelationIdTest) {
+ serialization::WindowAggregationOperationState proto;
+ proto.set_relation_id(kInvalidTableId);
+ proto.mutable_function()->set_aggregation_id(serialization::AggregateFunction::AVG);
+ proto.set_is_row(true);
+ proto.set_num_preceding(kValidNum);
+ proto.set_num_following(kValidNum);
+ EXPECT_FALSE(WindowAggregationOperationState::ProtoIsValid(proto, *database_.get()));
+}
+
+TEST_F(WindowAggregationOperationStateProtoTest, InvalidNumTest) {
+ serialization::WindowAggregationOperationState proto;
+ proto.set_relation_id(rel_id_);
+ proto.mutable_function()->set_aggregation_id(serialization::AggregateFunction::AVG);
+ proto.set_is_row(true);
+ proto.set_num_preceding(kInvalidNum);
+ proto.set_num_following(kValidNum);
+ EXPECT_FALSE(WindowAggregationOperationState::ProtoIsValid(proto, *database_.get()));
+
+ proto.set_num_preceding(kValidNum);
+ proto.set_num_following(kInvalidNum);
+ EXPECT_FALSE(WindowAggregationOperationState::ProtoIsValid(proto, *database_.get()));
+}
+
+TEST_F(WindowAggregationOperationStateProtoTest, ValidTest) {
+ serialization::WindowAggregationOperationState proto;
+ proto.set_relation_id(rel_id_);
+ proto.mutable_function()->set_aggregation_id(serialization::AggregateFunction::AVG);
+ proto.set_is_row(true);
+ proto.set_num_preceding(kValidNum);
+ proto.set_num_following(kValidNum);
+ EXPECT_TRUE(WindowAggregationOperationState::ProtoIsValid(proto, *database_.get()));
+}
+
+} // namespace quickstep
+
--- End diff --
Please remove this empty line.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quickstep pull request #49: QUICKSTEP-20: Added ExecutionGenerator...
Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/49#discussion_r69986111
--- Diff: storage/tests/WindowAggregationOperationState_unittest.cpp ---
@@ -0,0 +1,91 @@
+/**
+ * Copyright 2011-2015 Quickstep Technologies LLC.
+ * Copyright 2015-2016 Pivotal Software, Inc.
+ * Copyright 2016, Quickstep Research Group, Computer Sciences Department,
+ * University of Wisconsin\u2014Madison.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ **/
+
+#include <cstddef>
+#include <memory>
+
+#include "catalog/CatalogDatabase.hpp"
+#include "catalog/CatalogRelation.hpp"
+#include "catalog/CatalogTypedefs.hpp"
+#include "expressions/aggregation/AggregateFunction.pb.h"
+#include "storage/WindowAggregationOperationState.hpp"
+#include "storage/WindowAggregationOperationState.pb.h"
+
+#include "gtest/gtest.h"
+
+using std::unique_ptr;
+
+namespace quickstep {
+
+class WindowAggregationOperationStateProtoTest : public ::testing::Test {
+ protected:
+ static const relation_id kInvalidTableId = 100;
+ static const std::int64_t kInvalidNum = -10;
+ static const std::int64_t kValidNum = 10;
--- End diff --
Optionally, like [here](https://github.com/apache/incubator-quickstep/blob/master/relational_operators/tests/SortRunGenerationOperator_unittest.cpp#L80) we could move these three values in an anonymous namespace:
```
namespace {
constexpr relation_id kInvalidTableId = 100;
constexpr std::int64_t kInvalidNum = -10;
constexpr std::int64_t kValidNum = 10;
} // namespace
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quickstep pull request #49: QUICKSTEP-20: Added ExecutionGenerator...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-quickstep/pull/49
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quickstep pull request #49: QUICKSTEP-20: Added ExecutionGenerator...
Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/49#discussion_r69971207
--- Diff: query_optimizer/ExecutionGenerator.cpp ---
@@ -1639,5 +1641,101 @@ void ExecutionGenerator::convertTableGenerator(
temporary_relation_info_vec_.emplace_back(tablegen_index, output_relation);
}
+void ExecutionGenerator::convertWindowAggregate(
+ const P::WindowAggregatePtr &physical_plan) {
+ // Create window_aggregation_operation_state proto.
+ const QueryContext::window_aggregation_state_id window_aggr_state_index =
+ query_context_proto_->window_aggregation_states_size();
+ S::WindowAggregationOperationState *window_aggr_state_proto =
+ query_context_proto_->add_window_aggregation_states();
+
+ // Get input.
+ const CatalogRelationInfo *input_relation_info =
+ findRelationInfoOutputByPhysical(physical_plan->input());
+ window_aggr_state_proto->set_relation_id(input_relation_info->relation->getID());
+
+ // Get window aggregate function expression.
+ const E::AliasPtr &named_window_aggregate_expression =
+ physical_plan->window_aggregate_expression();
+ const E::WindowAggregateFunctionPtr &window_aggregate_function =
+ std::static_pointer_cast<const E::WindowAggregateFunction>(
+ named_window_aggregate_expression->expression());
+
+ // Set the AggregateFunction.
+ window_aggr_state_proto->mutable_function()->CopyFrom(
+ window_aggregate_function->window_aggregate().getProto());
+
+ // Set the arguments.
+ for (const E::ScalarPtr &argument : window_aggregate_function->arguments()) {
+ unique_ptr<const Scalar> concretized_argument(argument->concretize(attribute_substitution_map_));
+ window_aggr_state_proto->add_arguments()->CopyFrom(concretized_argument->getProto());
+ }
+
+ // Set partition keys.
+ const E::WindowInfo window_info = window_aggregate_function->window_info();
+ for (const E::ScalarPtr &partition_by_attribute
+ : window_info.partition_by_attributes) {
+ unique_ptr<const Scalar> concretized_partition_by_attribute(
+ partition_by_attribute->concretize(attribute_substitution_map_));
+ window_aggr_state_proto->add_partition_by_attributes()
+ ->CopyFrom(concretized_partition_by_attribute->getProto());
+ }
+
+ // Set window frame info.
+ if (window_info.frame_info == nullptr) {
+ // If the frame is not specified, use the default setting:
+ // 1. If ORDER BY key is specified, use cumulative aggregation:
+ // ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW.
+ // 2. If ORDER BY key is not specified either, use the whole partition:
+ // ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING.
+ window_aggr_state_proto->set_is_row(true); /* frame mode: ROWS */
+ window_aggr_state_proto->set_num_preceding(-1); /* UNBOUNDED PRECEDING */
+ if (window_info.order_by_attributes.empty()) {
+ window_aggr_state_proto->set_num_following(-1); /* UNBOUNDED FOLLOWING */
+ } else {
+ window_aggr_state_proto->set_num_following(0); /* CURRENT ROW */
--- End diff --
Code style issue:
For above four lines w/ comments, we have two ways:
Either `window_aggr_state_proto->set_num_following(0 /* Current row */);`
Or `window_aggr_state_proto->set_num_following(0); // Current row.`
And I prefer the second approach.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quickstep pull request #49: QUICKSTEP-20: Added ExecutionGenerator...
Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/49#discussion_r69970341
--- Diff: query_optimizer/ExecutionGenerator.cpp ---
@@ -1639,5 +1641,101 @@ void ExecutionGenerator::convertTableGenerator(
temporary_relation_info_vec_.emplace_back(tablegen_index, output_relation);
}
+void ExecutionGenerator::convertWindowAggregate(
+ const P::WindowAggregatePtr &physical_plan) {
+ // Create window_aggregation_operation_state proto.
+ const QueryContext::window_aggregation_state_id window_aggr_state_index =
+ query_context_proto_->window_aggregation_states_size();
+ S::WindowAggregationOperationState *window_aggr_state_proto =
+ query_context_proto_->add_window_aggregation_states();
+
+ // Get input.
+ const CatalogRelationInfo *input_relation_info =
+ findRelationInfoOutputByPhysical(physical_plan->input());
+ window_aggr_state_proto->set_relation_id(input_relation_info->relation->getID());
+
+ // Get window aggregate function expression.
+ const E::AliasPtr &named_window_aggregate_expression =
+ physical_plan->window_aggregate_expression();
+ const E::WindowAggregateFunctionPtr &window_aggregate_function =
+ std::static_pointer_cast<const E::WindowAggregateFunction>(
+ named_window_aggregate_expression->expression());
+
+ // Set the AggregateFunction.
+ window_aggr_state_proto->mutable_function()->CopyFrom(
+ window_aggregate_function->window_aggregate().getProto());
+
+ // Set the arguments.
+ for (const E::ScalarPtr &argument : window_aggregate_function->arguments()) {
+ unique_ptr<const Scalar> concretized_argument(argument->concretize(attribute_substitution_map_));
+ window_aggr_state_proto->add_arguments()->CopyFrom(concretized_argument->getProto());
+ }
+
+ // Set partition keys.
+ const E::WindowInfo window_info = window_aggregate_function->window_info();
+ for (const E::ScalarPtr &partition_by_attribute
+ : window_info.partition_by_attributes) {
+ unique_ptr<const Scalar> concretized_partition_by_attribute(
+ partition_by_attribute->concretize(attribute_substitution_map_));
+ window_aggr_state_proto->add_partition_by_attributes()
+ ->CopyFrom(concretized_partition_by_attribute->getProto());
--- End diff --
Ditto for `MergeFrom`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quickstep pull request #49: QUICKSTEP-20: Added ExecutionGenerator...
Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/49#discussion_r69970703
--- Diff: storage/WindowAggregationOperationState.proto ---
@@ -0,0 +1,33 @@
+// Copyright 2011-2015 Quickstep Technologies LLC.
+// Copyright 2015-2016 Pivotal Software, Inc.
+// Copyright 2016, Quickstep Research Group, Computer Sciences Department,
+// University of Wisconsin\u2014Madison.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+syntax = "proto2";
+
+package quickstep.serialization;
+
+import "expressions/aggregation/AggregateFunction.proto";
+import "expressions/Expressions.proto";
+
+message WindowAggregationOperationState {
+ required int32 relation_id = 1;
+ required AggregateFunction function = 2;
+ repeated Scalar arguments = 3;
+ repeated Scalar partition_by_attributes = 4;
+ required bool is_row = 5;
+ required int64 num_preceding = 6;
+ required int64 num_following = 7;
--- End diff --
And comments for `num_preceding` and `num_following` regarding the usage of `-1`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quickstep pull request #49: QUICKSTEP-20: Added ExecutionGenerator...
Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/49#discussion_r69974697
--- Diff: query_optimizer/ExecutionGenerator.cpp ---
@@ -1639,5 +1641,101 @@ void ExecutionGenerator::convertTableGenerator(
temporary_relation_info_vec_.emplace_back(tablegen_index, output_relation);
}
+void ExecutionGenerator::convertWindowAggregate(
+ const P::WindowAggregatePtr &physical_plan) {
+ // Create window_aggregation_operation_state proto.
+ const QueryContext::window_aggregation_state_id window_aggr_state_index =
+ query_context_proto_->window_aggregation_states_size();
+ S::WindowAggregationOperationState *window_aggr_state_proto =
+ query_context_proto_->add_window_aggregation_states();
+
+ // Get input.
+ const CatalogRelationInfo *input_relation_info =
+ findRelationInfoOutputByPhysical(physical_plan->input());
+ window_aggr_state_proto->set_relation_id(input_relation_info->relation->getID());
+
+ // Get window aggregate function expression.
+ const E::AliasPtr &named_window_aggregate_expression =
+ physical_plan->window_aggregate_expression();
+ const E::WindowAggregateFunctionPtr &window_aggregate_function =
+ std::static_pointer_cast<const E::WindowAggregateFunction>(
+ named_window_aggregate_expression->expression());
+
+ // Set the AggregateFunction.
+ window_aggr_state_proto->mutable_function()->CopyFrom(
+ window_aggregate_function->window_aggregate().getProto());
+
+ // Set the arguments.
+ for (const E::ScalarPtr &argument : window_aggregate_function->arguments()) {
+ unique_ptr<const Scalar> concretized_argument(argument->concretize(attribute_substitution_map_));
+ window_aggr_state_proto->add_arguments()->CopyFrom(concretized_argument->getProto());
+ }
+
+ // Set partition keys.
+ const E::WindowInfo window_info = window_aggregate_function->window_info();
+ for (const E::ScalarPtr &partition_by_attribute
+ : window_info.partition_by_attributes) {
+ unique_ptr<const Scalar> concretized_partition_by_attribute(
+ partition_by_attribute->concretize(attribute_substitution_map_));
+ window_aggr_state_proto->add_partition_by_attributes()
+ ->CopyFrom(concretized_partition_by_attribute->getProto());
+ }
+
+ // Set window frame info.
+ if (window_info.frame_info == nullptr) {
+ // If the frame is not specified, use the default setting:
+ // 1. If ORDER BY key is specified, use cumulative aggregation:
+ // ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW.
+ // 2. If ORDER BY key is not specified either, use the whole partition:
+ // ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING.
+ window_aggr_state_proto->set_is_row(true); /* frame mode: ROWS */
+ window_aggr_state_proto->set_num_preceding(-1); /* UNBOUNDED PRECEDING */
+ if (window_info.order_by_attributes.empty()) {
+ window_aggr_state_proto->set_num_following(-1); /* UNBOUNDED FOLLOWING */
+ } else {
+ window_aggr_state_proto->set_num_following(0); /* CURRENT ROW */
+ }
+ } else {
+ const E::WindowFrameInfo* window_frame_info = window_info.frame_info;
--- End diff --
Code style: `const E::WindowFrameInfo *window_frame_info`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quickstep pull request #49: QUICKSTEP-20: Added ExecutionGenerator...
Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/49#discussion_r69970397
--- Diff: query_optimizer/ExecutionGenerator.cpp ---
@@ -1639,5 +1641,101 @@ void ExecutionGenerator::convertTableGenerator(
temporary_relation_info_vec_.emplace_back(tablegen_index, output_relation);
}
+void ExecutionGenerator::convertWindowAggregate(
+ const P::WindowAggregatePtr &physical_plan) {
+ // Create window_aggregation_operation_state proto.
+ const QueryContext::window_aggregation_state_id window_aggr_state_index =
+ query_context_proto_->window_aggregation_states_size();
+ S::WindowAggregationOperationState *window_aggr_state_proto =
+ query_context_proto_->add_window_aggregation_states();
+
+ // Get input.
+ const CatalogRelationInfo *input_relation_info =
+ findRelationInfoOutputByPhysical(physical_plan->input());
+ window_aggr_state_proto->set_relation_id(input_relation_info->relation->getID());
+
+ // Get window aggregate function expression.
+ const E::AliasPtr &named_window_aggregate_expression =
+ physical_plan->window_aggregate_expression();
+ const E::WindowAggregateFunctionPtr &window_aggregate_function =
+ std::static_pointer_cast<const E::WindowAggregateFunction>(
+ named_window_aggregate_expression->expression());
+
+ // Set the AggregateFunction.
+ window_aggr_state_proto->mutable_function()->CopyFrom(
+ window_aggregate_function->window_aggregate().getProto());
+
+ // Set the arguments.
+ for (const E::ScalarPtr &argument : window_aggregate_function->arguments()) {
+ unique_ptr<const Scalar> concretized_argument(argument->concretize(attribute_substitution_map_));
+ window_aggr_state_proto->add_arguments()->CopyFrom(concretized_argument->getProto());
+ }
+
+ // Set partition keys.
+ const E::WindowInfo window_info = window_aggregate_function->window_info();
--- End diff --
I was wondering if we could use reference `&` here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quickstep pull request #49: QUICKSTEP-20: Added ExecutionGenerator...
Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/49#discussion_r69970266
--- Diff: query_optimizer/ExecutionGenerator.cpp ---
@@ -1639,5 +1641,101 @@ void ExecutionGenerator::convertTableGenerator(
temporary_relation_info_vec_.emplace_back(tablegen_index, output_relation);
}
+void ExecutionGenerator::convertWindowAggregate(
+ const P::WindowAggregatePtr &physical_plan) {
+ // Create window_aggregation_operation_state proto.
+ const QueryContext::window_aggregation_state_id window_aggr_state_index =
+ query_context_proto_->window_aggregation_states_size();
+ S::WindowAggregationOperationState *window_aggr_state_proto =
+ query_context_proto_->add_window_aggregation_states();
+
+ // Get input.
+ const CatalogRelationInfo *input_relation_info =
+ findRelationInfoOutputByPhysical(physical_plan->input());
+ window_aggr_state_proto->set_relation_id(input_relation_info->relation->getID());
+
+ // Get window aggregate function expression.
+ const E::AliasPtr &named_window_aggregate_expression =
+ physical_plan->window_aggregate_expression();
+ const E::WindowAggregateFunctionPtr &window_aggregate_function =
+ std::static_pointer_cast<const E::WindowAggregateFunction>(
+ named_window_aggregate_expression->expression());
+
+ // Set the AggregateFunction.
+ window_aggr_state_proto->mutable_function()->CopyFrom(
+ window_aggregate_function->window_aggregate().getProto());
+
+ // Set the arguments.
+ for (const E::ScalarPtr &argument : window_aggregate_function->arguments()) {
+ unique_ptr<const Scalar> concretized_argument(argument->concretize(attribute_substitution_map_));
+ window_aggr_state_proto->add_arguments()->CopyFrom(concretized_argument->getProto());
--- End diff --
Ditto for `MergeFrom`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quickstep pull request #49: QUICKSTEP-20: Added ExecutionGenerator...
Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/49#discussion_r69975485
--- Diff: query_optimizer/cost_model/SimpleCostModel.hpp ---
@@ -88,6 +89,10 @@ class SimpleCostModel : public CostModel {
std::size_t estimateCardinalityForAggregate(
const physical::AggregatePtr &physical_plan);
+ // Return the estimated cardinality of the input plan
--- End diff --
Code style: Add the period `.` in the end.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quickstep pull request #49: QUICKSTEP-20: Added ExecutionGenerator...
Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/49#discussion_r69970211
--- Diff: query_optimizer/ExecutionGenerator.cpp ---
@@ -1639,5 +1641,101 @@ void ExecutionGenerator::convertTableGenerator(
temporary_relation_info_vec_.emplace_back(tablegen_index, output_relation);
}
+void ExecutionGenerator::convertWindowAggregate(
+ const P::WindowAggregatePtr &physical_plan) {
+ // Create window_aggregation_operation_state proto.
+ const QueryContext::window_aggregation_state_id window_aggr_state_index =
+ query_context_proto_->window_aggregation_states_size();
+ S::WindowAggregationOperationState *window_aggr_state_proto =
+ query_context_proto_->add_window_aggregation_states();
+
+ // Get input.
+ const CatalogRelationInfo *input_relation_info =
+ findRelationInfoOutputByPhysical(physical_plan->input());
+ window_aggr_state_proto->set_relation_id(input_relation_info->relation->getID());
+
+ // Get window aggregate function expression.
+ const E::AliasPtr &named_window_aggregate_expression =
+ physical_plan->window_aggregate_expression();
+ const E::WindowAggregateFunctionPtr &window_aggregate_function =
+ std::static_pointer_cast<const E::WindowAggregateFunction>(
+ named_window_aggregate_expression->expression());
+
+ // Set the AggregateFunction.
+ window_aggr_state_proto->mutable_function()->CopyFrom(
--- End diff --
Please use `MergeFrom`, as we don't need to clean up.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quickstep issue #49: QUICKSTEP-20: Added ExecutionGenerator suppor...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on the issue:
https://github.com/apache/incubator-quickstep/pull/49
@zuyu Thanks a lot for your review. Addressed them in the latest commit :).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---