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 <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Ildar Absalyamov <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-HasComments: Yes
