I agree that expand should default to false in ConfigBuilder. Rationale: expand = false is the preferred modern behavior. We want SqlToRelConverter to leave sub-queries as sub-queries (wrapping in RexSubQuery) and then deal with them later with SubQueryRemoveRule. It is difficult for SqlToRelConverter to expand sub-queries and also to remove correlating variables, and there are bugs that are difficult to fix. I believe that the modern path has fewer bugs. We are not putting much effort into maintaining the old path.
Can you please log a bug? (Your images did not come through in the email. If they add significantly to your argument, you could attach them to the bug.) This will be a breaking change. However I think it will be acceptable with a release note. Julian > On Mar 23, 2020, at 1:16 AM, JiaTao Tao <[email protected]> wrote: > > Hi > > Now the default "expand" in SqlToRelConverter.ConfigBuilder is true but in > calcite's main process, actually, it is false( > `withExpand(THREAD_EXPAND.get())`) > > > > > > That leads we need to explicitly set `withExpand` to false when we use > SqlToRelConverter. > > So I think we should change the default to "false" in > SqlToRelConverter.ConfigBuilder. > > If you think so, I would like to do the minor change. > > Regards! > Aron Tao
