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 <rahilasye...@gmail.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@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
