[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-23 Thread cramja
Github user cramja commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112835858 --- Diff: cli/Flags.cpp --- @@ -87,4 +87,25 @@ DEFINE_bool(preload_buffer_pool, false, "accepting queries (should also set --buff

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-23 Thread cramja
Github user cramja commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112835877 --- Diff: cli/Flags.cpp --- @@ -87,4 +87,25 @@ DEFINE_bool(preload_buffer_pool, false, "accepting queries (should also set --buff

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-23 Thread cramja
Github user cramja commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112836065 --- Diff: cli/CMakeLists.txt --- @@ -15,7 +15,6 @@ # specific language governing permissions and limitations # under the License.

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-23 Thread cramja
Github user cramja commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112836146 --- Diff: cli/IOInterface.hpp --- @@ -0,0 +1,82 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more con

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-23 Thread cramja
Github user cramja commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112836342 --- Diff: cli/IOInterface.hpp --- @@ -0,0 +1,82 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more con

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-23 Thread cramja
Github user cramja commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112836622 --- Diff: cli/NetworkCliClientMain.cpp --- @@ -0,0 +1,62 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-23 Thread cramja
Github user cramja commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112836758 --- Diff: cli/NetworkIO.hpp --- @@ -0,0 +1,286 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more cont

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-23 Thread cramja
Github user cramja commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112837404 --- Diff: cli/NetworkIO.hpp --- @@ -0,0 +1,286 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more cont

[GitHub] incubator-quickstep issue #232: QUICKSTEP-87 Adds network cli interface.

2017-04-23 Thread cramja
Github user cramja commented on the issue: https://github.com/apache/incubator-quickstep/pull/232 @zuyu Thank you for review. I addressed the minor stuff and will look into Async when I have more time this coming week. --- If your project is set up for it, you can reply to this email

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112842641 --- Diff: cli/Flags.cpp --- @@ -87,4 +87,25 @@ DEFINE_bool(preload_buffer_pool, false, "accepting queries (should also set --buffer

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112842721 --- Diff: cli/NetworkIO.hpp --- @@ -0,0 +1,286 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contri

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/incubator-quickstep/pull/237 --- 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 fe

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112828079 --- Diff: expressions/scalar/tests/ScalarCaseExpression_unittest.cpp --- @@ -223,8 +223,8 @@ TEST_F(ScalarCaseExpressionTest, BasicComparisonAndLit

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112826903 --- Diff: expressions/scalar/Scalar.hpp --- @@ -55,6 +59,7 @@ class Scalar { kUnaryExpression, kBinaryExpression, kCaseEx

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112828368 --- Diff: query_optimizer/expressions/Scalar.hpp --- @@ -65,10 +67,49 @@ class Scalar : public Expression { const std::unordered_map& su

[GitHub] incubator-quickstep issue #237: QUICKSTEP-89 Add support for common subexpre...

2017-04-23 Thread zuyu
Github user zuyu commented on the issue: https://github.com/apache/incubator-quickstep/pull/237 @jianqiao This PR has a bug and some issues to fix. --- 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

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112828399 --- Diff: query_optimizer/expressions/Scalar.hpp --- @@ -65,10 +67,49 @@ class Scalar : public Expression { const std::unordered_map& su

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112826746 --- Diff: expressions/predicate/Predicate.hpp --- @@ -67,6 +72,11 @@ class Predicate { virtual ~Predicate() { --- End diff -- Ne

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112845784 --- Diff: query_optimizer/expressions/AttributeReference.cpp --- @@ -57,6 +60,22 @@ ::quickstep::Scalar *AttributeReference::concretize( retu

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112826967 --- Diff: expressions/scalar/ScalarAttribute.cpp --- @@ -139,25 +142,26 @@ ColumnVector* ScalarAttribute::getAllValues(ValueAccessor *accessor,

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112827855 --- Diff: expressions/scalar/ScalarSharedExpression.cpp --- @@ -0,0 +1,141 @@ +/** + * Licensed to the Apache Software Foundation (ASF) unde

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112826924 --- Diff: expressions/scalar/Scalar.hpp --- @@ -70,6 +75,11 @@ class Scalar { virtual ~Scalar() { --- End diff -- Change to `~Sc

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112827868 --- Diff: expressions/scalar/ScalarSharedExpression.cpp --- @@ -0,0 +1,141 @@ +/** + * Licensed to the Apache Software Foundation (ASF) unde

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112827764 --- Diff: expressions/scalar/ScalarCaseExpression.cpp --- @@ -420,15 +429,15 @@ void ScalarCaseExpression::MultiplexNativeColumnVector( void S

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112828010 --- Diff: expressions/scalar/ScalarSharedExpression.hpp --- @@ -0,0 +1,127 @@ +/** + * Licensed to the Apache Software Foundation (ASF) unde

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112827934 --- Diff: expressions/scalar/ScalarSharedExpression.cpp --- @@ -0,0 +1,141 @@ +/** + * Licensed to the Apache Software Foundation (ASF) unde

[GitHub] incubator-quickstep issue #237: QUICKSTEP-89 Add support for common subexpre...

2017-04-23 Thread pateljm
Github user pateljm commented on the issue: https://github.com/apache/incubator-quickstep/pull/237 Excellent work @jianqiao. LGTM. Merging. Side note: One of the jobs timed out, so we should consider removing some of the tests from the type system once you are done with the ne

[GitHub] incubator-quickstep issue #237: QUICKSTEP-89 Add support for common subexpre...

2017-04-23 Thread pateljm
Github user pateljm commented on the issue: https://github.com/apache/incubator-quickstep/pull/237 @zuyu -- can you expand on the bugs? The tests all passed (except for the one that timed out). --- If your project is set up for it, you can reply to this email and have your reply appe

[GitHub] incubator-quickstep issue #237: QUICKSTEP-89 Add support for common subexpre...

2017-04-23 Thread zuyu
Github user zuyu commented on the issue: https://github.com/apache/incubator-quickstep/pull/237 @pateljm This is a minor bug not caught by the tests. See [the comment](https://github.com/apache/incubator-quickstep/pull/237#discussion_r112826903). --- If your project is set up for it,

[GitHub] incubator-quickstep issue #237: QUICKSTEP-89 Add support for common subexpre...

2017-04-23 Thread jianqiao
Github user jianqiao commented on the issue: https://github.com/apache/incubator-quickstep/pull/237 Will fix the issues in one PR shortly after. --- 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 hav

[GitHub] incubator-quickstep issue #237: QUICKSTEP-89 Add support for common subexpre...

2017-04-23 Thread zuyu
Github user zuyu commented on the issue: https://github.com/apache/incubator-quickstep/pull/237 I have not done one pass. May have some more comments coming. --- 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

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112847711 --- Diff: query_optimizer/expressions/SimpleCase.cpp --- @@ -161,6 +163,50 @@ ::quickstep::Scalar* SimpleCase::concretize( else_result_ex

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112847577 --- Diff: query_optimizer/expressions/CommonSubexpression.hpp --- @@ -0,0 +1,141 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112848219 --- Diff: query_optimizer/rules/ExtractCommonSubexpression.hpp --- @@ -0,0 +1,179 @@ +/** + * Licensed to the Apache Software Foundation (AS

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112848647 --- Diff: query_optimizer/rules/ReuseAggregateExpressions.hpp --- @@ -0,0 +1,154 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112848194 --- Diff: query_optimizer/rules/ExtractCommonSubexpression.hpp --- @@ -0,0 +1,179 @@ +/** + * Licensed to the Apache Software Foundation (AS

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112848427 --- Diff: query_optimizer/rules/ReuseAggregateExpressions.cpp --- @@ -0,0 +1,349 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112847895 --- Diff: query_optimizer/rules/CollapseSelection.hpp --- @@ -0,0 +1,62 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under o

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112847881 --- Diff: query_optimizer/rules/CollapseSelection.cpp --- @@ -0,0 +1,59 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under o

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112848246 --- Diff: query_optimizer/rules/ReuseAggregateExpressions.cpp --- @@ -0,0 +1,349 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112848329 --- Diff: query_optimizer/rules/ReuseAggregateExpressions.cpp --- @@ -0,0 +1,349 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112847625 --- Diff: query_optimizer/expressions/CommonSubexpression.hpp --- @@ -0,0 +1,141 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112847605 --- Diff: query_optimizer/expressions/CommonSubexpression.hpp --- @@ -0,0 +1,141 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] incubator-quickstep pull request #237: QUICKSTEP-89 Add support for common s...

2017-04-23 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/237#discussion_r112848590 --- Diff: query_optimizer/rules/ReuseAggregateExpressions.cpp --- @@ -0,0 +1,349 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] incubator-quickstep pull request #239: Add ThreadPrivateCompactKeyHashTable ...

2017-04-23 Thread jianqiao
GitHub user jianqiao opened a pull request: https://github.com/apache/incubator-quickstep/pull/239 Add ThreadPrivateCompactKeyHashTable for aggregation. This PR implements a new specialized aggregation hash table that is preferable for two-phase (i.e. aggregate locally and then merg