Horton

Thread

Commits

  1. Show 'AS "?column?"' explicitly when it's important.

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

    The Post Office <noreply@postgresql.org> — 2022-05-19T12:03:37Z

    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

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

    Daniel Gustafsson <daniel@yesql.se> — 2022-05-20T12:51:08Z

    > 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/

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-20T14:34:20Z

    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

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

    Daniel Gustafsson <daniel@yesql.se> — 2022-05-20T15:07:29Z

    > 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/

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-20T15:19:33Z

    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

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-20T17:05:58Z

    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

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

    Daniel Gustafsson <daniel@yesql.se> — 2022-05-20T19:06:53Z

    > 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/

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-21T18:47:02Z

    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