You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Wegdan Ghazi <we...@teampicnic.com> on 2024/01/26 08:50:43 UTC
Question on `Expressions#constant` accepting only public field models
Hello devs!
I'm currently working on a `TranslatableTable` implementation, in which I
also use a custom implementation of `TableScan` that implements
`EnumerableRel#implement`.
For this logic, I need to pass Model Objects through generated code for
processing using `Expressions#constant`, similar to what is verified by
this test
<https://github.com/apache/calcite/blob/e17098d47f3c31e4d90cc17e6e1da1175bf49ae4/linq4j/src/test/java/org/apache/calcite/linq4j/test/ExpressionTest.java#L865>
.
Yet, for this to work I need to define models with public fields only, due
to the check depending on matching a constructor with
`value.getClass().getFields()` here
<https://github.com/apache/calcite/blob/e17098d47f3c31e4d90cc17e6e1da1175bf49ae4/linq4j/src/main/java/org/apache/calcite/linq4j/tree/ConstantExpression.java#L300>
and here
<https://github.com/apache/calcite/blob/e17098d47f3c31e4d90cc17e6e1da1175bf49ae4/linq4j/src/main/java/org/apache/calcite/linq4j/tree/ConstantExpression.java#L216>.
This means we can't use e.g. java records, and we also cannot declare
Models with private fields and public accessors.
This can be fixed by using `value.getClass().getDeclaredFields()` instead.
I've created a small test branch to verify it here
<https://github.com/apache/calcite/compare/main...WegdanGhazi:calcite:wip-constant-expression-fix>
.
I wanted to ask if there might be other reasons why this behaviour
shouldn't be permitted.
Thank you for your time!
Re: Question on `Expressions#constant` accepting only public field models
Posted by Julian Hyde <jh...@gmail.com>.
It’s reasonable to extend the kind of fields that we allow. You should log a jira case and submit a PR with appropriate tests.
It might be tricky to test Java records given that the test suite needs to compile and pass on JDK 8, but maybe you can do something using reflection.
Julian
> On Jan 26, 2024, at 12:50 AM, Wegdan Ghazi <we...@teampicnic.com> wrote:
>
> Hello devs!
>
> I'm currently working on a `TranslatableTable` implementation, in which I
> also use a custom implementation of `TableScan` that implements
> `EnumerableRel#implement`.
>
> For this logic, I need to pass Model Objects through generated code for
> processing using `Expressions#constant`, similar to what is verified by
> this test
> <https://github.com/apache/calcite/blob/e17098d47f3c31e4d90cc17e6e1da1175bf49ae4/linq4j/src/test/java/org/apache/calcite/linq4j/test/ExpressionTest.java#L865>
> .
>
> Yet, for this to work I need to define models with public fields only, due
> to the check depending on matching a constructor with
> `value.getClass().getFields()` here
> <https://github.com/apache/calcite/blob/e17098d47f3c31e4d90cc17e6e1da1175bf49ae4/linq4j/src/main/java/org/apache/calcite/linq4j/tree/ConstantExpression.java#L300>
> and here
> <https://github.com/apache/calcite/blob/e17098d47f3c31e4d90cc17e6e1da1175bf49ae4/linq4j/src/main/java/org/apache/calcite/linq4j/tree/ConstantExpression.java#L216>.
> This means we can't use e.g. java records, and we also cannot declare
> Models with private fields and public accessors.
>
> This can be fixed by using `value.getClass().getDeclaredFields()` instead.
> I've created a small test branch to verify it here
> <https://github.com/apache/calcite/compare/main...WegdanGhazi:calcite:wip-constant-expression-fix>
> .
>
> I wanted to ask if there might be other reasons why this behaviour
> shouldn't be permitted.
>
> Thank you for your time!