Re: BUG #17404: CREATE OR REPLACE VIEW does not properly change the COLLATION of an existing field. - Mailing list pgsql-bugs

On Tue, 15 Feb 2022 at 16:51, PG Bug reporting form <noreply@postgresql.org> wrote:
> The following bug has been logged on the website:
>
> Bug reference:      17404
> Logged by:          Pierre-Aurélien GEORGES
> Email address:      pageorge@unice.fr
> PostgreSQL version: 12.4
> Operating system:   Debian 8.3.0-6 (x86_64-pc-linux-gnu)
> Description:
>
> 3 STEPS TO REPRODUCE :
> ========================
> create TABLE my_table (my_text TEXT);
> create VIEW my_view AS SELECT my_text COLLATE "C" FROM my_table;
> create OR REPLACE view my_view AS SELECT my_text COLLATE "en_US.utf8" FROM
> my_table;
>
> EXPECTED RESULT :
> ===================
> The COLLATION of "my_view"."my_text" should have been changed to
> "en_US.utf8".
> - OR -
> I should have got an error message telling me that it is not possible.
>
> OBSERVED RESULT :
> ===================
> Nothing (it silently fails).
>
> select table_schema, table_name, column_name, collation_name from
> information_schema.columns where collation_name is not null and
> table_name='my_view';
> clearly shows that the COLLATION has not been changed as requested.
>
> What the doc says about CREATE OR REPLACE VIEW :
> =============================================
> "The new query must generate the same columns that were generated by the
> existing view query (that is, the same column names in the same order and
> with the *same data types*), but it may add additional columns to the end of
> the list.
> *The calculations giving rise to the output columns may be completely
> different*."
>
> It doesn't say wether the COLLATION is considered being part of the "data
> types" or not, i.e. it doesn't say wether it's possible to change the
> COLLATION of an existing field or not.
> My tests (under Postgres v. 12.4) trying to do so showed that postgres says
> nothing and seems to accept the SQL statement, but nothing is changed in the
> database (it silently fails). From the user perspective, I consider such a
> silent failure the WORST situation possible : I would prefer either 1) to
> get an error message saying that it's not possible to change the COLLATION
> of an existing field - OR even better - 2) to actually change the COLLATION
> of the existing field, as requested.
>
> For this reason, I have also reported a documentation issue.

I find the checkViewTupleDesc() function do not check the collation[1].  So it
will not update the collation of columns.

[1]
static void
checkViewTupleDesc(TupleDesc newdesc, TupleDesc olddesc)
{
    int         i;

    if (newdesc->natts < olddesc->natts)
        ereport(ERROR,
                (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
                 errmsg("cannot drop columns from view")));

    for (i = 0; i < olddesc->natts; i++)
    {
        Form_pg_attribute newattr = TupleDescAttr(newdesc, i);
        Form_pg_attribute oldattr = TupleDescAttr(olddesc, i);

        /* XXX msg not right, but we don't support DROP COL on view anyway */
        if (newattr->attisdropped != oldattr->attisdropped)
            ereport(ERROR,
                    (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
                     errmsg("cannot drop columns from view")));

        if (strcmp(NameStr(newattr->attname), NameStr(oldattr->attname)) != 0)
            ereport(ERROR,
                    (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
                     errmsg("cannot change name of view column \"%s\" to \"%s\"",
                            NameStr(oldattr->attname),
                            NameStr(newattr->attname)),
                     errhint("Use ALTER VIEW ... RENAME COLUMN ... to change name of view column instead.")));
        /* XXX would it be safe to allow atttypmod to change?  Not sure */
        if (newattr->atttypid != oldattr->atttypid ||
            newattr->atttypmod != oldattr->atttypmod)
            ereport(ERROR,
                    (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
                     errmsg("cannot change data type of view column \"%s\" from %s to %s",
                            NameStr(oldattr->attname),
                            format_type_with_typemod(oldattr->atttypid,
                                                     oldattr->atttypmod),
                            format_type_with_typemod(newattr->atttypid,
                                                     newattr->atttypmod))));
        /* We can ignore the remaining attributes of an attribute... */
    }

    /*
     * We ignore the constraint fields.  The new view desc can't have any
     * constraints, and the only ones that could be on the old view are
     * defaults, which we are happy to leave in place.
     */
}

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.