[jira] [Resolved] (CALCITE-5277) Make EnumerableRelImplementor stashedParameters order deterministic to increase BINDABLE_CACHE hit rate

["Ruben Q L (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/CALCITE-5277?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ruben Q L resolved CALCITE-5277.
--------------------------------
    Resolution: Fixed

Done in https://github.com/apache/calcite/commit/6302e6fa85be1d8d9efd8277f6a2bf43195776ac

> Make EnumerableRelImplementor stashedParameters order deterministic to increase BINDABLE_CACHE hit rate
> -------------------------------------------------------------------------------------------------------
>
>                 Key: CALCITE-5277
>                 URL: https://issues.apache.org/jira/browse/CALCITE-5277
>             Project: Calcite
>          Issue Type: Improvement
>          Components: core
>            Reporter: Ruben Q L
>            Assignee: Ruben Q L
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.32.0
>
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> Currently, EnumerableRelImplementor stashedParameters is a map with no deterministic order when its values are traversed.
> Because of this, it is possible to have two EnumerableRel which are effectively equal, but which generate a different dynamic code, just because the statements in which stashedParameters are defined are the same, but in different order.
> {code}
>     final Collection<Statement> stashed =
>         Collections2.transform(stashedParameters.values(),
>             input -> Expressions.declare(Modifier.FINAL, input,
>                 Expressions.convert_(
>                     Expressions.call(DataContext.ROOT,
>                         BuiltInMethod.DATA_CONTEXT_GET.method,
>                         Expressions.constant(input.name)),
>                     input.type)));
> {code}
> This has a disadvantage: in this situation, EnumerableInterpretable's BINDABLE_CACHE will not have a cache hit, so the Bindable will need to be unnecessarily recreated in EnumerableInterpretable#getBindable.
> If we guarantee a deterministic order in EnumerableRelImplementor stashedParameters (e.g. insertion order), we would have a cache hit in these situations (and we could skip the bindable re-creation).



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