[jira] [Commented] (CALCITE-2701) Make generated Baz classes immutable
[
https://issues.apache.org/jira/browse/CALCITE-2701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16699648#comment-16699648
]
Stamatis Zampetakis commented on CALCITE-2701:
--
By immutable, I was referring to the instances of the Baz classes produced by
Janino. My main problem is the instance variable root that gets assigned every
time the bind method is called.
Regarding the JANINO-169, as I wrote in the description the best I could find
is this
[commit|https://github.com/janino-compiler/janino/commit/fe5edc1904278c1c189f3379e06417aaf4a26315].
I couldn't even find the description of the actual issue.
> Make generated Baz classes immutable
>
>
> Key: CALCITE-2701
> URL: https://issues.apache.org/jira/browse/CALCITE-2701
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Affects Versions: 1.17.0
>Reporter: Stamatis Zampetakis
>Assignee: Julian Hyde
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.18.0
>
>
> Currently the Baz classes that are used to execute queries in the
> EnumerableConvention are stateful since the code generated by
> EnumerableRelImplementor#implementRoot method creates an instance variable
> (i.e., DataContext root). Every call to the bind method changes the value of
> the root variable making the object mutable.
> I propose to remove the instance variable and in the body of the method use
> the DataContext passed as a parameter.
> It appears that the addition of the instance variable was a workaround for a
> bug in Janino (JANINO-169). It seems that the bug is no longer present at the
> current version of Janino used in Calcite (3.0.9) since the test
> JdbcTest#testJanino169 (and the complete suite) pass successfully with the
> proposed modification.
> Unfortunately, I couldn't verify the fix in the release notes of Janino
> neither in the commit history. The best, I could find is the following
> commits in the Janino repository:
> {code:none}
> commit fe5edc1904278c1c189f3379e06417aaf4a26315
> Author: aunkrig
> Date: Wed Jan 22 21:19:48 2014 +
> Partly fixed JANINO-169 - now you get "Compiler limitation: Initializers
> cannot access local variables declared in an enclosing block".
> commit 4d2bacf16229ac1278eafa0fb4d5a0377134d3f2
> Author: aunkrig
> Date: Wed Jan 22 15:03:17 2014 +
> Added test case for JANINO-169.
> {code}
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (CALCITE-2701) Make generated Baz classes immutable
[
https://issues.apache.org/jira/browse/CALCITE-2701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16699551#comment-16699551
]
Julian Hyde commented on CALCITE-2701:
--
FWIW, This commit seems to be removing the workaround for JANINO-169 that was
added in
[8e2f4ec3|https://github.com/apache/calcite/commit/8e2f4ec326074e799788f0d83bf9aa647ed30d93].
Do you have a link to the Janino issue? I could only find [the Janino commit
that introduced the test case for
it|https://github.com/janino-compiler/janino/commit/4d2bacf16229ac1278eafa0fb4d5a0377134d3f2].
> Make generated Baz classes immutable
>
>
> Key: CALCITE-2701
> URL: https://issues.apache.org/jira/browse/CALCITE-2701
> Project: Calcite
> Issue Type: Improvement
> Components: core
>Affects Versions: 1.17.0
>Reporter: Stamatis Zampetakis
>Assignee: Julian Hyde
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.18.0
>
>
> Currently the Baz classes that are used to execute queries in the
> EnumerableConvention are stateful since the code generated by
> EnumerableRelImplementor#implementRoot method creates an instance variable
> (i.e., DataContext root). Every call to the bind method changes the value of
> the root variable making the object mutable.
> I propose to remove the instance variable and in the body of the method use
> the DataContext passed as a parameter.
> It appears that the addition of the instance variable was a workaround for a
> bug in Janino (JANINO-169). It seems that the bug is no longer present at the
> current version of Janino used in Calcite (3.0.9) since the test
> JdbcTest#testJanino169 (and the complete suite) pass successfully with the
> proposed modification.
> Unfortunately, I couldn't verify the fix in the release notes of Janino
> neither in the commit history. The best, I could find is the following
> commits in the Janino repository:
> {code:none}
> commit fe5edc1904278c1c189f3379e06417aaf4a26315
> Author: aunkrig
> Date: Wed Jan 22 21:19:48 2014 +
> Partly fixed JANINO-169 - now you get "Compiler limitation: Initializers
> cannot access local variables declared in an enclosing block".
> commit 4d2bacf16229ac1278eafa0fb4d5a0377134d3f2
> Author: aunkrig
> Date: Wed Jan 22 15:03:17 2014 +
> Added test case for JANINO-169.
> {code}
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
