sql: Support view queries with star expansions

[a-robinson]

Copied over from #9921 (comment)

We currently don't expand out star selectors before persisting a new view's query. If a dependent table has columns added to it, that can break later queries from the view depending on how that dependent table was used.

It's kind of a fluke, but a simple star select works just fine as columns are added because the view descriptor only lists the original columns as its ResultColumns:

root@:26257> create table star (a int primary key, b int);
CREATE TABLE
root@:26257> create view v as select * from star;
CREATE VIEW
root@:26257> insert into star values (1, 1), (2, 2);
INSERT 2
root@:26257> select * from star;
+---+---+
| a | b |
+---+---+
| 1 | 1 |
| 2 | 2 |
+---+---+
(2 rows)
root@:26257> SELECT * FROM v;
+---+---+
| a | b |
+---+---+
| 1 | 1 |
| 2 | 2 |
+---+---+
(2 rows)
root@:26257> alter table star add column c int;
ALTER TABLE
root@:26257> SELECT * FROM star;
+---+---+------+
| a | b |  c   |
+---+---+------+
| 1 | 1 | NULL |
| 2 | 2 | NULL |
+---+---+------+
(2 rows)
root@:26257> SELECT * FROM v;
+---+---+
| a | b |
+---+---+
| 1 | 1 |
| 2 | 2 |
+---+---+
(2 rows)

On the other hand, if the * isn't at the end of the query, it can break the view as columns are added:

root@:26257> create table t (a int primary key, b int);
CREATE TABLE
root@:26257> insert into t values (1, 1), (2, 2);
INSERT 2
root@:26257> CREATE VIEW tv AS SELECT t1.*, t2.b AS t2b FROM t AS t1 JOIN t AS t2 ON t1.a = t2.a;
CREATE VIEW
root@:26257> select * from tv;
+---+---+-----+
| a | b | t2b |
+---+---+-----+
| 1 | 1 |   1 |
| 2 | 2 |   2 |
+---+---+-----+
(2 rows)
root@:26257> ALTER TABLE t ADD COLUMN c INT;
ALTER TABLE
root@:26257> SELECT t1.*, t2.b AS t2b FROM t AS t1 JOIN t AS t2 ON t1.a = t2.a;
+---+---+------+-----+
| a | b |  c   | t2b |
+---+---+------+-----+
| 1 | 1 | NULL |   1 |
| 2 | 2 | NULL |   2 |
+---+---+------+-----+
(2 rows)
root@:26257> SELECT * FROM tv;
+---+---+------+
| a | b | t2b  |
+---+---+------+
| 1 | 1 | NULL |
| 2 | 2 | NULL |
+---+---+------+
(2 rows)

The logic for expanding stars appears to be a little broader in scope than the logic for normalizing table names, so I'm not sure if there's as well-contained of a fix for it as for #9921 (beyond moving to a semantic representation, of course).

Is this the sort of thing that we should block the rollout of views for? Is it ok to leave as a known issue? Is there a fix that could reasonably be put in place for it?

@knz @dt @nvanbenschoten

Epic CRDB-2410

Jira issue: CRDB-6144

[a-robinson]

As briefly discussed in the SQL meeting yesterday, we should "release" views but should either mark the problem as a known issue or not allow creating views with a star-expansion in them.

To avoid potential backwards-compatibility issues down the road, it seems much safer to prevent creation of views with a star-expansion, even if they kind of work now for simple cases, because it could be tough to migrate them properly down the road. I'll send out a PR for that soon if no one disagrees.

[a-robinson]

I'm not going to be working on this for a while since it'll depend on some sort of intermediate representation being in place. Unassigning in the meantime.

[martinrode]

I want to +1 this.

It is painful to have to unfold broader tables in expressions.

[rytaft]

cc @kevin-v-ngo and @awoods187 for visibility into possible future feature work

[ajwerner]

For anybody who finds themselves here, have a look at #10083 (comment) which has some nice context.

[simplenotezy]

Not sure why, but when we're trying to create a view like so:

CREATE OR REPLACE VIEW public. "match" AS
SELECT
	swipe.swiper_id AS user_id,
	swipe.swiped_id AS match_id,
	swipe.swiper_cell AS user_cell,
	swipe.swiped_cell AS match_cell
FROM
	swipe
	JOIN active_users u1 ON u1.id = swipe.swiper_id
	JOIN active_users u2 ON u2.id = swipe.swiped_id
WHERE
	swipe.swiper_like
	AND swipe.swiped_like
	AND swipe.deleted_at IS NULL;

We are getting the same error:

Query 1 ERROR: ERROR:  unimplemented: views do not currently support * expressions
HINT:  You have attempted to use a feature that is not yet implemented.
See: https://go.crdb.dev/issue-v/10028/v22.1

Not sure why, since we're not using SELECT *

Update

Turns out the error was triggered because of the ON statement was pointing to a non-existing column (id - should have been user_id)

[rytaft]

Thanks, @simplenotezy -- I've opened #84178 to track this new issue.

[chengxiong-ruan]

Thanks for tracking with this issue. I think I'm running into the same problem when tracking column dependencies for UDFs :) glad I ended up here.

[vy-ton]

@chengxiong-ruan will any of the planned UDF work help lift this restriction?

[ajwerner]

@rharding6373 can we apply #95710 to fix this issue in exactly the same way?

[knz]

@rharding6373 can we apply #95710 to fix this issue in exactly the same way?

I have investigated this further today. No we cannot do it easily.

On the one hand, removing the condition on inViewDef makes the CREATE VIEW statement succeed and derive an appropriate number of columns for the view schema.

On the other hand, the moment we do ALTER ADD COLUMN on the table underlying the view, the columns don't align and we get an execution panic.

So handling views properly here means we need to rewrite the * in the SQL stored in the view descriptor, or use a different (binary) encoding of the AST.

I am going to check next whether the UDFs behave properly in that case too.

[ajwerner]

I do not understand. Postgres eagerly binds a * to the set of columns which exist at the time the view was defined. The view is not affected by adding a column to the underlying table. We ought to do what postgres does and not what was initially described.

[ajwerner]

I am going to check next whether the UDFs behave properly in that case too.

The story for UDFs is more interesting. Postgres has two approaches to UDFs. The "legacy" textual definition, we sometimes called late-bound (AS $$ ... $$), and the modern early-bound format BEGIN ATOMIC; ...; END. Cockroach, for better or for worse, chose to always early-bound UDF bodies, even when using the legacy syntax.

I assume what we care about is supporting the RETURNS TABLE syntax (note that SETOF <tablename> returns a single record value). In the late-bound type, with a statically stated table return type, changing the shape of the table behind the * will break the UDF. In the early-bound format, the star is expanded eagerly like in views and thus will continue to work and will not be affected by the added column.

[Lewenhaupt]

According to this thread this should be closed but it appears to still be an issue? Just tried adding a simple view Joining on one other table and selecting main_table.* and it gives me the same error as above and links to this issue.

Tried again with the same "star" table as in the original question and that does not work in my serverless cluster.

[rytaft]

@Lewenhaupt this issue is only fixed in CockroachDB v23.1.x. Serverless clusters have not yet been upgraded to v23.1.x, but the plan is for this to happen in the next couple of weeks.