Can't really use a PR if it's just a bit of code that seems to be better than the previous bit of code. We'd love contributions but they are best packaged as follows:
> Here is a bug. > And here is a jira case describing how other people might experience the bug. > And here is a PR that includes a test case. Those kinds of contributions are easier to evaluate, and the description as a bug (or a feature) that will go into the release notes makes them more useful to the wider community. I quickly read through your observations and they seem plausible. I don't have time to dig deeper unless they arrive as a bug+PR. On Tue, Feb 14, 2023 at 9:13 AM Jonathan Sternberg <[email protected]> wrote: > > I took a deeper look today following your recommendation and ran into a > problem when I got into the implementation of the program builder. I think > the problem might be higher up since the error appears to be that it's > attempting to reduce a subquery as a constant. I found this section of > code: > https://github.com/apache/calcite/blob/79879f92abc4f2279496a0e6ca9066d9d24a1457/core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java#L1088-L1091 > . > > It appears to check the arguments to the subquery to see if any of them are > reducible to determine if the subquery itself is reducible. Since the > scalar query doesn't have any operands, it determines that the call is a > reducible constant. But, I'd probably guess that the subquery itself is > never reducible and, if it was desirable to reduce it, then that should be > done through a separate planner rule that expands the subquery into other > nodes. I changed this code to use "analyzeCall(subQuery, > Constancy.IRREDUCIBLE_CONSTANT);" and this seems to prevent the reducer > from attempting to reduce the subquery. Does this sound problematic and > would it be the correct fix for this? If so, I can prepare a PR and submit > it upstream. > > --Jonathan Sternberg > > On Mon, Feb 13, 2023 at 6:52 PM Julian Hyde <[email protected]> wrote: > > > We need to decide whether a RexSubQuery can appear inside a RexProgram. > > > > If yes, then RegisterInputShuttle should be extended to handle it, > > register it, and then it will become a RexLocalRef. > > > > If no, you will need to apply a rule to expand all RexSubQuery instances > > before the phase where you create a RexProgram. > > > > I am inclined to say yes. Or at least give it a try, and see whether you > > can get it to work without too much pain. > > > > Julian > > > > > > > On Feb 13, 2023, at 2:40 PM, Jonathan Sternberg <[email protected]> > > wrote: > > > > > > Hi, > > > > > > I'm attempting to update some code that previously used > > `withExpand(true)` > > > to use `withExpand(false)`. This has resulted in some plans that > > previously > > > used left joins to start using `SCALAR_QUERY`. > > > > > > The code also uses a HepPlanner > > > < > > https://calcite.apache.org/javadocAggregate/org/apache/calcite/plan/hep/HepPlanner.html > > > > > > with > > > the FilterReduceExpressionsRule > > > < > > https://calcite.apache.org/javadocAggregate/org/apache/calcite/rel/rules/ReduceExpressionsRule.FilterReduceExpressionsRule.html#%3Cinit%3E(java.lang.Class,boolean,org.apache.calcite.tools.RelBuilderFactory) > > >. > > > We use the default configuration and add it to the HepPlanner and then > > call > > > "findBestExp". > > > > > > When the planner runs, I get a ClassCastException here: > > > > > https://github.com/apache/calcite/blob/50d124615e0b07f7fbe6107b7c440d9737a00836/core/src/main/java/org/apache/calcite/rex/RexProgramBuilder.java#L303 > > > < > > https://github.com/apache/calcite/blob/50d124615e0b07f7fbe6107b7c440d9737a00836/core/src/main/java/org/apache/calcite/rex/RexExecutorImpl.java#L76-L77 > > > > > > > > > It seems when the RegisterInputShuttle does its thing, it returns a > > > RexSubQuery instead of a RexLocalRef. Is this a bug or am I doing > > something > > > wrong with this planner rule? I see this section of code for handling a > > > RexCall and I assume that registerInternal returns a RexLocalRef, but > > > visitSubQuery has its own implementation in the RexShuttle which calls > > > visitList on the subquery. Is this a bug or is there another way I should > > > be doing this? > > > > > > Thanks. > > > > > > --Jonathan Sternberg > > > >
