Agree. The change makes sense, and it must be clearly documented on the next release notes.
On Sat, Feb 11, 2023 at 12:18 AM Julian Hyde <jh...@apache.org> wrote: > Agreed. This is a breaking change. > > I've started work in a branch: > https://github.com/julianhyde/calcite/tree/3870-sql2rel-expand-false > > On Fri, Feb 10, 2023 at 1:47 PM Stamatis Zampetakis <zabe...@gmail.com> > wrote: > > > > It totally makes sense to have expand=false since this is what we > recommend. > > > > It can be a notable change though for those using SqlToRelConverter as it > > is so we have to at least put it in a prominent place in the release > notes. > > > > Best, > > Stamatis > > > > > > On Fri, Feb 10, 2023 at 10:32 PM Julian Hyde <jh...@apache.org> wrote: > > > > > You should definitely log a bug for this. Please do so. > > > > > > We would prefer that new features in SqlToRelConverter are developed > > > with expand=false. (Rationale: Keeping subqueries as RexSubQuery > > > expressions allows us to handle them later, via a planner rule, which > > > makes the logic more composable and therefore less buggy.) But we will > > > accept bug fixes to SqlToRelConverter running in expand=true mode if > > > they are clean and simple. > > > > > > That's quite a big 'if'. I suspect that it's quite hard to make a fix > > > without significant changes to SqlToRelConverter. If so, you should > > > stop, because we are not likely to accept it. > > > > > > Calcite committers, > > > > > > For a while https://issues.apache.org/jira/browse/CALCITE-3870 has > > > been open, proposing that expand=false is the default. Should we do > > > it? I think it will clarify our position, and make people less likely > > > to run into bugs when they use the old expand=true behavior by > > > accident. > > > > > > Julian > > > > > > > > > On Fri, Feb 10, 2023 at 9:57 AM Jonathan Sternberg <jonat...@bodo.ai> > > > wrote: > > > > > > > > Hi, > > > > > > > > I'm using Calcite for a project and am attempting to implement the > > > behavior > > > > for ANY/ALL such as: > > > > > > > > SELECT ... FROM ... WHERE a = ANY(SELECT ...) > > > > > > > > When I attempt to have Calcite create a plan for this query, I get > the > > > > runtime exception from here: > > > > > > > > https://github.com/apache/calcite/blob/717eb59a73e2b456266da1af5ff9b881b6f7eeed/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L5319 > > > > > > > > withExpand = true is the default behavior and, in other cases like > > > HAVING, > > > > it produces a more desirable plan so I'd prefer to keep withExpand > to its > > > > default value rather than overriding it. Is there a way to allow > > > subqueries > > > > to be expanded while also allowing subqueries that can't be expanded > to > > > > stay as subqueries? Would Calcite be open to a change that allowed > this > > > > behavior? > > > > > > > > Thanks. > > > > > > > > --Jonathan Sternberg > > > >
