Re: Assignment of valid collation for SET operations on queries with UNKNOWN types.

The way this patch has been written, it doesn't allow creating tables
with unknown type columns, which was allowed earlier. That breaks
backward compatibility. Users, who have created such tables will face
problems while loading dumps from earlier versions. pg_upgrade might
be an issue, but we may modify it to convert those columns to text.
Given that a column with unknown type is pretty much useless unless
casted to some other type, there may not be many such users out there.
But we should probably add a notice in release notes.

+-- check coercion of UNKNOWN type to text for literal constants
+create view v as select 'abc' a;
+create view v11 as select 'def' a;
+select a from v UNION select a from v11;
+

The comment says that it's testing coercion of unknown to text, but
nothing in the test guarantees that constant literals were casted to
text. The union could give the expected result if code merging the two
views knew how to handle unknown types. Instead \d v or \d v11 would
be a better test to test what the comment says. If we want to test
such a union the test is fine, but the comment needs to change.

For a materialized view you may have to modify
transformCreateTableAsStmt() to modify at targetlist of the query
similar to DefineVirtualRelation(), but I think that can be done as a
separate patch.

You might want to add some testcases to test the error report e.g.
(not necessarily in the same form) create view sv as select
relname::unknown from pg_class;
ERROR: column "relname" has type "unknown"

On Wed, Dec 14, 2016 at 3:32 PM, Rahila Syed <rahilasyed90(at)gmail(dot)com> wrote:
> Hello,
>
> Thank you for comments.
>
>>There is a similar code pattern for materialized views, see
>>create_ctas_nodata() where the attribute list is built
> create_ctas_nodata() is for creation of materialized views WITH NO DATA.
> For other materialized views and CREATE TABLE AS, column definitions are
> built in
> intorel_startup() function which has different code from that of CREATE VIEW
> which
> the patch deals with.
>
> Limiting the scope of the patch to include changing the type of literal
> constants
> to text only for plain views. Also, error out when column with UNKNOWN type
> is
> being created for other relations like tables and materialized views.
>
>>And actually, shouldn't this be just a plain error?
> Changed it to error in the attached patch.
>
>>Your patch has no regression tests, surely you want some to stress
>>this code path
> Added regression tests in the attached patch.
>
> Also adding this patch to CF 2017-01
>
> Thank you,
> Rahila Syed
>
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company