sql: allow references to top-level WITH from apply-join#65550
sql: allow references to top-level WITH from apply-join#65550craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
|
This is needed for the graphile introspection query. |
mgartner
left a comment
There was a problem hiding this comment.
Cool!
Reviewed 2 of 2 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @rytaft)
mgartner
left a comment
There was a problem hiding this comment.
Sorry, left a few questions/nits.
Reviewable status:
pkg/sql/opt/exec/execbuilder/relational.go, line 885 at r1 (raw file):
o.Init(b.evalCtx, b.catalog) f := o.Factory() md := f.Memo().Metadata()
nit: md := f.Metadata()
pkg/sql/opt/exec/execbuilder/relational.go, line 889 at r1 (raw file):
withID := withExprs[i].id memoExpr := b.mem.Metadata().WithBinding(withID) md.AddWithBinding(withID, memoExpr)
If the new lines below allow referrign to "outer" Withs, what does adding the with bindings do here?
pkg/sql/opt/exec/execbuilder/relational.go, line 909 at r1 (raw file):
if withExprs[i].id == t.With { memoExpr := b.mem.Metadata().WithBinding(t.With) f.Metadata().AddWithBinding(t.With, memoExpr)
nit: md.AddWithBinding(...)
This change fixes an unsupported case where the "inner" plan of an apply-join refers to a WITH that is above the apply join. This change adds support for this case by populating the necessary metadata in the memo and the execbuilder. Release note (sql change): references to WITH expressions from correlated subqueries are now always supported.
e0fe189 to
6bed4e2
Compare
RaduBerinde
left a comment
There was a problem hiding this comment.
TFTR!
Reviewable status:
pkg/sql/opt/exec/execbuilder/relational.go, line 889 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
If the new lines below allow referrign to "outer" Withs, what does adding the with bindings do here?
Oops, I meant to remove this. This doesn't do anything because CopyAndReplace clears the metadata.
mgartner
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/exec/execbuilder/relational.go, line 889 at r1 (raw file):
Previously, RaduBerinde wrote…
Oops, I meant to remove this. This doesn't do anything because CopyAndReplace clears the metadata.
👍
RaduBerinde
left a comment
There was a problem hiding this comment.
bors r+
Reviewable status:
|
Build succeeded: |
This change fixes an unsupported case where the "inner" plan of an
apply-join refers to a WITH that is above the apply join. This change
adds support for this case by populating the necessary metadata in the
memo and the execbuilder.
Release note (sql change): references to WITH expressions from
correlated subqueries are now always supported.