sql: properly reject nested generators in ROWS FROM #27390
Conversation
|
This change is |
There was a problem hiding this comment.
Thanks for the quick fix!
Reviewed 4 of 4 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained
pkg/sql/sem/tree/type_check.go, line 57 at r1 (raw file):
// inFuncExpr is temporarily set to true while type checking the // parameters of a function. Used to detect nested generators. inFuncExpr bool
Do you really need this flag? Seems like you should be able to get away without it...
pkg/sql/sem/tree/type_check.go, line 711 at r1 (raw file):
if ctx != nil { // We'll need to remember we are in a function application to // generate suitable errors in checkFunctionUsage().
I'm confused - why did you add this block of code after the call to checkFunctionUsage?
knz
force-pushed
the
20180711-rows-from-reject
branch
from
4596f9d
to
108ab92
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/sql/sem/tree/type_check.go, line 57 at r1 (raw file):
Previously, rytaft wrote…
Do you really need this flag? Seems like you should be able to get away without it...
Hah! Mind explaining me how?
pkg/sql/sem/tree/type_check.go, line 711 at r1 (raw file):
Previously, rytaft wrote…
I'm confused - why did you add this block of code after the call to
checkFunctionUsage?
Added clarification in comment.
See also the (new) comment above RejectNestedGenerators.
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2.
Reviewable status:
pkg/sql/sem/tree/type_check.go, line 57 at r1 (raw file):
Previously, knz (kena) wrote…
Hah! Mind explaining me how?
Ah never mind - I was confused.
pkg/sql/sem/tree/type_check.go, line 711 at r1 (raw file):
Previously, knz (kena) wrote…
Added clarification in comment.
See also the (new) comment above
RejectNestedGenerators.
Thanks!
pkg/sql/sem/tree/type_check.go, line 734 at r2 (raw file):
// to checkFunctionUsage() above) because the top-level FuncExpr // must be acceptable even if it is a SRF and // RehectNestedGenerators is set.
Rehect -> Reject
Prior to this patch, invalid uses of SRFs as arguments to other functions in ROWS FROM were not properly rejected, and were only caught at evaluation time (i.e. much too late). This patch fixes it by rejecting these uses early. Release note (bug fix): invalid uses of set-generating functions in FROM clauses are now reported with the same error code as PostgreSQL.
knz
force-pushed
the
20180711-rows-from-reject
branch
from
108ab92
to
e870a82
Compare
There was a problem hiding this comment.
Thank you!
Reviewable status:
pkg/sql/sem/tree/type_check.go, line 734 at r2 (raw file):
Previously, rytaft wrote…
Rehect -> Reject
Done.
|
bors r+ |
27390: sql: properly reject nested generators in ROWS FROM r=knz a=knz Fixes #27389. Prior to this patch, invalid uses of SRFs as arguments to other functions in ROWS FROM were not properly rejected, and were only caught at evaluation time (i.e. much too late). This patch fixes it by rejecting these uses early. Release note (bug fix): invalid uses of set-generating functions in FROM clauses are now reported with the same error code as PostgreSQL. 27396: sql: help the user understand unsupported correlation r=knz a=knz Fixes #24684. Prior to this patch, a client trying to use an unsupported correlated query would encounter an obscure error like "column v does not exist" or "no data source matches prefix". Given that the new optimizer code can determine whether a query is correlated, we can use this information to enhance the error message. Before: ``` > select * from pg_class a where exists (select * from pg_class b where a.oid = b.oid); pq: no data source matches prefix: a > select * from pg_class a where exists (select * from kv where v::oid = oid); pq: column "oid" does not exist ``` After: ``` > select * from pg_class a where exists (select * from pg_class b where a.oid = b.oid); pq: no data source matches prefix: a HINT: some correlated subqueries are not supported yet - see #3288 > select * from pg_class a where exists (select * from kv where v::oid = oid); pq: column "oid" does not exist HINT: some correlated subqueries are not supported yet - see #3288 ``` Note: some correlated queries do not benefit from this improvement, specifically those for which the optimizer code aborts early before it has detected correlation (e.g. because of some other unrelated feature). Release note (sql change): CockroachDB will now report a hint in the error message if it encounters a correlated query that it does not support yet. Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
Build succeeded |
Fixes #27389.
Prior to this patch, invalid uses of SRFs as arguments to other
functions in ROWS FROM were not properly rejected, and were only
caught at evaluation time (i.e. much too late).
This patch fixes it by rejecting these uses early.
Release note (bug fix): invalid uses of set-generating functions in
FROM clauses are now reported with the same error code as PostgreSQL.