Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-2701

Ensure that the "Baz" classes generated by Janino are thread-safe

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.17.0
    • Fix Version/s: 1.18.0
    • Component/s: core

      Description

      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:

      commit fe5edc1904278c1c189f3379e06417aaf4a26315
      Author: aunkrig <aunkrig@eb4544d6-894b-0410-9319-a9612837a279>
      Date: Wed Jan 22 21:19:48 2014 +0000
      
      Partly fixed JANINO-169 - now you get "Compiler limitation: Initializers cannot access local variables declared in an enclosing block".
      
      commit 4d2bacf16229ac1278eafa0fb4d5a0377134d3f2
      Author: aunkrig <aunkrig@eb4544d6-894b-0410-9319-a9612837a279>
      Date: Wed Jan 22 15:03:17 2014 +0000
      
      Added test case for JANINO-169.
      

       

       

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                julianhyde Julian Hyde
                Reporter:
                zabetak Stamatis Zampetakis
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: