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/15 18:00:59 UTC
[GitHub] incubator-quickstep pull request #58: QUICKSTEP-20: Supported Window Aggrega...
GitHub user shixuan-fan opened a pull request:
https://github.com/apache/incubator-quickstep/pull/58
QUICKSTEP-20: Supported Window Aggregation for AVG
Quickstep could now make the calculation for window aggregation function average. The work-flow is:
`ExecutionGenerator` constructs `WindowAggregationOperator` and initializes `WindowAggregationOperationState`
=> `WindowAggregationOperator::execute()` is called
=> `WindowAggregationOperationState::windowAggregate()`: combines all blocks' tuples into one `ValueAccessor`
=> `WindowAggregationHandle::calculate()`: actually does the calculation and return a `ColumnVector*` including all the calculated window aggregates
=> `WindowAggregationOperationState::windowAggregate()` will add the new `ColumnVector` into the created `ValueAccessor` and output the result to the `output_destination`.
We are using the simplest approach for the calculation: one thread will iterate through all the tuples. For each tuple, the thread will find all the elements in the given window and calculate the result.
In current version, we don't support `RANGE` frame mode. Also, other functions' handle will be created in the next PR.
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/58.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 #58
----
commit 2966405066b223d0079f8df3e231ed56750259ad
Author: shixuan-fan <sh...@apache.org>
Date: 2016-07-08T18:23:47Z
Added handle for avg
commit d5f535ee9f0e3c7a8ea7615a07699b83c5f905f4
Author: shixuan-fan <sh...@apache.org>
Date: 2016-07-11T14:11:47Z
Seperated calculation into two parts to check intermediate result
commit 897ffc6db0e177caf73131b39c80d6d077087c8e
Author: shixuan-fan <sh...@apache.org>
Date: 2016-07-12T04:54:20Z
Fixed SegFault
commit f4eb4a7ccff40d278a4f4e32737edc5f93fb7419
Author: shixuan-fan <sh...@apache.org>
Date: 2016-07-12T19:12:36Z
Fixed empty table when p_key or o_key specified
commit e16fdf75240a705a6d18a841cf94bcd78619f077
Author: shixuan-fan <sh...@apache.org>
Date: 2016-07-12T19:48:59Z
Minor fix
commit c50ce513941842c7ae7108ddb76054304514587a
Author: shixuan-fan <sh...@apache.org>
Date: 2016-07-13T17:00:41Z
Let WindowAggregationOperationState handle data copying
commit 0f5c41d1531032065e747b642b9013802f74ac06
Author: shixuan-fan <sh...@apache.org>
Date: 2016-07-13T21:26:29Z
Removed finalize()
commit 5f4647b73a57981838540185bf0bc63e80a3c1dd
Author: shixuan-fan <sh...@apache.org>
Date: 2016-07-14T16:29:08Z
Added unittest for avg on Cumulative aggregation and sliding window
commit 727761bb083a197698db7e0f155a95c9f6b60cf6
Author: shixuan-fan <sh...@apache.org>
Date: 2016-07-15T17:30:53Z
Added WindowAggregateFunctions for other functions without handle
commit ab9dc8b525ef5e24f9b821d23ea6ac6bc88e594e
Author: shixuan-fan <sh...@apache.org>
Date: 2016-07-15T17:38:05Z
Added all-in-one module
----
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71228801
--- Diff: expressions/window_aggregation/WindowAggregateFunctionFactory.hpp ---
@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ **/
+
+#ifndef QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_FACTORY_HPP_
+#define QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_FACTORY_HPP_
+
+#include <string>
+
+#include "expressions/window_aggregation/WindowAggregationID.hpp"
+#include "utility/Macros.hpp"
+
+namespace quickstep {
+
+class WindowAggregateFunction;
+namespace serialization { class WindowAggregateFunction; }
+
+/** \addtogroup Expressions
+ * @{
+ */
+
+/**
+ * @brief All-static factory class that provides access to the various concrete
+ * implementations of WindowAggregateFunction.
+ *
+ * WindowAggregateFunctionFactory allows client code to use any
+ * WindowAggregateFunction in Quickstep in a generic way without having to know
+ * about all the specific subclasses of WindowAggregateFunction. In particular,
+ * it is used to deserialize WindowAggregateFunctions used in
+ * WindowAggregationOperationState from their protobuf representations
+ * (originally created by the optimizer) when deserializing a QueryContext.
+ **/
+class WindowAggregateFunctionFactory {
--- End diff --
It is optional, and we could do the refactor later according to this [code style](https://google.github.io/styleguide/cppguide.html#Nonmember,_Static_Member,_and_Global_Functions).
We have to use the namespace install of static member functions.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71233398
--- Diff: query_optimizer/resolver/Resolver.cpp ---
@@ -2670,7 +2681,8 @@ E::ScalarPtr Resolver::resolveFunctionCall(
}
// Make sure that the aggregate can apply to the specified argument(s).
- if (!aggregate->canApplyToTypes(argument_types)) {
+ if ((aggregate != nullptr && !aggregate->canApplyToTypes(argument_types))
--- End diff --
Code style for the bool expressions:
https://google.github.io/styleguide/cppguide.html#Boolean_Expressions
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71250298
--- Diff: storage/WindowAggregationOperationState.cpp ---
@@ -176,4 +171,122 @@ bool WindowAggregationOperationState::ProtoIsValid(const serialization::WindowAg
return true;
}
+void WindowAggregationOperationState::windowAggregateBlocks(
+ InsertDestination *output_destination,
+ const std::vector<block_id> &block_ids) {
+ // TODO(Shixuan): This is a quick fix for currently unsupported functions in
+ // order to pass the query_optimizer test.
+ if (window_aggregation_handle_.get() == nullptr) {
+ std::cout << "The function will be supported in the near future :)\n";
+ return;
+ }
+
+ // TODO(Shixuan): RANGE frame mode should be supported to make SQL grammar
+ // work. This will need Order Key to be passed so that we know where the
+ // window should start and end.
+ if (!is_row_) {
+ std::cout << "Currently we don't support RANGE frame mode :(\n";
+ return;
+ }
+
+ // Get the total number of tuples.
+ int num_tuples = 0;
+ for (block_id block_idx : block_ids) {
+ num_tuples +=
+ storage_manager_->getBlock(block_idx, input_relation_)->getNumTuples();
+ }
+
+ // Construct column vectors for attributes.
+ std::vector<ColumnVector*> attribute_vecs;
+ for (std::size_t attr_id = 0; attr_id < input_relation_.size(); ++attr_id) {
+ const CatalogAttribute *attr = input_relation_.getAttributeById(attr_id);
+ const Type &type = attr->getType();
+
+ if (NativeColumnVector::UsableForType(type)) {
+ attribute_vecs.push_back(new NativeColumnVector(type, num_tuples));
+ } else {
+ attribute_vecs.push_back(new IndirectColumnVector(type, num_tuples));
+ }
+ }
+
+ // Construct column vectors for arguments.
+ std::vector<ColumnVector*> argument_vecs;
+ for (std::unique_ptr<const Scalar> &argument : arguments_) {
+ const Type &type = argument->getType();
+
+ if (NativeColumnVector::UsableForType(type)) {
+ argument_vecs.push_back(new NativeColumnVector(type, num_tuples));
+ } else {
+ argument_vecs.push_back(new IndirectColumnVector(type, num_tuples));
+ }
+ }
+
+ // TODO(Shixuan): Add Support for Vector Copy Elision Selection.
+ // Add tuples and arguments into ColumnVectors.
+ for (block_id block_idx : block_ids) {
+ BlockReference block = storage_manager_->getBlock(block_idx, input_relation_);
+ const TupleStorageSubBlock &tuple_block = block->getTupleStorageSubBlock();
+ SubBlocksReference sub_block_ref(tuple_block,
+ block->getIndices(),
+ block->getIndicesConsistent());
+ ValueAccessor *tuple_accessor = tuple_block.createValueAccessor();
+ ColumnVectorsValueAccessor *argument_accessor = new ColumnVectorsValueAccessor();
+ for (std::unique_ptr<const Scalar> &argument : arguments_) {
--- End diff --
Add `const` for `argument`.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71227070
--- Diff: expressions/window_aggregation/WindowAggregateFunctionFactory.cpp ---
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 "expressions/window_aggregation/WindowAggregateFunctionFactory.hpp"
+
+#include <string>
+#include <type_traits>
+
+#include "expressions/window_aggregation/WindowAggregateFunction.pb.h"
+#include "expressions/window_aggregation/WindowAggregateFunctionAvg.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunctionCount.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunctionMax.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunctionMin.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunctionSum.hpp"
+#include "expressions/window_aggregation/WindowAggregationID.hpp"
+
+#include "glog/logging.h"
+
+namespace quickstep {
+
+const WindowAggregateFunction& WindowAggregateFunctionFactory::Get(
+ const WindowAggregationID agg_id) {
+ switch (agg_id) {
+ case WindowAggregationID::kAvg:
+ return WindowAggregateFunctionAvg::Instance();
+ case WindowAggregationID::kCount:
+ return WindowAggregateFunctionCount::Instance();
+ case WindowAggregationID::kMax:
+ return WindowAggregateFunctionMax::Instance();
+ case WindowAggregationID::kMin:
+ return WindowAggregateFunctionMin::Instance();
+ case WindowAggregationID::kSum:
+ return WindowAggregateFunctionSum::Instance();
+ default: {
+ LOG(FATAL) << "Unrecognized WindowAggregationID: "
+ << static_cast<std::underlying_type<WindowAggregationID>::type>(agg_id);
+ }
+ }
+}
+
+const WindowAggregateFunction* WindowAggregateFunctionFactory::GetByName(
+ const std::string &name) {
+ if (name == "avg") {
+ return &WindowAggregateFunctionAvg::Instance();
+ } else if (name == "count") {
+ return &WindowAggregateFunctionCount::Instance();
+ } else if (name == "max") {
+ return &WindowAggregateFunctionMax::Instance();
+ } else if (name == "min") {
+ return &WindowAggregateFunctionMin::Instance();
+ } else if (name == "sum") {
+ return &WindowAggregateFunctionSum::Instance();
+ } else {
+ return nullptr;
+ }
+}
+
+bool WindowAggregateFunctionFactory::ProtoIsValid(
+ const serialization::WindowAggregateFunction &proto) {
+ return proto.IsInitialized()
--- End diff --
[Code style](https://google.github.io/styleguide/cppguide.html#Return_Values):
Please refactor to
```
return xxx &&
yyy;
```
---
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 #58: QUICKSTEP-20: Supported Window Aggregation fo...
Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on the issue:
https://github.com/apache/incubator-quickstep/pull/58
Wait for the CI.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71229567
--- Diff: expressions/window_aggregation/WindowAggregateFunctionMin.hpp ---
@@ -0,0 +1,75 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ **/
+
+#ifndef QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_MIN_HPP_
+#define QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_MIN_HPP_
+
+#include <string>
+#include <vector>
+
+#include "expressions/window_aggregation/WindowAggregateFunction.hpp"
+#include "expressions/window_aggregation/WindowAggregationID.hpp"
+#include "utility/Macros.hpp"
+
+namespace quickstep {
+
+class WindowAggregationHandle;
+class Type;
--- End diff --
Code style: reorder the declaration.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71232847
--- Diff: expressions/window_aggregation/tests/WindowAggregationHandleAvg_unittest.cpp ---
@@ -0,0 +1,398 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ * limitations under the License.
+ **/
+
+#include <cstddef>
+#include <cstdint>
+#include <memory>
+#include <vector>
+
+#include "catalog/CatalogTypedefs.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunction.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunctionFactory.hpp"
+#include "expressions/window_aggregation/WindowAggregationHandle.hpp"
+#include "expressions/window_aggregation/WindowAggregationHandleAvg.hpp"
+#include "expressions/window_aggregation/WindowAggregationID.hpp"
+#include "storage/ValueAccessor.hpp"
+#include "types/CharType.hpp"
+#include "types/DateOperatorOverloads.hpp"
+#include "types/DatetimeIntervalType.hpp"
+#include "types/DoubleType.hpp"
+#include "types/FloatType.hpp"
+#include "types/IntType.hpp"
+#include "types/IntervalLit.hpp"
+#include "types/LongType.hpp"
+#include "types/Type.hpp"
+#include "types/TypeFactory.hpp"
+#include "types/TypeID.hpp"
+#include "types/TypedValue.hpp"
+#include "types/VarCharType.hpp"
+#include "types/YearMonthIntervalType.hpp"
+#include "types/containers/ColumnVector.hpp"
+#include "types/containers/ColumnVectorsValueAccessor.hpp"
+
+#include "gtest/gtest.h"
+
+namespace quickstep {
+
+namespace {
+
+ constexpr int kNumTuples = 100;
+ constexpr int kNumTuplesPerPartition = 8;
+ constexpr int kNullInterval = 25;
+ constexpr int kNumPreceding = 2;
+ constexpr int kNumFollowing = 2;
+
+} // namespace
+
+// Attribute value could be null if set true.
+class WindowAggregationHandleAvgTest : public::testing::TestWithParam<bool> {
+ protected:
+ // Handle initialization.
+ void initializeHandle(const Type &argument_type) {
+ const WindowAggregateFunction &function =
+ WindowAggregateFunctionFactory::Get(WindowAggregationID::kAvg);
+ std::vector<const Type*> partition_key_types(1, &TypeFactory::GetType(kInt, false));
+ handle_avg_.reset(function.createHandle(std::vector<const Type*>(1, &argument_type),
+ std::move(partition_key_types)));
+ }
+
+ // Test canApplyToTypes().
+ static bool CanApplyToTypesTest(TypeID typeID) {
+ const Type &type = (typeID == kChar || typeID == kVarChar) ?
+ TypeFactory::GetType(typeID, static_cast<std::size_t>(10)) :
+ TypeFactory::GetType(typeID);
+
+ return WindowAggregateFunctionFactory::Get(WindowAggregationID::kAvg).canApplyToTypes(
+ std::vector<const Type*>(1, &type));
+ }
+
+ // Test resultTypeForArgumentTypes().
+ static bool ResultTypeForArgumentTypesTest(TypeID input_type_id,
+ TypeID output_type_id) {
+ const Type *result_type
+ = WindowAggregateFunctionFactory::Get(WindowAggregationID::kAvg).resultTypeForArgumentTypes(
+ std::vector<const Type*>(1, &TypeFactory::GetType(input_type_id)));
+ return (result_type->getTypeID() == output_type_id);
+ }
+
+ template <typename CppType>
+ static void CheckAvgValues(
+ const std::vector<CppType*> &expected,
+ const ColumnVector *actual) {
+ EXPECT_TRUE(actual->isNative());
+ const NativeColumnVector *native = static_cast<const NativeColumnVector*>(actual);
+
+ EXPECT_EQ(expected.size(), native->size());
+ for (std::size_t i = 0; i < expected.size(); ++i) {
+ if (expected[i] == nullptr) {
+ EXPECT_TRUE(native->getTypedValue(i).isNull());
+ } else {
+ EXPECT_EQ(*expected[i], native->getTypedValue(i).getLiteral<CppType>());
+ }
+ }
+ }
+
+ // Static templated methods for set a meaningful value to data types.
+ template <typename CppType>
+ static void SetDataType(int value, CppType *data) {
+ *data = value;
+ }
+
+ template <typename GenericType, typename OutputType = DoubleType>
+ void checkWindowAggregationAvgGeneric() {
+ const GenericType &type = GenericType::Instance(true);
+ initializeHandle(type);
+
+ // Create argument, partition key and cpptype vectors.
+ std::vector<typename GenericType::cpptype*> argument_cpp_vector;
+ argument_cpp_vector.reserve(kNumTuples);
+ ColumnVector *argument_type_vector =
+ createArgumentGeneric<GenericType>(&argument_cpp_vector);
+ NativeColumnVector *partition_key_vector =
+ new NativeColumnVector(IntType::InstanceNonNullable(), kNumTuples + 2);
+
+ for (int i = 0; i < kNumTuples; ++i) {
+ partition_key_vector->appendTypedValue(TypedValue(i / kNumTuplesPerPartition));
+ }
+
+ // Create tuple ValueAccessor.
+ ColumnVectorsValueAccessor *tuple_accessor = new ColumnVectorsValueAccessor();
+ tuple_accessor->addColumn(partition_key_vector);
+ tuple_accessor->addColumn(argument_type_vector);
+
+ // Test UNBOUNDED PRECEDING AND CURRENT ROW.
+ checkAccumulate<GenericType, OutputType>(tuple_accessor,
+ argument_type_vector,
+ argument_cpp_vector);
+ // Test kNumPreceding PRECEDING AND kNumFollowing FOLLOWING.
+ checkSlidingWindow<GenericType, OutputType>(tuple_accessor,
+ argument_type_vector,
+ argument_cpp_vector);
+ }
+
+ template <typename GenericType>
+ ColumnVector *createArgumentGeneric(
+ std::vector<typename GenericType::cpptype*> *argument_cpp_vector) {
+ const GenericType &type = GenericType::Instance(true);
+ NativeColumnVector *column = new NativeColumnVector(type, kNumTuples);
+
+ for (int i = 0; i < kNumTuples; ++i) {
+ // Insert a NULL every kNullInterval tuples.
+ if (i % kNullInterval == 0) {
+ argument_cpp_vector->push_back(nullptr);
+ column->appendTypedValue(type.makeNullValue());
+ continue;
+ }
+
+ typename GenericType::cpptype *val = new typename GenericType::cpptype;
+
+ if (type.getTypeID() == kInt || type.getTypeID() == kLong) {
+ SetDataType(i - 10, val);
+ } else {
+ SetDataType(static_cast<float>(i - 10) / 10, val);
+ }
+
+ column->appendTypedValue(type.makeValue(val));
+ argument_cpp_vector->push_back(val);
+ }
+
+ return column;
+ }
+
+ template <typename GenericType, typename OutputType>
+ void checkAccumulate(ColumnVectorsValueAccessor *tuple_accessor,
+ ColumnVector *argument_type_vector,
+ const std::vector<typename GenericType::cpptype*> &argument_cpp_vector) {
+ std::vector<ColumnVector*> arguments;
+ arguments.push_back(argument_type_vector);
+ // The partition key index is 0.
+ std::vector<attribute_id> partition_key(1, 0);
+
+ ColumnVector *result =
+ handle_avg_->calculate(tuple_accessor,
+ std::move(arguments),
+ partition_key,
+ true /* is_row */,
+ -1 /* num_preceding: UNBOUNDED PRECEDING */,
+ 0 /* num_following: CURRENT ROW */);
+
+ // Get the cpptype result.
+ std::vector<typename OutputType::cpptype*> result_cpp_vector;
+ bool is_null;
+ typename GenericType::cpptype sum;
+ int count;
+ for (std::size_t i = 0; i < argument_cpp_vector.size(); ++i) {
+ // Start of new partition
+ if (i % kNumTuplesPerPartition == 0) {
+ is_null = false;
+ SetDataType(0, &sum);
+ count = 0;
+ }
+
+ typename GenericType::cpptype *value = argument_cpp_vector[i];
+ if (value == nullptr) {
+ is_null = true;
+ }
+
+ if (is_null) {
+ result_cpp_vector.push_back(nullptr);
+ } else {
+ sum += *value;
+ count++;
+ typename OutputType::cpptype *result_cpp_value =
+ new typename OutputType::cpptype;
+ *result_cpp_value = static_cast<typename OutputType::cpptype>(sum) / count;
+ result_cpp_vector.push_back(result_cpp_value);
+ }
+ }
+
+ CheckAvgValues(result_cpp_vector, result);
+ }
+
+ template <typename GenericType, typename OutputType>
+ void checkSlidingWindow(ColumnVectorsValueAccessor *tuple_accessor,
+ ColumnVector *argument_type_vector,
+ const std::vector<typename GenericType::cpptype*> &argument_cpp_vector) {
+ std::vector<ColumnVector*> arguments;
+ arguments.push_back(argument_type_vector);
+ // The partition key index is 0.
+ std::vector<attribute_id> partition_key(1, 0);
+
+ ColumnVector *result =
+ handle_avg_->calculate(tuple_accessor,
+ std::move(arguments),
+ partition_key,
+ true /* is_row */,
+ kNumPreceding,
+ kNumFollowing);
+
+ // Get the cpptype result.
+ // For each value, calculate all surrounding values in the window.
+ std::vector<typename OutputType::cpptype*> result_cpp_vector;
+
+ for (std::size_t i = 0; i < argument_cpp_vector.size(); ++i) {
+ if (argument_cpp_vector[i] == nullptr) {
+ result_cpp_vector.push_back(nullptr);
+ continue;
+ }
+
+ typename GenericType::cpptype sum;
+ SetDataType(0, &sum);
+ sum += *argument_cpp_vector[i];
+ int count = 1;
+ bool is_null = false;
+
+ for (std::size_t precede = 1; precede <= kNumPreceding; ++precede) {
+ // Not the same partition.
+ if (i / kNumTuplesPerPartition != (i - precede) / kNumTuplesPerPartition
+ || i < precede) {
+ break;
+ }
+
+ if (argument_cpp_vector[i - precede] == nullptr) {
+ is_null = true;
+ break;
+ }
+
+ sum += *argument_cpp_vector[i - precede];
+ count++;
+ }
+
+ for (int follow = 1; follow <= kNumPreceding; ++follow) {
+ // Not the same partition.
+ if (i / kNumTuplesPerPartition != (i + follow) / kNumTuplesPerPartition
--- End diff --
Ditto for the code style for boolean expressions:
https://google.github.io/styleguide/cppguide.html#Boolean_Expressions
---
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 #58: QUICKSTEP-20: Supported Window Aggregation fo...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on the issue:
https://github.com/apache/incubator-quickstep/pull/58
@zuyu Thanks for all your efforts! I modified the codes according to your suggestion. Your second pass will be most appreciated. Take your time, no hurry.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/58#discussion_r71358472
--- Diff: relational_operators/WindowAggregationOperator.cpp ---
@@ -67,6 +73,11 @@ serialization::WorkOrder* WindowAggregationOperator::createWorkOrderProto() {
proto->set_query_id(query_id_);
proto->SetExtension(serialization::WindowAggregationWorkOrder::window_aggr_state_index,
window_aggregation_state_index_);
+
+ std::vector<block_id> relation_blocks = input_relation_.getBlocksSnapshot();
--- End diff --
Good catch!
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/58#discussion_r71349440
--- Diff: expressions/window_aggregation/CMakeLists.txt ---
@@ -0,0 +1,212 @@
+# Copyright 2011-2015 Quickstep Technologies LLC.
+# Copyright 2015 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.
+
+QS_PROTOBUF_GENERATE_CPP(expressions_windowaggregation_WindowAggregateFunction_proto_srcs
+ expressions_windowaggregation_WindowAggregateFunction_proto_hdrs
+ WindowAggregateFunction.proto)
+
+# Declare micro-libs:
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunction
+ WindowAggregateFunction.cpp
+ WindowAggregateFunction.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunction_proto
+ ${expressions_windowaggregation_WindowAggregateFunction_proto_srcs})
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionAvg
+ WindowAggregateFunctionAvg.cpp
+ WindowAggregateFunctionAvg.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionCount
+ WindowAggregateFunctionCount.cpp
+ WindowAggregateFunctionCount.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionMax
+ WindowAggregateFunctionMax.cpp
+ WindowAggregateFunctionMax.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionMin
+ WindowAggregateFunctionMin.cpp
+ WindowAggregateFunctionMin.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionSum
+ WindowAggregateFunctionSum.cpp
+ WindowAggregateFunctionSum.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionFactory
--- End diff --
That's weird. Maybe I missed this commit during the squashing. Will fix it in the next 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.
---
[GitHub] incubator-quickstep pull request #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/58#discussion_r71350702
--- Diff: expressions/window_aggregation/WindowAggregateFunctionCount.cpp ---
@@ -0,0 +1,59 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 "expressions/window_aggregation/WindowAggregateFunctionCount.hpp"
+
+#include <vector>
+
+#include "expressions/window_aggregation/WindowAggregationHandle.hpp"
+#include "types/Type.hpp"
+#include "types/TypeFactory.hpp"
+#include "types/TypeID.hpp"
+
+#include "glog/logging.h"
+
+namespace quickstep {
+
+bool WindowAggregateFunctionCount::canApplyToTypes(
+ const std::vector<const Type*> &argument_types) const {
+ // COUNT may be nullary (i.e. COUNT(*)) or unary.
+ return argument_types.size() <= 1;
+}
+
+const Type* WindowAggregateFunctionCount::resultTypeForArgumentTypes(
+ const std::vector<const Type*> &argument_types) const {
+ if (!canApplyToTypes(argument_types)) {
+ return nullptr;
+ }
+
+ return &TypeFactory::GetType(kLong);
+}
+
+WindowAggregationHandle* WindowAggregateFunctionCount::createHandle(
+ std::vector<const Type*> &&argument_types,
--- End diff --
You are right. A reference would be better.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/58#discussion_r71361264
--- Diff: storage/WindowAggregationOperationState.cpp ---
@@ -126,8 +121,8 @@ WindowAggregationOperationState* WindowAggregationOperationState::ReconstructFro
const std::int64_t num_preceding = proto.num_preceding();
const std::int64_t num_following = proto.num_following();
- return new WindowAggregationOperationState(database.getRelationSchemaById(proto.relation_id()),
- aggregate_function,
+ return new WindowAggregationOperationState(database.getRelationSchemaById(proto.input_relation_id()),
+ window_aggregate_function,
--- End diff --
Sounds great.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71229902
--- Diff: expressions/window_aggregation/WindowAggregationHandle.hpp ---
@@ -0,0 +1,137 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ **/
+
+#ifndef QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATION_HANDLE_HPP_
+#define QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATION_HANDLE_HPP_
+
+#include <cstddef>
+#include <memory>
+#include <vector>
+
+#include "catalog/CatalogRelationSchema.hpp"
+#include "catalog/CatalogTypedefs.hpp"
+#include "storage/StorageBlockInfo.hpp"
+#include "types/TypedValue.hpp"
+#include "types/containers/ColumnVector.hpp"
+#include "types/containers/ColumnVectorsValueAccessor.hpp"
+#include "types/operations/comparisons/Comparison.hpp"
+#include "types/operations/comparisons/ComparisonFactory.hpp"
+#include "types/operations/comparisons/ComparisonID.hpp"
+#include "utility/Macros.hpp"
+
+namespace quickstep {
+
+class InsertDestinationInterface;
+class Scalar;
+class StorageManager;
+class Type;
+class ValueAccessor;
+
+/** \addtogroup Expressions
+ * @{
+ */
+
+/**
+ * @brief WindowAggregationHandle encapsulates logic for actually computing
+ * window aggregates with particular argument(s).
+ * @note See also WindowAggregateFunction, which represents a SQL aggregate
+ * function in the abstract sense.
+ *
+ * A WindowAggregationHandle is created by calling
+ * WindowAggregateFunction::createHandle(). The WindowAggregationHandle object
+ * provides methods that are used to actually compute the window aggregate,
+ * storing intermediate results in WindowAggregationState objects.
+ *
+ * The work flow for computing a window aggregate is:
+ * 1. Create an initial state by createInitialState().
+ * 2. One thread will handle all the computation, iterating from the first
+ * tuple to the last tuple. Note there will be two modes that could be
+ * used upon different situations:
+ * a. If the window aggregate is defined as accumulative, which are:
+ * i. Functions applied to whole partition, such as rank(), ntile()
+ * and dense_rank().
+ * ii. The window frame is defined as "BETWEEN UNBOUNDED PRECEDING
+ * AND CURRENT ROW" or "BETWEEN CURRENT ROW AND UNBOUNDED
+ * FOLLOWING".
+ * Then, for functions except median, we could store some global
+ * values in the state without keeping all the tuple values around.
+ * b. If the window frame is sliding, such as "BETWEEN 3 PRECEDING AND
+ * 3 FOLLOWING", we have to store all the tuples in the state so that
+ * we could know which values should be dropped as the window slides.
+ * For each computed value, generate a tuple store in the column vector.
+ * 3. Insert the new column into the original relation and return.
+ *
+ * TODO(Shixuan): Currently we don't support parallelization. The basic idea for
+ * parallelization is to calculate the partial result inside each block. Each
+ * block could visit the following blocks as long as the block's last partition
+ * is not finished. WindowAggregationOperationState will be used for handling
+ * the global state of the calculation.
+ **/
+
+class WindowAggregationHandle {
+ public:
+ /**
+ * @brief Destructor.
+ **/
+ virtual ~WindowAggregationHandle() {}
+
+ /**
+ * @brief Calculate the window aggregate result.
+ *
+ * @param block_accessors A pointer to the value accessor of block attributes.
+ * @param arguments The ColumnVectors of arguments
+ * @param partition_by_ids The ids of partition keys.
+ * @param is_row True if the frame mode is ROWS, false if it is RANGE.
+ * @param num_preceding The number of rows/range that precedes the current row.
+ * @param num_following The number of rows/range that follows the current row.
+ *
+ * @return A ColumnVector of the calculated window aggregates.
+ **/
+ virtual ColumnVector* calculate(ColumnVectorsValueAccessor* block_accessors,
+ std::vector<ColumnVector*> &&arguments,
+ const std::vector<attribute_id> &partition_by_ids,
+ const bool is_row,
+ const std::int64_t num_preceding,
+ const std::int64_t num_following) const = 0;
+
+ protected:
+ /**
+ * @brief Constructor.
+ *
+ * @param partition_key_types The Types of the partition key.
+ **/
+ explicit WindowAggregationHandle(
+ std::vector<const Type*> &&partition_key_types) {
--- End diff --
I was wondering why using a rvalue 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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71233163
--- Diff: query_optimizer/expressions/WindowAggregateFunction.hpp ---
@@ -33,7 +33,7 @@
namespace quickstep {
-class AggregateFunction;
+class WindowAggregateFunction;
--- End diff --
Code style: reorder the declaration.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71369895
--- Diff: query_optimizer/tests/execution_generator/Select.test ---
@@ -953,19 +953,79 @@ WHERE double_col < 0
==
# Window Aggregation Test.
-# Currently this is not supported, an empty table will be returned.
-SELECT avg(int_col) OVER w FROM test
+SELECT char_col, long_col, avg(long_col) OVER w FROM test
WINDOW w AS
-(PARTITION BY char_col
- ORDER BY long_col DESC NULLS LAST
+(ORDER BY char_col DESC NULLS LAST
--- End diff --
I get it, but do we have other alternatives to keep the semantics of `window aggregation`: aggregating over incoming tuples.
It particular makes more sense for the following next test cases w/ `NULL`s.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71250716
--- Diff: storage/WindowAggregationOperationState.cpp ---
@@ -126,8 +121,8 @@ WindowAggregationOperationState* WindowAggregationOperationState::ReconstructFro
const std::int64_t num_preceding = proto.num_preceding();
const std::int64_t num_following = proto.num_following();
- return new WindowAggregationOperationState(database.getRelationSchemaById(proto.relation_id()),
- aggregate_function,
+ return new WindowAggregationOperationState(database.getRelationSchemaById(proto.input_relation_id()),
+ window_aggregate_function,
--- End diff --
FYI, we could directly use `&WindowAggregateFunctionFactory::ReconstructFromProto(proto.function())` for `window_aggregate_function`.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71229745
--- Diff: expressions/window_aggregation/WindowAggregateFunctionSum.hpp ---
@@ -0,0 +1,75 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ **/
+
+#ifndef QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_SUM_HPP_
+#define QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_SUM_HPP_
+
+#include <string>
+#include <vector>
+
+#include "expressions/window_aggregation/WindowAggregateFunction.hpp"
+#include "expressions/window_aggregation/WindowAggregationID.hpp"
+#include "utility/Macros.hpp"
+
+namespace quickstep {
+
+class WindowAggregationHandle;
+class Type;
--- End diff --
Code style: reorder the declaration.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/58#discussion_r71371793
--- Diff: relational_operators/WorkOrder.proto ---
@@ -249,6 +249,7 @@ message WindowAggregationWorkOrder {
extend WorkOrder {
// All required
optional uint32 window_aggr_state_index = 336;
- optional int32 insert_destination_index = 337;
+ repeated fixed64 block_ids = 337;
--- End diff --
Let's leave it as this to match the order in the fields of `WindowAggregationWorkOrder`.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71402612
--- Diff: query_optimizer/tests/execution_generator/Select.test ---
@@ -953,19 +953,79 @@ WHERE double_col < 0
==
# Window Aggregation Test.
-# Currently this is not supported, an empty table will be returned.
-SELECT avg(int_col) OVER w FROM test
+SELECT char_col, long_col, avg(long_col) OVER w FROM test
WINDOW w AS
-(PARTITION BY char_col
- ORDER BY long_col DESC NULLS LAST
+(ORDER BY char_col DESC NULLS LAST
--- End diff --
So, any proposed changes?
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71222352
--- Diff: expressions/window_aggregation/CMakeLists.txt ---
@@ -0,0 +1,212 @@
+# Copyright 2011-2015 Quickstep Technologies LLC.
+# Copyright 2015 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.
+
+QS_PROTOBUF_GENERATE_CPP(expressions_windowaggregation_WindowAggregateFunction_proto_srcs
+ expressions_windowaggregation_WindowAggregateFunction_proto_hdrs
+ WindowAggregateFunction.proto)
+
+# Declare micro-libs:
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunction
+ WindowAggregateFunction.cpp
+ WindowAggregateFunction.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunction_proto
+ ${expressions_windowaggregation_WindowAggregateFunction_proto_srcs})
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionAvg
+ WindowAggregateFunctionAvg.cpp
+ WindowAggregateFunctionAvg.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionCount
+ WindowAggregateFunctionCount.cpp
+ WindowAggregateFunctionCount.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionMax
+ WindowAggregateFunctionMax.cpp
+ WindowAggregateFunctionMax.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionMin
+ WindowAggregateFunctionMin.cpp
+ WindowAggregateFunctionMin.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionSum
+ WindowAggregateFunctionSum.cpp
+ WindowAggregateFunctionSum.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionFactory
--- End diff --
Unfortunately, I did not see the proposed changes.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71250248
--- Diff: storage/WindowAggregationOperationState.cpp ---
@@ -176,4 +171,122 @@ bool WindowAggregationOperationState::ProtoIsValid(const serialization::WindowAg
return true;
}
+void WindowAggregationOperationState::windowAggregateBlocks(
+ InsertDestination *output_destination,
+ const std::vector<block_id> &block_ids) {
+ // TODO(Shixuan): This is a quick fix for currently unsupported functions in
+ // order to pass the query_optimizer test.
+ if (window_aggregation_handle_.get() == nullptr) {
+ std::cout << "The function will be supported in the near future :)\n";
+ return;
+ }
+
+ // TODO(Shixuan): RANGE frame mode should be supported to make SQL grammar
+ // work. This will need Order Key to be passed so that we know where the
+ // window should start and end.
+ if (!is_row_) {
+ std::cout << "Currently we don't support RANGE frame mode :(\n";
+ return;
+ }
+
+ // Get the total number of tuples.
+ int num_tuples = 0;
+ for (block_id block_idx : block_ids) {
+ num_tuples +=
+ storage_manager_->getBlock(block_idx, input_relation_)->getNumTuples();
+ }
+
+ // Construct column vectors for attributes.
+ std::vector<ColumnVector*> attribute_vecs;
+ for (std::size_t attr_id = 0; attr_id < input_relation_.size(); ++attr_id) {
+ const CatalogAttribute *attr = input_relation_.getAttributeById(attr_id);
+ const Type &type = attr->getType();
+
+ if (NativeColumnVector::UsableForType(type)) {
+ attribute_vecs.push_back(new NativeColumnVector(type, num_tuples));
+ } else {
+ attribute_vecs.push_back(new IndirectColumnVector(type, num_tuples));
+ }
+ }
+
+ // Construct column vectors for arguments.
+ std::vector<ColumnVector*> argument_vecs;
+ for (std::unique_ptr<const Scalar> &argument : arguments_) {
+ const Type &type = argument->getType();
+
+ if (NativeColumnVector::UsableForType(type)) {
+ argument_vecs.push_back(new NativeColumnVector(type, num_tuples));
+ } else {
+ argument_vecs.push_back(new IndirectColumnVector(type, num_tuples));
+ }
+ }
+
+ // TODO(Shixuan): Add Support for Vector Copy Elision Selection.
+ // Add tuples and arguments into ColumnVectors.
+ for (block_id block_idx : block_ids) {
--- End diff --
Add `const` for `block_idx`.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71232229
--- Diff: expressions/window_aggregation/tests/WindowAggregationHandleAvg_unittest.cpp ---
@@ -0,0 +1,398 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ * limitations under the License.
+ **/
+
+#include <cstddef>
+#include <cstdint>
+#include <memory>
+#include <vector>
+
+#include "catalog/CatalogTypedefs.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunction.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunctionFactory.hpp"
+#include "expressions/window_aggregation/WindowAggregationHandle.hpp"
+#include "expressions/window_aggregation/WindowAggregationHandleAvg.hpp"
+#include "expressions/window_aggregation/WindowAggregationID.hpp"
+#include "storage/ValueAccessor.hpp"
+#include "types/CharType.hpp"
+#include "types/DateOperatorOverloads.hpp"
+#include "types/DatetimeIntervalType.hpp"
+#include "types/DoubleType.hpp"
+#include "types/FloatType.hpp"
+#include "types/IntType.hpp"
+#include "types/IntervalLit.hpp"
+#include "types/LongType.hpp"
+#include "types/Type.hpp"
+#include "types/TypeFactory.hpp"
+#include "types/TypeID.hpp"
+#include "types/TypedValue.hpp"
+#include "types/VarCharType.hpp"
+#include "types/YearMonthIntervalType.hpp"
+#include "types/containers/ColumnVector.hpp"
+#include "types/containers/ColumnVectorsValueAccessor.hpp"
+
+#include "gtest/gtest.h"
+
+namespace quickstep {
+
+namespace {
+
+ constexpr int kNumTuples = 100;
+ constexpr int kNumTuplesPerPartition = 8;
+ constexpr int kNullInterval = 25;
+ constexpr int kNumPreceding = 2;
+ constexpr int kNumFollowing = 2;
+
+} // namespace
+
+// Attribute value could be null if set true.
+class WindowAggregationHandleAvgTest : public::testing::TestWithParam<bool> {
+ protected:
+ // Handle initialization.
+ void initializeHandle(const Type &argument_type) {
+ const WindowAggregateFunction &function =
+ WindowAggregateFunctionFactory::Get(WindowAggregationID::kAvg);
+ std::vector<const Type*> partition_key_types(1, &TypeFactory::GetType(kInt, false));
+ handle_avg_.reset(function.createHandle(std::vector<const Type*>(1, &argument_type),
+ std::move(partition_key_types)));
+ }
+
+ // Test canApplyToTypes().
+ static bool CanApplyToTypesTest(TypeID typeID) {
+ const Type &type = (typeID == kChar || typeID == kVarChar) ?
+ TypeFactory::GetType(typeID, static_cast<std::size_t>(10)) :
+ TypeFactory::GetType(typeID);
+
+ return WindowAggregateFunctionFactory::Get(WindowAggregationID::kAvg).canApplyToTypes(
+ std::vector<const Type*>(1, &type));
+ }
+
+ // Test resultTypeForArgumentTypes().
+ static bool ResultTypeForArgumentTypesTest(TypeID input_type_id,
+ TypeID output_type_id) {
--- End diff --
Code style: add one whitespace indentation.
---
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 #58: QUICKSTEP-20: Supported Window Aggregation fo...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on the issue:
https://github.com/apache/incubator-quickstep/pull/58
Sounds good. Thanks a lot! :)
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-quickstep/pull/58
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71020759
--- Diff: expressions/window_aggregation/CMakeLists.txt ---
@@ -0,0 +1,212 @@
+# Copyright 2011-2015 Quickstep Technologies LLC.
+# Copyright 2015 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.
+
+QS_PROTOBUF_GENERATE_CPP(expressions_windowaggregation_WindowAggregateFunction_proto_srcs
+ expressions_windowaggregation_WindowAggregateFunction_proto_hdrs
+ WindowAggregateFunction.proto)
+
+# Declare micro-libs:
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunction
+ WindowAggregateFunction.cpp
+ WindowAggregateFunction.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunction_proto
+ ${expressions_windowaggregation_WindowAggregateFunction_proto_srcs})
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionAvg
+ WindowAggregateFunctionAvg.cpp
+ WindowAggregateFunctionAvg.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionCount
+ WindowAggregateFunctionCount.cpp
+ WindowAggregateFunctionCount.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionMax
+ WindowAggregateFunctionMax.cpp
+ WindowAggregateFunctionMax.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionMin
+ WindowAggregateFunctionMin.cpp
+ WindowAggregateFunctionMin.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionSum
+ WindowAggregateFunctionSum.cpp
+ WindowAggregateFunctionSum.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionFactory
--- End diff --
Code style: please move it before `Max`.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/58#discussion_r71388812
--- Diff: query_optimizer/tests/execution_generator/Select.test ---
@@ -953,19 +953,79 @@ WHERE double_col < 0
==
# Window Aggregation Test.
-# Currently this is not supported, an empty table will be returned.
-SELECT avg(int_col) OVER w FROM test
+SELECT char_col, long_col, avg(long_col) OVER w FROM test
WINDOW w AS
-(PARTITION BY char_col
- ORDER BY long_col DESC NULLS LAST
+(ORDER BY char_col DESC NULLS LAST
--- End diff --
Ah I get what you are saying. In that case we don't actually need to do a sort since nothing will be changed, and the sort attribute will be empty (though now we don't pass the vector into the handle).
The sort attribute is not needed currently since we only support ROWS mode. To support RANGE mode, we have to add sort attribute to calculate the window frame. They will be introduced in the future PR.
Thanks for pointing that out :)
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71232741
--- Diff: expressions/window_aggregation/tests/WindowAggregationHandleAvg_unittest.cpp ---
@@ -0,0 +1,398 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ * limitations under the License.
+ **/
+
+#include <cstddef>
+#include <cstdint>
+#include <memory>
+#include <vector>
+
+#include "catalog/CatalogTypedefs.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunction.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunctionFactory.hpp"
+#include "expressions/window_aggregation/WindowAggregationHandle.hpp"
+#include "expressions/window_aggregation/WindowAggregationHandleAvg.hpp"
+#include "expressions/window_aggregation/WindowAggregationID.hpp"
+#include "storage/ValueAccessor.hpp"
+#include "types/CharType.hpp"
+#include "types/DateOperatorOverloads.hpp"
+#include "types/DatetimeIntervalType.hpp"
+#include "types/DoubleType.hpp"
+#include "types/FloatType.hpp"
+#include "types/IntType.hpp"
+#include "types/IntervalLit.hpp"
+#include "types/LongType.hpp"
+#include "types/Type.hpp"
+#include "types/TypeFactory.hpp"
+#include "types/TypeID.hpp"
+#include "types/TypedValue.hpp"
+#include "types/VarCharType.hpp"
+#include "types/YearMonthIntervalType.hpp"
+#include "types/containers/ColumnVector.hpp"
+#include "types/containers/ColumnVectorsValueAccessor.hpp"
+
+#include "gtest/gtest.h"
+
+namespace quickstep {
+
+namespace {
+
+ constexpr int kNumTuples = 100;
+ constexpr int kNumTuplesPerPartition = 8;
+ constexpr int kNullInterval = 25;
+ constexpr int kNumPreceding = 2;
+ constexpr int kNumFollowing = 2;
+
+} // namespace
+
+// Attribute value could be null if set true.
+class WindowAggregationHandleAvgTest : public::testing::TestWithParam<bool> {
+ protected:
+ // Handle initialization.
+ void initializeHandle(const Type &argument_type) {
+ const WindowAggregateFunction &function =
+ WindowAggregateFunctionFactory::Get(WindowAggregationID::kAvg);
+ std::vector<const Type*> partition_key_types(1, &TypeFactory::GetType(kInt, false));
+ handle_avg_.reset(function.createHandle(std::vector<const Type*>(1, &argument_type),
+ std::move(partition_key_types)));
+ }
+
+ // Test canApplyToTypes().
+ static bool CanApplyToTypesTest(TypeID typeID) {
+ const Type &type = (typeID == kChar || typeID == kVarChar) ?
+ TypeFactory::GetType(typeID, static_cast<std::size_t>(10)) :
+ TypeFactory::GetType(typeID);
+
+ return WindowAggregateFunctionFactory::Get(WindowAggregationID::kAvg).canApplyToTypes(
+ std::vector<const Type*>(1, &type));
+ }
+
+ // Test resultTypeForArgumentTypes().
+ static bool ResultTypeForArgumentTypesTest(TypeID input_type_id,
+ TypeID output_type_id) {
+ const Type *result_type
+ = WindowAggregateFunctionFactory::Get(WindowAggregationID::kAvg).resultTypeForArgumentTypes(
+ std::vector<const Type*>(1, &TypeFactory::GetType(input_type_id)));
+ return (result_type->getTypeID() == output_type_id);
+ }
+
+ template <typename CppType>
+ static void CheckAvgValues(
+ const std::vector<CppType*> &expected,
+ const ColumnVector *actual) {
+ EXPECT_TRUE(actual->isNative());
+ const NativeColumnVector *native = static_cast<const NativeColumnVector*>(actual);
+
+ EXPECT_EQ(expected.size(), native->size());
+ for (std::size_t i = 0; i < expected.size(); ++i) {
+ if (expected[i] == nullptr) {
+ EXPECT_TRUE(native->getTypedValue(i).isNull());
+ } else {
+ EXPECT_EQ(*expected[i], native->getTypedValue(i).getLiteral<CppType>());
+ }
+ }
+ }
+
+ // Static templated methods for set a meaningful value to data types.
+ template <typename CppType>
+ static void SetDataType(int value, CppType *data) {
+ *data = value;
+ }
+
+ template <typename GenericType, typename OutputType = DoubleType>
+ void checkWindowAggregationAvgGeneric() {
+ const GenericType &type = GenericType::Instance(true);
+ initializeHandle(type);
+
+ // Create argument, partition key and cpptype vectors.
+ std::vector<typename GenericType::cpptype*> argument_cpp_vector;
+ argument_cpp_vector.reserve(kNumTuples);
+ ColumnVector *argument_type_vector =
+ createArgumentGeneric<GenericType>(&argument_cpp_vector);
+ NativeColumnVector *partition_key_vector =
+ new NativeColumnVector(IntType::InstanceNonNullable(), kNumTuples + 2);
+
+ for (int i = 0; i < kNumTuples; ++i) {
+ partition_key_vector->appendTypedValue(TypedValue(i / kNumTuplesPerPartition));
+ }
+
+ // Create tuple ValueAccessor.
+ ColumnVectorsValueAccessor *tuple_accessor = new ColumnVectorsValueAccessor();
+ tuple_accessor->addColumn(partition_key_vector);
+ tuple_accessor->addColumn(argument_type_vector);
+
+ // Test UNBOUNDED PRECEDING AND CURRENT ROW.
+ checkAccumulate<GenericType, OutputType>(tuple_accessor,
+ argument_type_vector,
+ argument_cpp_vector);
+ // Test kNumPreceding PRECEDING AND kNumFollowing FOLLOWING.
+ checkSlidingWindow<GenericType, OutputType>(tuple_accessor,
+ argument_type_vector,
+ argument_cpp_vector);
+ }
+
+ template <typename GenericType>
+ ColumnVector *createArgumentGeneric(
+ std::vector<typename GenericType::cpptype*> *argument_cpp_vector) {
+ const GenericType &type = GenericType::Instance(true);
+ NativeColumnVector *column = new NativeColumnVector(type, kNumTuples);
+
+ for (int i = 0; i < kNumTuples; ++i) {
+ // Insert a NULL every kNullInterval tuples.
+ if (i % kNullInterval == 0) {
+ argument_cpp_vector->push_back(nullptr);
+ column->appendTypedValue(type.makeNullValue());
+ continue;
+ }
+
+ typename GenericType::cpptype *val = new typename GenericType::cpptype;
+
+ if (type.getTypeID() == kInt || type.getTypeID() == kLong) {
+ SetDataType(i - 10, val);
+ } else {
+ SetDataType(static_cast<float>(i - 10) / 10, val);
+ }
+
+ column->appendTypedValue(type.makeValue(val));
+ argument_cpp_vector->push_back(val);
+ }
+
+ return column;
+ }
+
+ template <typename GenericType, typename OutputType>
+ void checkAccumulate(ColumnVectorsValueAccessor *tuple_accessor,
+ ColumnVector *argument_type_vector,
+ const std::vector<typename GenericType::cpptype*> &argument_cpp_vector) {
+ std::vector<ColumnVector*> arguments;
+ arguments.push_back(argument_type_vector);
+ // The partition key index is 0.
+ std::vector<attribute_id> partition_key(1, 0);
+
+ ColumnVector *result =
+ handle_avg_->calculate(tuple_accessor,
+ std::move(arguments),
+ partition_key,
+ true /* is_row */,
+ -1 /* num_preceding: UNBOUNDED PRECEDING */,
+ 0 /* num_following: CURRENT ROW */);
+
+ // Get the cpptype result.
+ std::vector<typename OutputType::cpptype*> result_cpp_vector;
+ bool is_null;
+ typename GenericType::cpptype sum;
+ int count;
+ for (std::size_t i = 0; i < argument_cpp_vector.size(); ++i) {
+ // Start of new partition
+ if (i % kNumTuplesPerPartition == 0) {
+ is_null = false;
+ SetDataType(0, &sum);
+ count = 0;
+ }
+
+ typename GenericType::cpptype *value = argument_cpp_vector[i];
+ if (value == nullptr) {
+ is_null = true;
+ }
+
+ if (is_null) {
+ result_cpp_vector.push_back(nullptr);
+ } else {
+ sum += *value;
+ count++;
+ typename OutputType::cpptype *result_cpp_value =
+ new typename OutputType::cpptype;
+ *result_cpp_value = static_cast<typename OutputType::cpptype>(sum) / count;
+ result_cpp_vector.push_back(result_cpp_value);
+ }
+ }
+
+ CheckAvgValues(result_cpp_vector, result);
+ }
+
+ template <typename GenericType, typename OutputType>
+ void checkSlidingWindow(ColumnVectorsValueAccessor *tuple_accessor,
+ ColumnVector *argument_type_vector,
+ const std::vector<typename GenericType::cpptype*> &argument_cpp_vector) {
+ std::vector<ColumnVector*> arguments;
+ arguments.push_back(argument_type_vector);
+ // The partition key index is 0.
+ std::vector<attribute_id> partition_key(1, 0);
+
+ ColumnVector *result =
+ handle_avg_->calculate(tuple_accessor,
+ std::move(arguments),
+ partition_key,
+ true /* is_row */,
+ kNumPreceding,
+ kNumFollowing);
+
+ // Get the cpptype result.
+ // For each value, calculate all surrounding values in the window.
+ std::vector<typename OutputType::cpptype*> result_cpp_vector;
+
+ for (std::size_t i = 0; i < argument_cpp_vector.size(); ++i) {
+ if (argument_cpp_vector[i] == nullptr) {
+ result_cpp_vector.push_back(nullptr);
+ continue;
+ }
+
+ typename GenericType::cpptype sum;
+ SetDataType(0, &sum);
+ sum += *argument_cpp_vector[i];
+ int count = 1;
+ bool is_null = false;
+
+ for (std::size_t precede = 1; precede <= kNumPreceding; ++precede) {
+ // Not the same partition.
+ if (i / kNumTuplesPerPartition != (i - precede) / kNumTuplesPerPartition
--- End diff --
[Code style](https://google.github.io/styleguide/cppguide.html#Boolean_Expressions):
Please refactor to
```
if (xxx ||
yyy)
```
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71246020
--- Diff: relational_operators/WorkOrder.proto ---
@@ -249,6 +249,7 @@ message WindowAggregationWorkOrder {
extend WorkOrder {
// All required
optional uint32 window_aggr_state_index = 336;
- optional int32 insert_destination_index = 337;
+ repeated fixed64 block_ids = 337;
--- End diff --
FYI, we don't need to change the number assigned to `insert_destination_index`, but just add a new field for `block_ids` while assigning `338`.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71244117
--- Diff: relational_operators/WindowAggregationOperator.hpp ---
@@ -58,16 +58,19 @@ class WindowAggregationOperator : public RelationalOperator {
*
* @param query_id The ID of this query.
* @param input_relation The relation to perform aggregation over.
+ * @param output_relation The relation for output.
* @param window_aggregation_state_index The index of WindowAggregationState
* in QueryContext.
* @param output_destination_index The index of InsertDestination in
* QueryContext for the output.
**/
WindowAggregationOperator(const std::size_t query_id,
+ const CatalogRelation &input_relation,
--- End diff --
Remove this 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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71369255
--- Diff: relational_operators/WorkOrder.proto ---
@@ -249,6 +249,7 @@ message WindowAggregationWorkOrder {
extend WorkOrder {
// All required
optional uint32 window_aggr_state_index = 336;
- optional int32 insert_destination_index = 337;
+ repeated fixed64 block_ids = 337;
--- End diff --
So, do we want to refactor this changes?
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71383605
--- Diff: query_optimizer/tests/execution_generator/Select.test ---
@@ -953,19 +953,79 @@ WHERE double_col < 0
==
# Window Aggregation Test.
-# Currently this is not supported, an empty table will be returned.
-SELECT avg(int_col) OVER w FROM test
+SELECT char_col, long_col, avg(long_col) OVER w FROM test
WINDOW w AS
-(PARTITION BY char_col
- ORDER BY long_col DESC NULLS LAST
+(ORDER BY char_col DESC NULLS LAST
--- End diff --
I mean, we print the aggregation result according to the tuple order in the input table.
To that end, we may use a tuple id as the sort attribute.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71229020
--- Diff: expressions/window_aggregation/WindowAggregateFunctionMax.hpp ---
@@ -0,0 +1,75 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ **/
+
+#ifndef QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_MAX_HPP_
+#define QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_MAX_HPP_
+
+#include <string>
+#include <vector>
+
+#include "expressions/window_aggregation/WindowAggregateFunction.hpp"
+#include "expressions/window_aggregation/WindowAggregationID.hpp"
+#include "utility/Macros.hpp"
+
+namespace quickstep {
+
+class WindowAggregationHandle;
+class Type;
--- End diff --
Code style: reorder the declaration.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/58#discussion_r71582584
--- Diff: query_optimizer/tests/execution_generator/Select.test ---
@@ -953,19 +953,79 @@ WHERE double_col < 0
==
# Window Aggregation Test.
-# Currently this is not supported, an empty table will be returned.
-SELECT avg(int_col) OVER w FROM test
+SELECT char_col, long_col, avg(long_col) OVER w FROM test
WINDOW w AS
-(PARTITION BY char_col
- ORDER BY long_col DESC NULLS LAST
+(ORDER BY char_col DESC NULLS LAST
--- End diff --
@zuyu I found that the code logic here is a little bit wrong. For window w/ NULLs, the NULL values should be ignored rather than directly mark the result NULL. Only when all the values in the window are NULLs should the window aggregate result be marked NULL.
Will fix this in the next 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.
---
[GitHub] incubator-quickstep pull request #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/58#discussion_r71360169
--- Diff: storage/WindowAggregationOperationState.cpp ---
@@ -55,14 +62,12 @@ WindowAggregationOperationState::WindowAggregationOperationState(
StorageManager *storage_manager)
: input_relation_(input_relation),
arguments_(std::move(arguments)),
- partition_by_attributes_(std::move(partition_by_attributes)),
--- End diff --
We will use attribute_id instead of Scalar.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71250159
--- Diff: storage/WindowAggregationOperationState.cpp ---
@@ -176,4 +171,122 @@ bool WindowAggregationOperationState::ProtoIsValid(const serialization::WindowAg
return true;
}
+void WindowAggregationOperationState::windowAggregateBlocks(
+ InsertDestination *output_destination,
+ const std::vector<block_id> &block_ids) {
+ // TODO(Shixuan): This is a quick fix for currently unsupported functions in
+ // order to pass the query_optimizer test.
+ if (window_aggregation_handle_.get() == nullptr) {
+ std::cout << "The function will be supported in the near future :)\n";
+ return;
+ }
+
+ // TODO(Shixuan): RANGE frame mode should be supported to make SQL grammar
+ // work. This will need Order Key to be passed so that we know where the
+ // window should start and end.
+ if (!is_row_) {
+ std::cout << "Currently we don't support RANGE frame mode :(\n";
+ return;
+ }
+
+ // Get the total number of tuples.
+ int num_tuples = 0;
+ for (block_id block_idx : block_ids) {
--- End diff --
Add `const` for `block_id block_idx`.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/58#discussion_r71372226
--- Diff: query_optimizer/tests/execution_generator/Select.test ---
@@ -953,19 +953,79 @@ WHERE double_col < 0
==
# Window Aggregation Test.
-# Currently this is not supported, an empty table will be returned.
-SELECT avg(int_col) OVER w FROM test
+SELECT char_col, long_col, avg(long_col) OVER w FROM test
WINDOW w AS
-(PARTITION BY char_col
- ORDER BY long_col DESC NULLS LAST
+(ORDER BY char_col DESC NULLS LAST
--- End diff --
I don't quite understand. Could you please clarify this?
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71250408
--- Diff: storage/WindowAggregationOperationState.proto ---
@@ -19,15 +19,15 @@ syntax = "proto2";
package quickstep.serialization;
-import "expressions/aggregation/AggregateFunction.proto";
+import "expressions/window_aggregation/WindowAggregateFunction.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; // -1 means UNBOUNDED PRECEDING.
- required int64 num_following = 7; // -1 means UNBOUNDED FOLLOWING.
+ required int32 input_relation_id = 1;
+ required WindowAggregateFunction function = 3;
--- End diff --
Something wrong w/ the number assignments and below.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71244079
--- Diff: relational_operators/WindowAggregationOperator.cpp ---
@@ -67,6 +73,11 @@ serialization::WorkOrder* WindowAggregationOperator::createWorkOrderProto() {
proto->set_query_id(query_id_);
proto->SetExtension(serialization::WindowAggregationWorkOrder::window_aggr_state_index,
window_aggregation_state_index_);
+
+ std::vector<block_id> relation_blocks = input_relation_.getBlocksSnapshot();
--- End diff --
We could do some improvements on the code:
```
const std::vector<block_id> relation_blocks = input_relation_.getBlocksSnapshot();
for (const block_id bid : relation_blocks) { ... }
```
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/58#discussion_r71031464
--- Diff: expressions/window_aggregation/CMakeLists.txt ---
@@ -0,0 +1,212 @@
+# Copyright 2011-2015 Quickstep Technologies LLC.
+# Copyright 2015 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.
+
+QS_PROTOBUF_GENERATE_CPP(expressions_windowaggregation_WindowAggregateFunction_proto_srcs
+ expressions_windowaggregation_WindowAggregateFunction_proto_hdrs
+ WindowAggregateFunction.proto)
+
+# Declare micro-libs:
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunction
+ WindowAggregateFunction.cpp
+ WindowAggregateFunction.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunction_proto
+ ${expressions_windowaggregation_WindowAggregateFunction_proto_srcs})
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionAvg
+ WindowAggregateFunctionAvg.cpp
+ WindowAggregateFunctionAvg.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionCount
+ WindowAggregateFunctionCount.cpp
+ WindowAggregateFunctionCount.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionMax
+ WindowAggregateFunctionMax.cpp
+ WindowAggregateFunctionMax.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionMin
+ WindowAggregateFunctionMin.cpp
+ WindowAggregateFunctionMin.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionSum
+ WindowAggregateFunctionSum.cpp
+ WindowAggregateFunctionSum.hpp)
+add_library(quickstep_expressions_windowaggregation_WindowAggregateFunctionFactory
--- End diff --
Fixed! :)
---
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 #58: QUICKSTEP-20: Supported Window Aggregation fo...
Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on the issue:
https://github.com/apache/incubator-quickstep/pull/58
@shixuan-fan I've done one pass. Let me know if you want another.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71250202
--- Diff: storage/WindowAggregationOperationState.cpp ---
@@ -176,4 +171,122 @@ bool WindowAggregationOperationState::ProtoIsValid(const serialization::WindowAg
return true;
}
+void WindowAggregationOperationState::windowAggregateBlocks(
+ InsertDestination *output_destination,
+ const std::vector<block_id> &block_ids) {
+ // TODO(Shixuan): This is a quick fix for currently unsupported functions in
+ // order to pass the query_optimizer test.
+ if (window_aggregation_handle_.get() == nullptr) {
+ std::cout << "The function will be supported in the near future :)\n";
+ return;
+ }
+
+ // TODO(Shixuan): RANGE frame mode should be supported to make SQL grammar
+ // work. This will need Order Key to be passed so that we know where the
+ // window should start and end.
+ if (!is_row_) {
+ std::cout << "Currently we don't support RANGE frame mode :(\n";
+ return;
+ }
+
+ // Get the total number of tuples.
+ int num_tuples = 0;
+ for (block_id block_idx : block_ids) {
+ num_tuples +=
+ storage_manager_->getBlock(block_idx, input_relation_)->getNumTuples();
+ }
+
+ // Construct column vectors for attributes.
+ std::vector<ColumnVector*> attribute_vecs;
+ for (std::size_t attr_id = 0; attr_id < input_relation_.size(); ++attr_id) {
+ const CatalogAttribute *attr = input_relation_.getAttributeById(attr_id);
+ const Type &type = attr->getType();
+
+ if (NativeColumnVector::UsableForType(type)) {
+ attribute_vecs.push_back(new NativeColumnVector(type, num_tuples));
+ } else {
+ attribute_vecs.push_back(new IndirectColumnVector(type, num_tuples));
+ }
+ }
+
+ // Construct column vectors for arguments.
+ std::vector<ColumnVector*> argument_vecs;
+ for (std::unique_ptr<const Scalar> &argument : arguments_) {
--- End diff --
Add `const` for `std::unique_ptr<const Scalar> &argument`.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71034269
--- Diff: expressions/window_aggregation/WindowAggregateFunction.hpp ---
@@ -0,0 +1,149 @@
+/**
+ * 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.
+ **/
+
+#ifndef QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_HPP_
+#define QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_HPP_
+
+#include <string>
+#include <vector>
+
+#include "catalog/CatalogTypedefs.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunction.pb.h"
+#include "expressions/window_aggregation/WindowAggregationID.hpp"
+#include "storage/StorageBlockInfo.hpp"
+#include "utility/Macros.hpp"
+
+namespace quickstep {
+
+class CatalogRelationSchema;
+class WindowAggregationHandle;
+class Type;
--- End diff --
Code style: Reorder declarations.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71246129
--- Diff: storage/WindowAggregationOperationState.cpp ---
@@ -55,14 +62,12 @@ WindowAggregationOperationState::WindowAggregationOperationState(
StorageManager *storage_manager)
: input_relation_(input_relation),
arguments_(std::move(arguments)),
- partition_by_attributes_(std::move(partition_by_attributes)),
--- End diff --
Question: are we removing the support for the partitioned tuples?
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/58#discussion_r71358314
--- Diff: query_optimizer/tests/execution_generator/Select.test ---
@@ -953,19 +953,79 @@ WHERE double_col < 0
==
# Window Aggregation Test.
-# Currently this is not supported, an empty table will be returned.
-SELECT avg(int_col) OVER w FROM test
+SELECT char_col, long_col, avg(long_col) OVER w FROM test
WINDOW w AS
-(PARTITION BY char_col
- ORDER BY long_col DESC NULLS LAST
+(ORDER BY char_col DESC NULLS LAST
--- End diff --
Then the table will be in the same order as the input table when the window aggregates are calculated. And the calculated window aggregate for each row might depend on different values.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71232384
--- Diff: expressions/window_aggregation/tests/WindowAggregationHandleAvg_unittest.cpp ---
@@ -0,0 +1,398 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ * limitations under the License.
+ **/
+
+#include <cstddef>
+#include <cstdint>
+#include <memory>
+#include <vector>
+
+#include "catalog/CatalogTypedefs.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunction.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunctionFactory.hpp"
+#include "expressions/window_aggregation/WindowAggregationHandle.hpp"
+#include "expressions/window_aggregation/WindowAggregationHandleAvg.hpp"
+#include "expressions/window_aggregation/WindowAggregationID.hpp"
+#include "storage/ValueAccessor.hpp"
+#include "types/CharType.hpp"
+#include "types/DateOperatorOverloads.hpp"
+#include "types/DatetimeIntervalType.hpp"
+#include "types/DoubleType.hpp"
+#include "types/FloatType.hpp"
+#include "types/IntType.hpp"
+#include "types/IntervalLit.hpp"
+#include "types/LongType.hpp"
+#include "types/Type.hpp"
+#include "types/TypeFactory.hpp"
+#include "types/TypeID.hpp"
+#include "types/TypedValue.hpp"
+#include "types/VarCharType.hpp"
+#include "types/YearMonthIntervalType.hpp"
+#include "types/containers/ColumnVector.hpp"
+#include "types/containers/ColumnVectorsValueAccessor.hpp"
+
+#include "gtest/gtest.h"
+
+namespace quickstep {
+
+namespace {
+
+ constexpr int kNumTuples = 100;
+ constexpr int kNumTuplesPerPartition = 8;
+ constexpr int kNullInterval = 25;
+ constexpr int kNumPreceding = 2;
+ constexpr int kNumFollowing = 2;
+
+} // namespace
+
+// Attribute value could be null if set true.
+class WindowAggregationHandleAvgTest : public::testing::TestWithParam<bool> {
+ protected:
+ // Handle initialization.
+ void initializeHandle(const Type &argument_type) {
+ const WindowAggregateFunction &function =
+ WindowAggregateFunctionFactory::Get(WindowAggregationID::kAvg);
+ std::vector<const Type*> partition_key_types(1, &TypeFactory::GetType(kInt, false));
+ handle_avg_.reset(function.createHandle(std::vector<const Type*>(1, &argument_type),
+ std::move(partition_key_types)));
+ }
+
+ // Test canApplyToTypes().
+ static bool CanApplyToTypesTest(TypeID typeID) {
+ const Type &type = (typeID == kChar || typeID == kVarChar) ?
+ TypeFactory::GetType(typeID, static_cast<std::size_t>(10)) :
+ TypeFactory::GetType(typeID);
+
+ return WindowAggregateFunctionFactory::Get(WindowAggregationID::kAvg).canApplyToTypes(
+ std::vector<const Type*>(1, &type));
+ }
+
+ // Test resultTypeForArgumentTypes().
+ static bool ResultTypeForArgumentTypesTest(TypeID input_type_id,
+ TypeID output_type_id) {
+ const Type *result_type
+ = WindowAggregateFunctionFactory::Get(WindowAggregationID::kAvg).resultTypeForArgumentTypes(
+ std::vector<const Type*>(1, &TypeFactory::GetType(input_type_id)));
+ return (result_type->getTypeID() == output_type_id);
+ }
+
+ template <typename CppType>
+ static void CheckAvgValues(
+ const std::vector<CppType*> &expected,
+ const ColumnVector *actual) {
+ EXPECT_TRUE(actual->isNative());
+ const NativeColumnVector *native = static_cast<const NativeColumnVector*>(actual);
+
+ EXPECT_EQ(expected.size(), native->size());
+ for (std::size_t i = 0; i < expected.size(); ++i) {
+ if (expected[i] == nullptr) {
+ EXPECT_TRUE(native->getTypedValue(i).isNull());
+ } else {
+ EXPECT_EQ(*expected[i], native->getTypedValue(i).getLiteral<CppType>());
+ }
+ }
+ }
+
+ // Static templated methods for set a meaningful value to data types.
+ template <typename CppType>
+ static void SetDataType(int value, CppType *data) {
+ *data = value;
+ }
+
+ template <typename GenericType, typename OutputType = DoubleType>
+ void checkWindowAggregationAvgGeneric() {
+ const GenericType &type = GenericType::Instance(true);
+ initializeHandle(type);
+
+ // Create argument, partition key and cpptype vectors.
+ std::vector<typename GenericType::cpptype*> argument_cpp_vector;
+ argument_cpp_vector.reserve(kNumTuples);
+ ColumnVector *argument_type_vector =
+ createArgumentGeneric<GenericType>(&argument_cpp_vector);
+ NativeColumnVector *partition_key_vector =
+ new NativeColumnVector(IntType::InstanceNonNullable(), kNumTuples + 2);
--- End diff --
Code style: add two more whitespace indentation.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71233338
--- Diff: query_optimizer/resolver/Resolver.cpp ---
@@ -2656,11 +2666,12 @@ E::ScalarPtr Resolver::resolveFunctionCall(
}
// Make sure a naked COUNT() with no arguments wasn't specified.
- if ((aggregate->getAggregationID() == AggregationID::kCount)
- && (resolved_arguments.empty())
- && (!count_star)) {
- THROW_SQL_ERROR_AT(&parse_function_call)
- << "COUNT aggregate requires an argument (either scalar or star (*))";
+ if ((aggregate != nullptr && aggregate->getAggregationID() == AggregationID::kCount)
--- End diff --
Code style for the bool expressions:
https://google.github.io/styleguide/cppguide.html#Boolean_Expressions
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71226022
--- Diff: expressions/window_aggregation/WindowAggregateFunctionAvg.cpp ---
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 "expressions/window_aggregation/WindowAggregateFunctionAvg.hpp"
+
+#include <vector>
+
+#include "expressions/window_aggregation/WindowAggregationHandleAvg.hpp"
+#include "types/Type.hpp"
+#include "types/TypeFactory.hpp"
+#include "types/TypeID.hpp"
+#include "types/operations/binary_operations/BinaryOperation.hpp"
+#include "types/operations/binary_operations/BinaryOperationFactory.hpp"
+#include "types/operations/binary_operations/BinaryOperationID.hpp"
+
+#include "glog/logging.h"
+
+namespace quickstep {
+
+bool WindowAggregateFunctionAvg::canApplyToTypes(
+ const std::vector<const Type*> &argument_types) const {
+ // AVG is unary.
+ if (argument_types.size() != 1) {
+ return false;
+ }
+
+ // Argument must be addable and divisible.
+ return BinaryOperationFactory::GetBinaryOperation(BinaryOperationID::kAdd)
--- End diff --
[Code style](https://google.github.io/styleguide/cppguide.html#Return_Values):
Please refactor to
```
return xxx &&
yyy;
```
---
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 #58: QUICKSTEP-20: Supported Window Aggregation fo...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on the issue:
https://github.com/apache/incubator-quickstep/pull/58
@zuyu I have a new version on my local repo which supports `RANGE`, and also optimizes the calculation from quadratic to linear. Could we have this one merged so that I could open another PR for the new feature? Thanks!
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/58#discussion_r71403917
--- Diff: query_optimizer/tests/execution_generator/Select.test ---
@@ -953,19 +953,79 @@ WHERE double_col < 0
==
# Window Aggregation Test.
-# Currently this is not supported, an empty table will be returned.
-SELECT avg(int_col) OVER w FROM test
+SELECT char_col, long_col, avg(long_col) OVER w FROM test
WINDOW w AS
-(PARTITION BY char_col
- ORDER BY long_col DESC NULLS LAST
+(ORDER BY char_col DESC NULLS LAST
--- End diff --
When the `RANGE` mode is introduced, a new parameter will be added to the `WindowAggregationHandle::calculate()` method, and would most likely be `const std::vector<attribute_id> &order_by_ids`. As for the cases where `ORDER BY` clause is missing, the vector would simply be empty, which indicates that the table is sorted by `tuple_id`.
I don't think we need to explicitly add `tuple_id` as a sort attribute, because it is not actually an attribute.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71226594
--- Diff: expressions/window_aggregation/WindowAggregateFunctionCount.hpp ---
@@ -0,0 +1,75 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ **/
+
+#ifndef QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_COUNT_HPP_
+#define QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_COUNT_HPP_
+
+#include <string>
+#include <vector>
+
+#include "expressions/window_aggregation/WindowAggregateFunction.hpp"
+#include "expressions/window_aggregation/WindowAggregationID.hpp"
+#include "utility/Macros.hpp"
+
+namespace quickstep {
+
+class WindowAggregationHandle;
+class Type;
--- End diff --
Code style: Declaration ordering.
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71243543
--- Diff: query_optimizer/tests/execution_generator/Select.test ---
@@ -953,19 +953,79 @@ WHERE double_col < 0
==
# Window Aggregation Test.
-# Currently this is not supported, an empty table will be returned.
-SELECT avg(int_col) OVER w FROM test
+SELECT char_col, long_col, avg(long_col) OVER w FROM test
WINDOW w AS
-(PARTITION BY char_col
- ORDER BY long_col DESC NULLS LAST
+(ORDER BY char_col DESC NULLS LAST
--- End diff --
Quick question: What if we remove the `order by` clause?
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
Posted by shixuan-fan <gi...@git.apache.org>.
Github user shixuan-fan commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/58#discussion_r71039570
--- Diff: expressions/window_aggregation/WindowAggregateFunction.hpp ---
@@ -0,0 +1,149 @@
+/**
+ * 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.
+ **/
+
+#ifndef QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_HPP_
+#define QUICKSTEP_EXPRESSIONS_WINDOW_AGGREGATION_WINDOW_AGGREGATE_FUNCTION_HPP_
+
+#include <string>
+#include <vector>
+
+#include "catalog/CatalogTypedefs.hpp"
+#include "expressions/window_aggregation/WindowAggregateFunction.pb.h"
+#include "expressions/window_aggregation/WindowAggregationID.hpp"
+#include "storage/StorageBlockInfo.hpp"
+#include "utility/Macros.hpp"
+
+namespace quickstep {
+
+class CatalogRelationSchema;
+class WindowAggregationHandle;
+class Type;
--- End diff --
Done!
---
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 #58: QUICKSTEP-20: Supported Window Aggrega...
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/58#discussion_r71226487
--- Diff: expressions/window_aggregation/WindowAggregateFunctionCount.cpp ---
@@ -0,0 +1,59 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 "expressions/window_aggregation/WindowAggregateFunctionCount.hpp"
+
+#include <vector>
+
+#include "expressions/window_aggregation/WindowAggregationHandle.hpp"
+#include "types/Type.hpp"
+#include "types/TypeFactory.hpp"
+#include "types/TypeID.hpp"
+
+#include "glog/logging.h"
+
+namespace quickstep {
+
+bool WindowAggregateFunctionCount::canApplyToTypes(
+ const std::vector<const Type*> &argument_types) const {
+ // COUNT may be nullary (i.e. COUNT(*)) or unary.
+ return argument_types.size() <= 1;
+}
+
+const Type* WindowAggregateFunctionCount::resultTypeForArgumentTypes(
+ const std::vector<const Type*> &argument_types) const {
+ if (!canApplyToTypes(argument_types)) {
+ return nullptr;
+ }
+
+ return &TypeFactory::GetType(kLong);
+}
+
+WindowAggregationHandle* WindowAggregateFunctionCount::createHandle(
+ std::vector<const Type*> &&argument_types,
--- End diff --
I am curious why we use the rvalue 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.
---