Unfortunately, so far I have not been able to reproduce the issue with a unit test, neither on Github CI [1] nor in a local environment. In any case, analyzing the code we can see that the risk is in theory present; and my original message shows that it can manifest in practice under certain circumstances. Thus, even though I cannot get to reproduce it with a unit test, I will create a Jira and submit a PR to solve the issue. IMO, the latest suggestion from Julian (DEFAULT_CALL to be converted to a method) is the simplest and clearest way to break the cycle.
Regards, Ruben [1] https://github.com/apache/calcite/pull/2251 On Fri, Nov 6, 2020 at 8:27 PM Ruben Q L <[email protected]> wrote: > Thanks for the feedback. > FYI, I have been able to workaround the issue in the application by > tweaking its code to force some Calcite classes to be loaded at different > points. > Nevertheless, it's true it would be nice to break the circular dependency > on Calcite's side; I'll work on the proposed suggestions. > Ruben > > On Fri, Nov 6, 2020 at 7:24 PM Julian Hyde <[email protected]> wrote: > >> DEFAULT_CALL could also be converted to a method. There’s no harm if such >> SqlCall objects are created each time we need them. >> >> > On Nov 6, 2020, at 11:05 AM, Vladimir Sitnikov < >> [email protected]> wrote: >> > >> > Just in case, I've added a GitHub job that runs Calcite tests with >> OpenJ9 >> > JVM 1.8 in >> > >> https://github.com/apache/calcite/commit/d226a9456a8cfdb108a50744d702e915b46c7ffa >> > >> > Julian>The problem occurs at InferTypes line 41, which is a lambda >> > >> > I believe lambda/reference is irrelevant since SqlOperandTypeInference >> (the >> > type of the field) is interface with a method that receives >> SqlCallBinding. >> > So the implementation does not matter, and JVM would have to load and >> link >> > SqlCallBinding. >> > >> > --- >> > >> > Technically speaking, circular dependencies between classes/packages are >> > sad, so it might be nice to heal that anyway. >> > I believe we could easily fix SqlCallBinding. >> > >> > The current code is as follows, and we do not really need DEFAUL_CALL >> to be >> > present on SqlCallBinding >> > >> > public class SqlCallBinding extends SqlOperatorBinding { >> > private static final SqlCall DEFAULT_CALL = >> > SqlStdOperatorTable.DEFAULT.createCall(SqlParserPos.ZERO); >> > >> > *Ruben*, could you please try wrap that with a holder class as follows? >> > It would break the circular dependency. >> > >> > public class SqlCallBinding extends SqlOperatorBinding { >> > private static class DefaultCallHolder { >> > private static final SqlCall DEFAULT_CALL = >> > SqlStdOperatorTable.DEFAULT.createCall(SqlParserPos.ZERO); >> > } >> > >> > --- >> > >> > RexToLixTranslator, JAVA_TO_SQL_METHOD_MAP is public field. I wonder if >> we >> > can make it private and move to a holder class as well. >> > >> > Vladimir >> >>
