sql: views created using a DB context can't be selected from outside that DB context

[a-robinson]

This goes back to our original conversation about semantic vs syntactic encoding on the RFC PR. We're currently relying on the parser to format a text-based representation of the query to be saved for later, but it's not clear to me that the parser has enough context to qualify the table names in a view's query with their databases if they weren't fully qualified by the user.

It may require some awkward hoop-jumping to persist the properly-qualified name given that the parser doesn't have the session state. Any suggestions on this?

Basic repro example:

root@:26257> create database test;
CREATE DATABASE
root@:26257> set database = test;
SET
root@:26257> CREATE TABLE kv (k INT PRIMARY KEY, v INT);
CREATE TABLE
root@:26257> INSERT INTO kv VALUES (1, 2), (3, 4), (5, 6), (7, 8);
INSERT 4
root@:26257> CREATE VIEW kview AS SELECT * FROM kv;
CREATE VIEW
root@:26257> SELECT * FROM test.kview;
+---+---+
| k | v |
+---+---+
| 1 | 2 |
| 3 | 4 |
| 5 | 6 |
| 7 | 8 |
+---+---+
root@:26257> set database = foo;
SET
root@:26257> select * from test.kview;
pq: table "kv" does not exist

@dt @knz

[knz]

I think it would be fair to normalize names, which would add the missing db name prefix, before storing the query in the view descriptor.

[a-robinson]

Right, but do you have any thoughts on how? It feels kind of gross modifying the formatter to have a FmtFlag for database normalization, but I'm not seeing many other answers.

[knz]

This needs to be a separate phase than Format(). You need to:

1. create a new visitor that normalizes table names (just that) in the AST
2. run this visitor
3. Format the result as usual

I acknowledge that the 3-step idea above looks simpler than the code needed to do it. The "better solution" is one more use case for an IR that does this normalization stuff without involving all the planNode madness. Unfortunately this will only happen in a few weeks at the earliest...

[a-robinson]

A few updates from yesterday's adventures that I wasn't able to send out because I didn't have internet:

1. Adapting Walk for this, as we discussed, continues to be very awkward. I started adding WalkTableExpr logic to squeeze TableExprs into the Walk traversal (a-robinson@2b461b2), but quickly found that more than just WalkTableExpr was needed. We'd also need to get into adding something like WalkJoinCond to support walking into a JoinTableExpr's Cond member, which potentially contains full expressions such as table selects that would need to be normalized. Seeing more exceptions like this makes me think that expanding Walk to actually traverse an entire parse tree is abusing it, likely error prone, and pretty badly duplicating the logic used to build the query plan.
2. Adapting Format for it was pretty straightforward. I threw together sql: Persist DB names to view descriptors with new Format flag #10009 as a proof of concept.
3. I thought of another problem with the current approach towards the end of my adventure that doesn't look quite as easy to fix. We currently don't expand out star selectors before persisting the 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.

For example, 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 (beyond moving to a semantic representation, of course).

[a-robinson]

Closing now that #10009 is in. I've opened #10028 to track the star expansion issue raised in my last comment.