Jackie-Jiang commented on pull request #8228:
URL: https://github.com/apache/pinot/pull/8228#issuecomment-1047362480
> > > At the moment, getNexBlock has a conditional split for all/partially
ordered columns (where computeAllOrdered is where all columns in select are
also in the order by if I understand correctly )
> > > If we introduce a case for when all orderBy columns are pre-sorted
(early termination case) then wouldn't we again have a
allOrdered/partialOrdered case within that as well?
> >
> >
> > No. If all the order-by columns are sorted, we can simply pick the first
`limit` docs without worrying about the optimization of partialOrdered case.
>
> May be we can get a slack call to resolve this quickly.
>
> The code "without this PR" looks like below
>
> ```
> if (_expressions.size() == _orderByExpressions.size()) {
> return computeAllOrdered();
> } else {
> return computePartiallyOrdered();
> }
> ```
>
> Here `computeAllOrdered` is where all columns in projection are in order
by clause. And, `computePartiallyOrdered` is where projection has more columns
that in order by clause.
>
> The two cases will also apply to the early terminate case. So with this
optimization we will have 4 cases.
>
> * computeAllOrderedWithEarlyTermination
> * computePartiallyOrderedWithEarlyTermination
> * computeAllOrdered
> * computePartiallyOrdered
>
> I am not sure this is any more readable that what I have.
What I am suggesting is something like:
```
if (allOrderByColumnsSorted()) {
return computeAllSorted();
} else if (_expressions.size() == _orderByExpressions.size()) {
return computeAllOrdered();
} else {
return computePartiallyOrdered();
}
```
If all order-by columns are sorted, we don't need to do the partially
ordered optimization, and the logic is all separated which is more clear and
can avoid unnecessary overhead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]