Luo Chen has posted comments on this change.

Change subject: [ASTERIXDB-2344] LIMIT pushdown for primary index
......................................................................


Patch Set 5:

(8 comments)

For UnnestMap vs LeftOuterUnnestMap, I used the former intentionally since this 
patch does not handle the join case.

https://asterix-gerrit.ics.uci.edu/#/c/2541/5/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit-scan-select/limit-scan-select.1.ddl.sqlpp
File 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/limit/limit-scan-select/limit-scan-select.1.ddl.sqlpp:

Line 36: create  dataset DBLP1(DBLPType) primary key id;
> Test case name: push-limit-to-primary-scan-select ?
Done


https://asterix-gerrit.ics.uci.edu/#/c/2541/5/asterixdb/asterix-app/src/test/resources/runtimets/results/limit/limit-scan-select/limit-scan-select.3.adm
File 
asterixdb/asterix-app/src/test/resources/runtimets/results/limit/limit-scan-select/limit-scan-select.3.adm:

Line 20:                   -- EMPTY_TUPLE_SOURCE  |PARTITIONED|
> Is this actual result that we expect?
Yes, this is the query plan.


https://asterix-gerrit.ics.uci.edu/#/c/2541/5/asterixdb/asterix-app/src/test/resources/runtimets/results/limit/limit-scan/limit-scan.3.adm
File 
asterixdb/asterix-app/src/test/resources/runtimets/results/limit/limit-scan/limit-scan.3.adm:

Line 20:                   -- EMPTY_TUPLE_SOURCE  |PARTITIONED|
> Is this actual result that we expect?
Yes.


https://asterix-gerrit.ics.uci.edu/#/c/2541/5/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 336:             plan.append(" select condition 
(").append(selectCondition.getValue().accept(exprVisitor, indent))
> maybe "condition"?
Done


Line 345:             plan.append(" output limit 
").append(String.valueOf(outputLimit));
> Maybe just "limit"?
Done


https://asterix-gerrit.ics.uci.edu/#/c/2541/5/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexSearchOperatorNodePushable.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexSearchOperatorNodePushable.java:

Line 278:         try {
> Maybe we need to add one more if() here to not go through the following for
I don't think this is necessary. If finished = true, then this loop won't be 
executed anyway.


Line 385: 
> Why is the following class needed in this class? Also, can you put a commen
Because ITupleFilter uses IFrameTupleReference (and all IScalarEvaluators)...


Line 417:                     "getFrameTupleAccessor is not supported by 
ReferenceFrameTupleReference");
> Let's use ErrorCode?
Not sure that is necessary. ErrorCode was used for external message for users. 
But here it's just a bug to call this method...


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I824fcad79995325e12a1a81d629160025294b915
Gerrit-PatchSet: 5
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Luo Chen <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <[email protected]>
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Luo Chen <[email protected]>
Gerrit-Reviewer: Taewoo Kim (please use [email protected]) <[email protected]>
Gerrit-Reviewer: Taewoo Kim <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to