Dmitry Lychagin has posted comments on this change.

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

Patch Set 9:


Line 46: public class PushLimitIntoPrimaryRule implements IAlgebraicRewriteRule 
Can we rename it into PushLimitIntoPrimaryIndexSearchRule ?

Line 68:         if (childOp.getValue().getOperatorTag() == 
LogicalOperatorTag.EXCHANGE) {
How come PushLimitIntoOrderByRule which runs before this rule does not consider 
EXCHANGE operators? Can you explain this? 
It seems that ONE_TO_ONE Exchange operators are introduced later by the 
RuleCollections.prepareForJobGenRuleCollection(). So if you see EXCHANGE at 
this point then it's not 1:1 and the LIMIT should not be pushed through it. Is 
this correct?

Line 79:             context.computeAndSetTypeEnvironmentForOperator(op);
use OperatorPropertiesUtil.typeOpRec(opRef, context) instead.

Line 85:         long outputLimit = 
first you need to check whether maxobject expression is a constant: 
limitOp.getMaxObjects().getValue().getExpressionTag() == 
because AccessMethodUtils.getInt64Constant() does not do that.

Line 86:         if (limit.getOffset() != null && limit.getOffset().getValue() 
!= null) {
you do need to check limit.getOffset().getValue() != null, but you do need to 
check that limit.getOffset() is a constant expression.

Line 136:         ILogicalExpression unnestExpr = 
check that this op.getOutputLimit() is null, so nothing got pushed to it yet.

Line 150:         boolean isSource = op.getInputs().isEmpty()
also check that op.getOutpuLimit() is null, so nothing got pushed to it yet.

Line 152:         boolean isDataset = op.getDataSource() instanceof 
use ((DataSource)op.getDataSource()).getDatasourceType() == 
DataSource.Type.INTERNAL_DATASET instead of instanceof and the next line

PS9, Line 1548: getTupleFilterFactory
can we rename it back to createTupleFilterFactory()? it creates a new instance 
so "create*" is better than "get*"

Line 238:             if (tupleFilter != null && 
!tupleFilter.accept(referenceFilterTuple.reset(tuple))) {
It seems that if tupleFilter expression uses any variables from the 
index-search's input then it'll no longer work because you're evaluating in on 
the tuple you get from the cursor which does not have those fields. You need 
address this problem in the optimizer rule you added. Check that all free 
variables of the select expression you're pushing into the index-search are 
produced by that index-search. 
Actually, it's not only variables from the operator input, but other values 
added to the output tuple by this method (appendSearchCallbackProceedResult, 
appendIndexFilter). Any of them can in theory be used by the pushed select 

Line 268:             FrameUtils.appendConcatToWriter(writer, appender, 
accessor, tupleIndex,
before outputting this missing tuple you need to check here that it:
1) satisfies the tupleFilter, and 
2) still within the limit (corner case but LIMIT 0 is possible)

To view, visit
To unsubscribe, visit

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

Reply via email to