BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.

Started by PG Bug reporting formalmost 4 years ago8 messagesbugs
Jump to latest
#1PG Bug reporting form
noreply@postgresql.org

The following bug has been logged on the website:

Bug reference: 17486
Logged by: Nicolas Lutic
Email address: n.lutic@loxodata.com
PostgreSQL version: 14.3
Operating system: Debian 11
Description:

Hi Team,

I found something weird. Restoring a view fails if this view contains an
attribute without alias name.

Please find below the details to reproduce the problem:

psql -h localhost -c 'CREATE DATABASE demo;'

psql -h localhost -d demo

demo=# CREATE VIEW v_static_select as
WITH static_select as (
select
1 as foo,
'text'
)
select * from static_select;

demo=# \d+ v_static_select
View "public.v_static_select"
Column | Type | Collation | Nullable | Default | Storage |
Description

----------+---------+-----------+----------+---------+----------+-------------
foo | integer | | | | plain |
?column? | text | | | | extended |
View definition:
WITH static_select AS (
SELECT 1 AS foo,
'text'::text
)
SELECT static_select.foo,
static_select."?column?"
FROM static_select;

demo=# select * from v_static_select;
foo | ?column?
-----+----------
1 | text
(1 row)

pg_dump -h localhost -p5432 -U postgres -d demo -Fc -f /tmp/demo.dump

psql -h localhost -p5432 -U postgres -d demo -c 'DROP VIEW
v_static_select ;'

pg_restore -s -h localhost -p5432 -U postgres -d demo /tmp/demo.dump

pg_restore: error: could not execute query: ERROR: column
static_select.?column? does not exist
LINE 7: static_select."?column?"
^
Command was: CREATE VIEW public.v_static_select AS
WITH static_select AS (
SELECT 1 AS foo,
'text'::text
)
SELECT static_select.foo,
static_select."?column?"
FROM static_select;

Regards, Nicolas Lutic

#2Daniel Gustafsson
daniel@yesql.se
In reply to: PG Bug reporting form (#1)
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.

On 19 May 2022, at 14:03, PG Bug reporting form <noreply@postgresql.org> wrote:

I found something weird. Restoring a view fails if this view contains an
attribute without alias name.

This is not unique to 14, it can be reproduced further down as well.

pg_restore: error: could not execute query: ERROR: column
static_select.?column? does not exist
LINE 7: static_select."?column?"
^
Command was: CREATE VIEW public.v_static_select AS
WITH static_select AS (
SELECT 1 AS foo,
'text'::text
)
SELECT static_select.foo,
static_select."?column?"
FROM static_select;

Since there is no column name given, FigureColname() will do its best to figure
something out and the typecast to TEXT added will lead it to chose the column
type as the column name. This will lead the qualified "?column?" target col in
the view query to not resolve. When creating the view the ::text explicit cast
is added after column names are resolved so this only breaks on restoring the
view definition for the expanded SELECT *.

--
Daniel Gustafsson https://vmware.com/

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#2)
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.

Daniel Gustafsson <daniel@yesql.se> writes:

On 19 May 2022, at 14:03, PG Bug reporting form <noreply@postgresql.org> wrote:
I found something weird. Restoring a view fails if this view contains an
attribute without alias name.

This is not unique to 14, it can be reproduced further down as well.

Yeah, it's an ancient behavior; interesting that no one has complained
before. This code in ruleutils is assuming that FigureColname will
make the same choice at reload as it did before; which is shaky both
because we aren't necessarily printing the identical raw text, and
because there's no guarantee we won't change those rules in future.

Perhaps we should just tweak ruleutils so that the alias is always
printed for non-Var columns, even when it's "?column?". That's kind of
ugly, but if you wanted non-ugly you should have selected a better column
name to start with.

regards, tom lane

#4Daniel Gustafsson
daniel@yesql.se
In reply to: Tom Lane (#3)
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.

On 20 May 2022, at 16:34, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Perhaps we should just tweak ruleutils so that the alias is always
printed for non-Var columns, even when it's "?column?". That's kind of
ugly, but if you wanted non-ugly you should have selected a better column
name to start with.

That might be the path of least confusion, and as you rightly say, if you don't
like the ugliness then there is a very easy way to fix it.

--
Daniel Gustafsson https://vmware.com/

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#4)
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.

Daniel Gustafsson <daniel@yesql.se> writes:

On 20 May 2022, at 16:34, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Perhaps we should just tweak ruleutils so that the alias is always
printed for non-Var columns, even when it's "?column?". That's kind of
ugly, but if you wanted non-ugly you should have selected a better column
name to start with.

That might be the path of least confusion, and as you rightly say, if you don't
like the ugliness then there is a very easy way to fix it.

Hmm ... it's a very easy code change, but it results in a lot of
changes in the regression tests (and I've only tried the core tests
so far). Given the lack of prior complaints, I wonder if it's going
to be worth this much behavioral churn.

It'd be better if we could do this only when the name is actually
referenced somewhere, but I don't think that's an easy thing to
determine.

regards, tom lane

Attachments:

tweak-ruleutils-column-name-rule.patchtext/x-diff; charset=us-ascii; name=tweak-ruleutils-column-name-rule.patchDownload+2-2
regression.diffstext/x-diff; charset=us-ascii; name=regression.diffsDownload+48-46
#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#5)
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.

I wrote:

Hmm ... it's a very easy code change, but it results in a lot of
changes in the regression tests (and I've only tried the core tests
so far). Given the lack of prior complaints, I wonder if it's going
to be worth this much behavioral churn.

It'd be better if we could do this only when the name is actually
referenced somewhere, but I don't think that's an easy thing to
determine.

I thought of a compromise position that's not hard to implement:
change the behavior only if the SELECT output column name is *possibly*
visible elsewhere, which it is not in (for example) an EXISTS subquery.
This is easy to keep track of while descending the parse tree.
The attached quick-hack draft results in only one place changing in
the regression tests, and that's a place where a view's visible
column name is already "?column?", so whoever wrote that test case
didn't give a fig for prettiness anyway. This seems like it might be
an acceptable amount of behavioral churn.

Thoughts?

regards, tom lane

Attachments:

display-column-name-when-needed-wip.patchtext/x-diff; charset=us-ascii; name=display-column-name-when-needed-wip.patchDownload+43-30
#7Daniel Gustafsson
daniel@yesql.se
In reply to: Tom Lane (#6)
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.

On 20 May 2022, at 19:05, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I wrote:

Hmm ... it's a very easy code change, but it results in a lot of
changes in the regression tests (and I've only tried the core tests
so far). Given the lack of prior complaints, I wonder if it's going
to be worth this much behavioral churn.

It'd be better if we could do this only when the name is actually
referenced somewhere, but I don't think that's an easy thing to
determine.

I thought of a compromise position that's not hard to implement:
change the behavior only if the SELECT output column name is *possibly*
visible elsewhere, which it is not in (for example) an EXISTS subquery.

Nice one! I think that's even better than the previous version actually.
Skimming the patch it seems like a reasonable approach.

--
Daniel Gustafsson https://vmware.com/

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#7)
Re: BUG #17486: [pg_restore] Restoring a view fails if this view contains an attribute without alias name.

Daniel Gustafsson <daniel@yesql.se> writes:

On 20 May 2022, at 19:05, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I thought of a compromise position that's not hard to implement:
change the behavior only if the SELECT output column name is *possibly*
visible elsewhere, which it is not in (for example) an EXISTS subquery.

Nice one! I think that's even better than the previous version actually.
Skimming the patch it seems like a reasonable approach.

Pushed, thanks for looking.

regards, tom lane