Change in asterixdb[master]: [ASTERIXDB-2286][COMP][FUN][HYR] Parallel Sort Optimization

["Dmitry Lychagin (Code Review)" <do...@asterixdb.incubator.apache.org>]

Dmitry Lychagin has posted comments on this change.

Change subject: [ASTERIXDB-2286][COMP][FUN][HYR] Parallel Sort Optimization
......................................................................


Patch Set 25:

(9 comments)

still reviewing. here's the first batch

https://asterix-gerrit.ics.uci.edu/#/c/2393/25/asterixdb/asterix-lang-aql/pom.xml
File asterixdb/asterix-lang-aql/pom.xml:

Line 182:       <version>0.3.4-SNAPSHOT</version>
remove hardcoded version from here


https://asterix-gerrit.ics.uci.edu/#/c/2393/25/asterixdb/asterix-lang-common/pom.xml
File asterixdb/asterix-lang-common/pom.xml:

Line 109:       <version>0.3.4-SNAPSHOT</version>
remove hard-coded version


https://asterix-gerrit.ics.uci.edu/#/c/2393/25/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/std/LocalSamplingAggregateDescriptor.java
File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/std/LocalSamplingAggregateDescriptor.java:

Line 73:         private static final int NUM_SAMPLES = 100;
we should make this configurable by users (in a separate change)


Line 121:                 oneSampleBuilder.addItem(inputFieldValue);
Should we check here that the inputFieldValue is of a primitive type? Otherwise it might contain records with closed types that must be opened before inserting them into this list. We can do it in a follow up change


https://asterix-gerrit.ics.uci.edu/#/c/2393/25/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/SubstituteVariableVisitor.java
File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/SubstituteVariableVisitor.java:

Line 466:         op.getRangeMapExpression().getValue().substituteVar(arg.first, arg.second);
need to call substVarTypes(op, arg); after this line


https://asterix-gerrit.ics.uci.edu/#/c/2393/25/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/ForwardPOperator.java
File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/ForwardPOperator.java:

Line 122:         // TODO(ali): check this
seems to be fine. let's remove this comment


https://asterix-gerrit.ics.uci.edu/#/c/2393/25/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/prettyprint/LogicalOperatorPrettyPrintVisitor.java
File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/prettyprint/LogicalOperatorPrettyPrintVisitor.java:

Line 470:         addIndent(indent).append("forward");
print rangemap key and expression?


https://asterix-gerrit.ics.uci.edu/#/c/2393/25/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/prettyprint/LogicalOperatorPrettyPrintVisitorJson.java
File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/prettyprint/LogicalOperatorPrettyPrintVisitorJson.java:

Line 652:         addIndent(indent).append("\"operator\": \"forward\"");
print rangemap key and expression?


https://asterix-gerrit.ics.uci.edu/#/c/2393/25/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/collectors/SequentialMergeFrameReader.java
File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/collectors/SequentialMergeFrameReader.java:

Line 41:         partitionBatchManager.getNextBatch(senders, numSenders);
is there a guarantee that the senders will be returned in the order you need? which code gives that guarantee?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I73e128029a46f45e6b68c23dfb9310d5de10582f
Gerrit-PatchSet: 25
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ali Alsuliman <al...@gmail.com>
Gerrit-Reviewer: Ali Alsuliman <al...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-Reviewer: Ildar Absalyamov <il...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Taewoo Kim <wa...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-HasComments: Yes