Luo Chen has posted comments on this change.

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


Patch Set 10:

(11 comments)

https://asterix-gerrit.ics.uci.edu/#/c/2541/9/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushLimitIntoPrimaryRule.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/PushLimitIntoPrimaryRule.java:

Line 46
> Can we rename it into PushLimitIntoPrimaryIndexSearchRule ?
Done


Line 68
> How come PushLimitIntoOrderByRule which runs before this rule does not cons
I copied this pattern from PushLimitIntoOrderByRule.
But we have another rule CopyLimitDownRule which copies limit as down as 
possible. As a result, limit could before EXCHANGE. And actually this helps 
since each partition can decide when to stop sending data safely.


Line 79
> use OperatorPropertiesUtil.typeOpRec(opRef, context) instead.
Done


Line 85
> first you need to check whether maxobject expression is a constant: limitOp
Done


Line 86
> you do need to check limit.getOffset().getValue() != null, but you do need 
Done


Line 136
> check that this op.getOutputLimit() is null, so nothing got pushed to it ye
Done


Line 150
> also check that op.getOutpuLimit() is null, so nothing got pushed to it yet
Done


Line 152
> use ((DataSource)op.getDataSource()).getDatasourceType() == DataSource.Type
Done


https://asterix-gerrit.ics.uci.edu/#/c/2541/9/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/MetadataProvider.java
File 
asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/MetadataProvider.java:

PS9, Line 1548: createTupleFilterFact
> can we rename it back to createTupleFilterFactory()? it creates a new insta
Done


https://asterix-gerrit.ics.uci.edu/#/c/2541/9/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 238:             if (tupleFilter != null && 
!tupleFilter.accept(referenceFilterTuple.reset(tuple))) {
> It seems that if tupleFilter expression uses any variables from the index-s
Done. I added a check in the optimization rule. Please let me know if that is 
correctly implemented.


Line 268:             FrameUtils.appendConcatToWriter(writer, appender, 
accessor, tupleIndex,
> before outputting this missing tuple you need to check here that it:
I think currently this is OK because we won't push LIMIT into here if retain 
missing is used. Further more, pushing more data into next operators is also OK 
(actually we have to push more data than limited because of multiple 
partitions), since there is another LIMIT operator guarding the final result.


-- 
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: 10
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