asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yingyi Bu (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: ASTERIXDB-1566: Fixed External Hash Group By to conform to t...
Date Mon, 31 Oct 2016 23:22:24 GMT
Yingyi Bu has posted comments on this change.

Change subject: ASTERIXDB-1566: Fixed External Hash Group By to conform to the memory budget
......................................................................


Patch Set 23:

(7 comments)

Why tableSize in ExternalGroupWriteOperatorNodePushable is still tuple count?   Should you
use the new formula there as well?

https://asterix-gerrit.ics.uci.edu/#/c/1056/23/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SetAsterixPhysicalOperatorsRule.java
File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SetAsterixPhysicalOperatorsRule.java:

Line 157:                                     int numberOfGroupByColumns = gby.getGroupByList().size();
+ gby.getDecorList().size() ?


https://asterix-gerrit.ics.uci.edu/#/c/1056/23/hyracks-fullstack/algebricks/algebricks-rewriter/pom.xml
File hyracks-fullstack/algebricks/algebricks-rewriter/pom.xml:

Line 67:       <artifactId>hyracks-dataflow-std</artifactId>
Why rewriter needs runtime operator?


https://asterix-gerrit.ics.uci.edu/#/c/1056/23/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SetAlgebricksPhysicalOperatorsRule.java
File hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SetAlgebricksPhysicalOperatorsRule.java:

Line 173:                                     int numberOfGroupByColumns = gby.getGroupByList().size();
+ gby.getDecorList().size() ?


Line 429:         int numberOfBits = numberOfGroupByColumns * 4 * 8;
numberOfBits --> numberOfBytes


Line 438:         long groupByTableByteSize = groupByTableSize * SerializableHashTable.getExpectedByteSizePerHashValue();
This seems to create an unnecessary dependency. 

The rewriter shouldn't depend on the runtime.

Use a constant to store the value in the physicalOptimizationConfig and use phyiscalOptimizationConfig.getExpectedByteSizePerHashValue()?


https://asterix-gerrit.ics.uci.edu/#/c/1056/23/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/structures/SerializableHashTable.java
File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/structures/SerializableHashTable.java:

Line 32:  * .
why period here?


Line 36:  * .
why period here?


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1056
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b323e9a2141b4c1dd1652a360d2d9354d3bc3f5
Gerrit-PatchSet: 23
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Taewoo Kim <wangsaeu@yahoo.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Jianfeng Jia <jianfeng.jia@gmail.com>
Gerrit-Reviewer: Michael Blow <mblow@apache.org>
Gerrit-Reviewer: Taewoo Kim <wangsaeu@yahoo.com>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Wenhai Li <lwhaymail@yahoo.com>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message