[GitHub] incubator-quickstep pull request #180: Fix the bug with SingleIdentityHashFi...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/180#discussion_r99859847 --- Diff: query_optimizer/rules/AttachLIPFilters.cpp --- @@ -128,7 +129,7 @@ void AttachLIPFilters::attachLIPFilters(

[GitHub] incubator-quickstep pull request #177: Reduce the number of group-by attribu...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/177#discussion_r99865183 --- Diff: query_optimizer/Optimizer.cpp --- @@ -30,10 +30,11 @@ void Optimizer::generateQueryHandle(const ParseStatement _statement,

[GitHub] incubator-quickstep pull request #177: Reduce the number of group-by attribu...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/177#discussion_r99865239 --- Diff: query_optimizer/rules/ReduceGroupByAttributes.cpp --- @@ -0,0 +1,217 @@ +/** + * Licensed to the Apache Software Foundation

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99867096 --- Diff: query_optimizer/ExecutionGenerator.cpp --- @@ -371,6 +378,109 @@ void ExecutionGenerator::dropAllTemporaryRelations() { }

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99882605 --- Diff: query_optimizer/ExecutionGenerator.cpp --- @@ -371,6 +378,109 @@ void ExecutionGenerator::dropAllTemporaryRelations() { }

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99883469 --- Diff: query_optimizer/ExecutionGenerator.hpp --- @@ -427,7 +445,7 @@ class ExecutionGenerator { /** * @brief The cost model

[GitHub] incubator-quickstep pull request #180: Fix the bug with SingleIdentityHashFi...

Github user asfgit closed the pull request at: https://github.com/apache/incubator-quickstep/pull/180 --- 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

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99874032 --- Diff: query_optimizer/ExecutionGenerator.cpp --- @@ -371,6 +378,109 @@ void ExecutionGenerator::dropAllTemporaryRelations() { }

[GitHub] incubator-quickstep issue #183: Adds regex to specify 16.+ versions of Darwi...

Github user cramja commented on the issue: https://github.com/apache/incubator-quickstep/pull/183 BTW this is so we can build on Mac. @pateljm I'm still finding that I must specify `cmake -DUSE_TCMALLOC=FALSE ..`. Then it builds successfully on Mac. --- If your project is

[GitHub] incubator-quickstep pull request #183: Adds regex to specify 16.+ versions o...

GitHub user cramja opened a pull request: https://github.com/apache/incubator-quickstep/pull/183 Adds regex to specify 16.+ versions of Darwin Before this change, only one recent version of Darwin was correctly having the compiler flags modified to allow for deprecated syscalls.

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99882633 --- Diff: query_optimizer/ExecutionGenerator.cpp --- @@ -371,6 +378,109 @@ void ExecutionGenerator::dropAllTemporaryRelations() { }

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99897008 --- Diff: storage/PackedPayloadHashTable.cpp --- @@ -0,0 +1,463 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99897239 --- Diff: storage/AggregationOperationState.hpp --- @@ -156,6 +152,29 @@ class AggregationOperationState { const CatalogDatabaseLite

[GitHub] incubator-quickstep issue #179: QUICKSTEP-70-71 Improve aggregation performa...

Github user jianqiao commented on the issue: https://github.com/apache/incubator-quickstep/pull/179 For the question about `PartitionedHashTablePool` and `HashTablePool`. Note that their use patterns are different so perhaps it is not natural to merge them into one class.

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99908259 --- Diff: storage/AggregationOperationState.hpp --- @@ -156,6 +152,29 @@ class AggregationOperationState { const CatalogDatabaseLite );

[GitHub] incubator-quickstep issue #179: QUICKSTEP-70-71 Improve aggregation performa...

Github user zuyu commented on the issue: https://github.com/apache/incubator-quickstep/pull/179 Please resync with the master branch, and I will merge it. 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

[GitHub] incubator-quickstep issue #179: QUICKSTEP-70-71 Improve aggregation performa...

Github user jianqiao commented on the issue: https://github.com/apache/incubator-quickstep/pull/179 Updated, and tested locally. --- 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

[GitHub] incubator-quickstep issue #183: Adds regex to specify 16.+ versions of Darwi...

Github user cramja commented on the issue: https://github.com/apache/incubator-quickstep/pull/183 What was your solution to tc malloc issue on mac, just ignore it? --- 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

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99914989 --- Diff: storage/AggregationOperationState.cpp --- @@ -353,187 +353,286 @@ bool AggregationOperationState::ProtoIsValid( return true;

[GitHub] incubator-quickstep issue #183: Adds regex to specify 16.+ versions of Darwi...

Github user zuyu commented on the issue: https://github.com/apache/incubator-quickstep/pull/183 So far it is only the problem for Mac, so I disabled `tcmalloc` by `-DUSE_TCMALLOC=0` in `cmake` configuration. --- If your project is set up for it, you can reply to this email and have

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99928307 --- Diff: storage/AggregationOperationState.cpp --- @@ -353,187 +353,286 @@ bool AggregationOperationState::ProtoIsValid( return true;

[GitHub] incubator-quickstep issue #179: QUICKSTEP-70-71 Improve aggregation performa...

Github user jianqiao commented on the issue: https://github.com/apache/incubator-quickstep/pull/179 There is a `CMakeLists` to be updated -- do not merge at this moment. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] incubator-quickstep pull request #183: Adds regex to specify 16.+ versions o...

Github user cramja closed the pull request at: https://github.com/apache/incubator-quickstep/pull/183 --- 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

[GitHub] incubator-quickstep issue #183: Adds regex to specify 16.+ versions of Darwi...

Github user zuyu commented on the issue: https://github.com/apache/incubator-quickstep/pull/183 Next time, before merging, sync again your local branch with the master, and this PR will automatically merged, instead of manually closed. --- If your project is set up for it, you can

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99918342 --- Diff: storage/AggregationOperationState.hpp --- @@ -156,6 +152,29 @@ class AggregationOperationState { const CatalogDatabaseLite

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99929968 --- Diff: relational_operators/WorkOrderFactory.cpp --- @@ -186,6 +186,7 @@ WorkOrder* WorkOrderFactory::ReconstructFromProto(const

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99930761 --- Diff: storage/AggregationOperationState.cpp --- @@ -556,80 +655,83 @@ void AggregationOperationState::finalizeSingleState(

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99915265 --- Diff: storage/AggregationOperationState.cpp --- @@ -353,187 +353,286 @@ bool AggregationOperationState::ProtoIsValid( return true;

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99922836 --- Diff: relational_operators/InitializeAggregationOperator.cpp --- @@ -0,0 +1,72 @@ +/** + * Licensed to the Apache Software

[GitHub] incubator-quickstep issue #183: Adds regex to specify 16.+ versions of Darwi...

Github user cramja commented on the issue: https://github.com/apache/incubator-quickstep/pull/183 Merged, closing --- 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

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99924782 --- Diff: relational_operators/InitializeAggregationOperator.cpp --- @@ -0,0 +1,72 @@ +/** + * Licensed to the Apache Software Foundation

Re: Editing documentation for release: BUILDING.md

Any objections if I remove the 'Getting cmake' section? It seems long, and like information that a database programmer would probably already know/google and quickly find out. Instead I can link to cmake's webpage in the prereq section On Tue, Feb 7, 2017 at 6:25 PM, Marc Spehlmann

[GitHub] incubator-quickstep issue #179: QUICKSTEP-70-71 Improve aggregation performa...

Github user pateljm commented on the issue: https://github.com/apache/incubator-quickstep/pull/179 Very impressive algorithmic change @jianqiao !! --- 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

Re: Editing documentation for release: BUILDING.md

Great changes! +1 for replacing the current building doc with this one. Cheers, Jignesh On 2/7/17, 6:43 PM, "Marc Spehlmann" wrote: For example, here is the edited doc:

[GitHub] incubator-quickstep issue #177: Reduce the number of group-by attributes by ...

Github user pateljm commented on the issue: https://github.com/apache/incubator-quickstep/pull/177 @jianqiao Another neat optimization! I also like how your PRs are clean. Thank you for adding the pdf, which makes understanding this optimization super easy. @zuyu Great

Re: Editing documentation for release: BUILDING.md

Marc, Should we get rid of curl from the build instructions? We don’t need it with the current instruction flow. Cheers, Jignesh On 2/7/17, 6:43 PM, "Marc Spehlmann" wrote: For example, here is the edited doc:

[GitHub] incubator-quickstep pull request #177: Reduce the number of group-by attribu...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/177#discussion_r99963866 --- Diff: query_optimizer/rules/ReduceGroupByAttributes.hpp --- @@ -0,0 +1,143 @@ +/** + * Licensed to the Apache Software Foundation

[GitHub] incubator-quickstep issue #183: Adds regex to specify 16.+ versions of Darwi...

Github user pateljm commented on the issue: https://github.com/apache/incubator-quickstep/pull/183 Thanks @cramja !! +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled

[GitHub] incubator-quickstep pull request #177: Reduce the number of group-by attribu...

Github user asfgit closed the pull request at: https://github.com/apache/incubator-quickstep/pull/177 --- 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

[GitHub] incubator-quickstep pull request #177: Reduce the number of group-by attribu...

Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/177#discussion_r99964669 --- Diff: query_optimizer/rules/ReduceGroupByAttributes.hpp --- @@ -0,0 +1,143 @@ +/** + * Licensed to the Apache Software Foundation (ASF)

[GitHub] incubator-quickstep pull request #177: Reduce the number of group-by attribu...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/177#discussion_r99964422 --- Diff: query_optimizer/rules/ReduceGroupByAttributes.hpp --- @@ -0,0 +1,143 @@ +/** + * Licensed to the Apache Software Foundation

Re: Editing documentation for release: BUILDING.md

For example, here is the edited doc: https://github.com/cramja/incubator-quickstep/blob/refactor-building/BUILDING.md#prerequisites On Tue, Feb 7, 2017 at 6:28 PM, Marc Spehlmann wrote: > Any objections if I remove the 'Getting cmake' section? It seems long, and > like

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99916031 --- Diff: storage/AggregationOperationState.cpp --- @@ -353,187 +353,286 @@ bool AggregationOperationState::ProtoIsValid( return true;

[GitHub] incubator-quickstep pull request #184: Refactor building.md

GitHub user cramja opened a pull request: https://github.com/apache/incubator-quickstep/pull/184 Refactor building.md * Added table of content * updated building instructions * Removed some redundancy * Moved bonus sections into the appendix You can merge this pull

Re: Editing documentation for release: BUILDING.md

curl is needed for the download_script. There's probably many other prereqs (git/bash) that are required, but they're sort of obvious to system builders. So I could see removing it for that reason. On Tue, Feb 7, 2017 at 6:51 PM, Jignesh Patel wrote: > Marc, > > Should

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99901875 --- Diff: storage/AggregationOperationState.hpp --- @@ -156,6 +152,29 @@ class AggregationOperationState { const CatalogDatabaseLite

[GitHub] incubator-quickstep issue #183: Adds regex to specify 16.+ versions of Darwi...

Github user zuyu commented on the issue: https://github.com/apache/incubator-quickstep/pull/183 I sometimes encountered `tcmalloc` build issues on Mac, but it worked for a while before the third party changes. --- If your project is set up for it, you can reply to this email and

[GitHub] incubator-quickstep pull request #179: QUICKSTEP-70-71 Improve aggregation p...

Github user jianqiao commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/179#discussion_r99884619 --- Diff: query_optimizer/ExecutionGenerator.cpp --- @@ -1495,9 +1607,28 @@ void ExecutionGenerator::convertAggregate( } if