tkalkirill commented on code in PR #13389:
URL: https://github.com/apache/ignite/pull/13389#discussion_r3860524650
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteNestedLoopJoin.java:
##########
@@ -88,12 +88,16 @@ public IgniteNestedLoopJoin(RelInput input) {
if (Double.isInfinite(rightCnt))
return costFactory.makeInfiniteCost();
- double rows = leftCnt * rightCnt;
-
double rightSize = rightCnt * getRight().getRowType().getFieldCount()
* IgniteCost.AVERAGE_FIELD_SIZE;
- return costFactory.makeCost(rows,
- rows * (IgniteCost.ROW_COMPARISON_COST +
IgniteCost.ROW_PASS_THROUGH_COST), 0, rightSize, 0);
+ double rowCnt = mq.getRowCount(this);
+
+ RelOptCost cost = costFactory.makeCost(rowCnt,
Review Comment:
I agree that the current cost model is heuristic and preserving existing
plans is useful. However, output cardinality and the amount of work are
different estimates. Even with exact statistics, an NLJ over two 1M-row inputs
may perform up to 1e12 comparisons while producing a single row, so `10 *
rowCnt` still severely underestimates its CPU cost.
Could we keep rowCnt as the output cardinality, but calculate CPU from
`leftCnt * rightCnt`, using `rowCnt` only for the pass-through part? Also,
`multiplyBy(10)` scales memory cost as well as CPU, which seems unintended.
If this changes `PlanSplitterTest`, I suggest reviewing the affected plans
and updating justified expectations rather than tuning a global coefficient
specifically to preserve snapshots.
--
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]
