[jira] [Commented] (CALCITE-5647) RelMdPopulationSize calls rel.estimateRowCount instead of mq.getRowCount

["Adam Kennedy (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CALCITE-5647?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17712574#comment-17712574 ] 

Adam Kennedy commented on CALCITE-5647:
---------------------------------------

Fix is available in the following PR.
https://github.com/apache/calcite/pull/3151

My apologies if I'm doing anything incorrectly or not via the correct approach here, as this is my first attempt to navigate the contribution process.

With regards to tests, while I'd love to be able to write a test case for just this one change, all this change does is deal with the result of there not being a much larger test suite which would swap out the RelMdRowCount entirely, sub-class all of the relations to make them throw exceptions (as we did in our application) and verify that a wide range of queries still success despite having this alternative setup. 

I'm not sure how to setup such a large intrusive test suite in the Calcite core project, nor am I sure it should be needed just to correct this one flaw (although there are several other potential similar cases elsewhere in the Calcite codebase that we have not triggered yet as we don't use materialized views and we also full override computeSelfCost.

Most of the inappropriate direct usage of estimateRowCount is either from other estimateRowCount methods (which are fine) or from computeSelfCost method (which probably are not fine but don't break in our case).

> RelMdPopulationSize calls rel.estimateRowCount instead of mq.getRowCount
> ------------------------------------------------------------------------
>
>                 Key: CALCITE-5647
>                 URL: https://issues.apache.org/jira/browse/CALCITE-5647
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.34.0
>            Reporter: Adam Kennedy
>            Priority: Minor
>              Labels: pull-request-available
>   Original Estimate: 1h
>          Time Spent: 10m
>  Remaining Estimate: 50m
>
> A few locations in Calcite call rel.estimateRowCount(mq) when they should instead call mq.getRowCount(red).
> We detected this because we implemented row count estimation entirely within an alternative handle instead of RelMdRowCount, and then override estimateRowCount to ensure the custom handler is user, by throwing an unreachable code exception.
> A few places in Calcite trigger these unreachable exceptions because they do not use mq.getRowCount.
> The most easily triggered on is in RelMdPopulationSize for the Values parameter.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)