sql: allow stars inside view definitions #97515
Conversation
|
This change is |
| ) AS SELECT t.a, t.b FROM test.public.t | ||
| star3 CREATE VIEW public.star3 ( | ||
| a | ||
| ) AS SELECT a FROM test.public.t ORDER BY t.* |
There was a problem hiding this comment.
I am noting how the stars inside ORDER BY and GROUP BY are preserved here. This means they are also perserved in UDFs.
I believe this is OK? But I think we need some semantic expert to double check. In any case, this choice is not specific to this PR so it should not block it.
knz
force-pushed
the
20230222-create-view-star
branch
from
09d2c5d
to
007378d
Compare
There was a problem hiding this comment.
Reviewed 1 of 4 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @ajwerner, @knz, @mgartner, and @rharding6373)
pkg/sql/opt/optbuilder/project.go line 151 at r1 (raw file):
aliases, exprs := b.expandStar(e.Expr, inScope) if b.insideFuncDef || b.insideViewDef {
I think there's another case in Builder.buildDataSource that needs to be changed (see #96375).
pkg/sql/logictest/testdata/logic_test/views line 439 at r1 (raw file):
statement ok CREATE VIEW star16 AS SELECT t1.a, t2.a AS a2 FROM t AS t1 JOIN t AS t2 ON t1.a IN (SELECT a FROM (SELECT * FROM t))
Could you also add the cases from #96375? (similar to the existing join test cases, but without aliases)
pkg/sql/logictest/testdata/logic_test/views line 486 at r1 (raw file):
Previously, knz (Raphael 'kena' Poss) wrote…
I am noting how the stars inside ORDER BY and GROUP BY are preserved here. This means they are also perserved in UDFs.
I believe this is OK? But I think we need some semantic expert to double check. In any case, this choice is not specific to this PR so it should not block it.
That's a good catch, and I think it's a bug.
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/optbuilder/project.go line 151 at r1 (raw file):
Previously, DrewKimball (Drew Kimball) wrote…
I think there's another case in
Builder.buildDataSourcethat needs to be changed (see #96375).
hum no that would be a regression - we currently allow views to reference subqueries withut an alias, so removing that here would be a regression from current behavior.
pkg/sql/logictest/testdata/logic_test/views line 439 at r1 (raw file):
Previously, DrewKimball (Drew Kimball) wrote…
Could you also add the cases from #96375? (similar to the existing join test cases, but without aliases)
i'll look into it.
pkg/sql/logictest/testdata/logic_test/views line 486 at r1 (raw file):
Previously, DrewKimball (Drew Kimball) wrote…
That's a good catch, and I think it's a bug.
i'll file an issue.
There was a problem hiding this comment.
Reviewable status:
pkg/sql/logictest/testdata/logic_test/views line 486 at r1 (raw file):
Previously, knz (Raphael 'kena' Poss) wrote…
i'll file an issue.
knz
force-pushed
the
20230222-create-view-star
branch
from
007378d
to
8298d85
Compare
There was a problem hiding this comment.
RFAL
Reviewable status:
pkg/sql/opt/optbuilder/project.go line 151 at r1 (raw file):
Previously, knz (Raphael 'kena' Poss) wrote…
hum no that would be a regression - we currently allow views to reference subqueries withut an alias, so removing that here would be a regression from current behavior.
I double checked - we do not need the special case related to #96375 here because CREATE VIEW separately rejects duplicate column names, so if there's an ambiguity it will be prevented this way.
pkg/sql/logictest/testdata/logic_test/views line 439 at r1 (raw file):
Previously, knz (Raphael 'kena' Poss) wrote…
i'll look into it.
Done.
There was a problem hiding this comment.
Very nice, thanks for all the test cases! I only have suggestions for a couple more.
Reviewable status:
pkg/sql/logictest/testdata/logic_test/views line 440 at r1 (raw file):
statement ok CREATE VIEW star16 AS SELECT t1.a, t2.a AS a2 FROM t AS t1 JOIN t AS t2 ON t1.a IN (SELECT a FROM (SELECT * FROM t))
Could you please add the following tests (or similar) to make sure schema changes and dependencies are handled correctly?
statement ok
ALTER TABLE t ADD COLUMN c INT;
statement error
ALTER TABLE t DROP COLUMN b;
# we expect the columns in the view to be unchanged
statement ok
ALTER TABLE t RENAME COLUMN b TO d;
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/optbuilder/project.go line 151 at r1 (raw file):
CREATE VIEW separately rejects duplicate column names
I think this will only apply to the top-level columns returned by the view, but it seems ok anyway since apparently views bind differently from UDFs: #96375 (comment)
pkg/sql/logictest/testdata/logic_test/views line 468 at r3 (raw file):
# This error is the same as in postgres.
[nit] postgres gives subquery in FROM must have an alias, so not the same error here.
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/optbuilder/project.go line 151 at r1 (raw file):
Previously, DrewKimball (Drew Kimball) wrote…
CREATE VIEW separately rejects duplicate column names
I think this will only apply to the top-level columns returned by the view, but it seems ok anyway since apparently views bind differently from UDFs: #96375 (comment)
I investigated this further and we have indeed an 'interesting' situation here:
select array[z.*]::string from (select * from (select a from t), (select a from t)) z;This example is interesting because there's just 1 unambiguous "outer" column, but we need to be careful "in the middle".
With my current code, we get invalid behavior as follows:
demo@127.0.0.1:26257/movr> show create view v;
table_name | create_statement
-------------+-----------------------------------------------------------------------------------------------
v | CREATE VIEW public.v (
| "array"
| ) AS SELECT
| ARRAY[z.*]::STRING
| FROM
| (SELECT a, a FROM (SELECT a FROM movr.public.t), (SELECT a FROM movr.public.t)) AS z
(1 row)
Time: 9ms total (execution 9ms / network 0ms)
demo@127.0.0.1:26257/movr> select * from v;
ERROR: column reference "a" is ambiguous (candidates: <anonymous>.a)
SQLSTATE: 42702
Which means my PR needs to solve a subset of #96375 before we consider the job done.
pkg/sql/logictest/testdata/logic_test/views line 440 at r1 (raw file):
Previously, rharding6373 (Rachael Harding) wrote…
Could you please add the following tests (or similar) to make sure schema changes and dependencies are handled correctly?
statement ok ALTER TABLE t ADD COLUMN c INT; statement error ALTER TABLE t DROP COLUMN b; # we expect the columns in the view to be unchanged statement ok ALTER TABLE t RENAME COLUMN b TO d;
Done.
pkg/sql/logictest/testdata/logic_test/views line 468 at r3 (raw file):
Previously, DrewKimball (Drew Kimball) wrote…
[nit] postgres gives
subquery in FROM must have an alias, so not the same error here.
Good point. Changed to add a test using pg's syntax.
knz
force-pushed
the
20230222-create-view-star
branch
from
8298d85
to
ce54cd5
Compare
knz
force-pushed
the
20230222-create-view-star
branch
from
ce54cd5
to
c737489
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/optbuilder/project.go line 151 at r1 (raw file):
Previously, knz (Raphael 'kena' Poss) wrote…
I investigated this further and we have indeed an 'interesting' situation here:
select array[z.*]::string from (select * from (select a from t), (select a from t)) z;This example is interesting because there's just 1 unambiguous "outer" column, but we need to be careful "in the middle".
With my current code, we get invalid behavior as follows:
demo@127.0.0.1:26257/movr> show create view v; table_name | create_statement -------------+----------------------------------------------------------------------------------------------- v | CREATE VIEW public.v ( | "array" | ) AS SELECT | ARRAY[z.*]::STRING | FROM | (SELECT a, a FROM (SELECT a FROM movr.public.t), (SELECT a FROM movr.public.t)) AS z (1 row) Time: 9ms total (execution 9ms / network 0ms) demo@127.0.0.1:26257/movr> select * from v; ERROR: column reference "a" is ambiguous (candidates: <anonymous>.a) SQLSTATE: 42702Which means my PR needs to solve a subset of #96375 before we consider the job done.
Rebased on top of #97544, which fixes this. PTAL.
knz
force-pushed
the
20230222-create-view-star
branch
from
c737489
to
ba086e2
Compare
Release note (bug fix): CockroachDB now supports using subqueries in UDFs without an AS clause, for consistency with the syntax supported outside of UDFs. (Subqueries without an AS clause is a CRDB-specific extension.)
Release note (sql change): It is now possible to use `*` inside CREATE VIEW. The list of columns is expanded at the time the view is created, so that new columns added after the view was defined are not included in the view. This behavior is the same as PostgreSQL.
knz
force-pushed
the
20230222-create-view-star
branch
from
ba086e2
to
d59c96f
Compare
|
TFYRs! bors r=DrewKimball,rharding6373 |
|
Build succeeded: |
Star expressions have been allowed in UDFs since cockroachdb#95710 and in views since cockroachdb#97515. This commit lifts a restriction that prevented table references from being resolved as TupleStars in SELECT lists inside UDFs and views. For example, an expression like `SELECT tbl FROM tbl` is now allowed inside views and UDFs. Fixes cockroachdb#104927 Release note (sql change): Table names are now allowed in SELECT lists inside view and UDF definitions.
104677: ui: Redirect Range Report page to Hot Ranges page r=gtr a=gtr Fixes: #102377. Previously, when a user selected a range ID from the Hot Ranges page, the left side menu would switch to Advanced Debug and the back button would also redirect to the Advanced Dedug page. This commit ensures that when a range ID is selected, the left side menu will stay on the Hot Ranges page and also changes the back button to redirect back to the Hot Ranges page. <img width="791" alt="image" src="https://github.com/cockroachdb/cockroach/assets/35943354/41afa924-7395-4101-a14c-bb4cdeccec0c"> Release note (ui change): The Range Report page (route `/reports/range/:rangeID`) shows the "Hot Ranges" menu item as highlighted in the left side menu. The back button in the Range Report page redirects back to the Hot Ranges page. 104929: opt: resolve TupleStars in UDFs and views r=mgartner a=mgartner Star expressions have been allowed in UDFs since #95710 and in views since #97515. This commit lifts a restriction that prevented table references from being resolved as TupleStars in SELECT lists inside UDFs and views. For example, an expression like `SELECT tbl FROM tbl` is now allowed inside views and UDFs. Fixes #104927 Fixes #97602 Release note (sql change): Table names are now allowed in SELECT lists inside view and UDF definitions. 104946: sql: update tenant_span builtins for consistent output r=arulajmani a=stevendanna The codec's TenantSpan() function was changed to avoid PrefixEnd(). Here, we update overloads of crdb_internal.tenant_span to all use TenantSpan() to avoid inconsistent output from different overloads. Note, I don't believe that the use of PrefixEnd() in this function constituted a problem in our current usage as the only real use of this function was in calls to crdb_internal.fingerprint() or crdb_internal.scan() where the EndKey is only used as an upper-bound for iteration. Informs #104928 Epic: none Release note: None Co-authored-by: gtr <gerardo@cockroachlabs.com> Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com> Co-authored-by: Steven Danna <danna@cockroachlabs.com>
First commit from #97544.
Fixes #10028.
Release note (sql change): It is now possible to use
*inside CREATE VIEW. The list of columns is expanded at the time the view is created, so that new columns added after the view was defined are not included in the view. This behavior is the same as PostgreSQL.