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

On Tue, 15 Feb 2022 at 16:51, PG Bug reporting form <noreply(at)postgresql(dot)org> wrote:
> The following bug has been logged on the website:
>
> Bug reference: 17404
> Logged by: Pierre-Aurélien GEORGES
> Email address: pageorge(at)unice(dot)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.