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

Vladimir Sitnikov commented on CALCITE-3769:
--------------------------------------------

{quote} This fix does not expect to break the plans for engines that use 
Calcite as a dependency lib, because the Enumerations is just a physical 
implementations for Calcite itself.{quote}
[~danny0405], I do not buy that. PR#1790 does multiple modifications to the 
expected test outputs.
In other words, client code that validates execution plans might need to 
require changes.

Certain tests require the addition of the rule 
(EnumerableRules.ENUMERABLE_TABLE_SCAN_RULE), and you add hooks to some of the 
tests.

All of the above look very well as backward compatibility breakage to me. You 
basically force clients to do the similar updates in their own code.

{quote}Julian: if we do it right, the APIs will be different but the results 
will be the same.{quote}
Looking forward to such a change that does **not** edit existing tests


> Deprecate TableScanRule
> -----------------------
>
>                 Key: CALCITE-3769
>                 URL: https://issues.apache.org/jira/browse/CALCITE-3769
>             Project: Calcite
>          Issue Type: Wish
>          Components: core
>    Affects Versions: 1.21.0
>            Reporter: Danny Chen
>            Assignee: Danny Chen
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.22.0
>
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> The TableScanRule is the only planner rule that for a logical node(e.g. the 
> table scan), its function is to pass along the cluster object and invoke the 
> RelOptTable#toRel which is very trivial because it supplies only a simple 
> ToRelContext that does not support expanding view/passing table hints.
> For rels that come from the sql-to-rel conversion, there is already a table 
> conversion logic[1]. This code gives a more powerful ToRelContext that has 
> the complete functionality.
> The only reason that I saw the meaning of existing TableScanRule is for the 
> TableScan that comes from the RelBuilder#scan.
> So I would suggest to deprecate the TableScanRule, instead, we support 
> translating the table directly in RelBuilder#scan,
> We also add a new interface RelBuilder#scan(Iterable<String> tableNames, 
> ToRelContext context), so that user can pass in a more powerful ToRelContext 
> explicitly.
> [1] 
> https://github.com/apache/calcite/blob/d6fa25cd11625ad7b4b74dafbd0211c701b38d49/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L3498



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to