korlov42 commented on code in PR #6044:
URL: https://github.com/apache/ignite-3/pull/6044#discussion_r2149153065
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/IgniteLimit.java:
##########
@@ -176,12 +176,19 @@ public RelOptCost computeSelfCost(RelOptPlanner planner,
RelMetadataQuery mq) {
/** {@inheritDoc} */
@Override
public double estimateRowCount(RelMetadataQuery mq) {
- double inputRowCount = mq.getRowCount(getInput());
+ return estimateRowCount(mq.getRowCount(getInput()), offset, fetch);
+ }
+ /** Returns the estimated row count based on provided input and offset and
fetch attributes. */
+ public static double estimateRowCount(
+ double inputRowCount,
+ @Nullable RexNode offset,
+ @Nullable RexNode fetch
+ ) {
double lim = fetch != null ? doubleFromRex(fetch, inputRowCount *
FETCH_IS_PARAM_FACTOR) : inputRowCount;
double off = offset != null ? doubleFromRex(offset, inputRowCount *
OFFSET_IS_PARAM_FACTOR) : 0;
- return Math.max(0, Math.min(lim, inputRowCount - off));
+ return Math.max(1, Math.min(lim, inputRowCount - off));
Review Comment:
Cost function of every rel is basically derived function of
`estimatedRowCount` of an input, hence once any input returns 0, the cost of
the every plan will probably be a 0. This makes query planner extremely
unstable in terms of choosing the best plan (when cost of every option is 0,
then it's matter of luck which one will be returned). Also, there is an issue
with values less-than-1 when row count is used as denominator to compute
fractions. To address these issues, the cost estimation is patched on a
metadata handler level (see
`org.apache.calcite.rel.metadata.RelMdUtil#validateResult`).
I changed this calculation to make result of direct call to
`estimateRowCount` to be more consistent with the result provided by metadata.
--
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]
