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

On Mon, Jan 23, 2017 at 4:23 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I wrote:
>> I spent some time fooling with this today and came up with the attached
>> patch. I think this is basically the direction we should go in, but
>> there are various loose ends:
>
> Here's an updated patch that's rebased against today's HEAD and addresses
> most of the loose ends:
>
>> 1. I adjusted a couple of existing regression tests whose results are
>> affected, but didn't do anything towards adding new tests showing the
>> desirable results of this change (as per the examples in this thread).
>
> Added some regression test cases. These are mostly designed to prove
> that coercion to text occurs where expected, but I did include a couple
> of queries that would have failed outright before.

+1. Thanks.

>
>> 2. I didn't do anything about docs, either, though maybe no change
>> is needed. One user-visible change from this is that queries should
>> never return any "unknown"-type columns to clients anymore. But I
>> think that is not documented now, so maybe there's nothing to change.
>
> The only thing I could find in the SGML docs that directly addresses
> unknown-type columns was a remark in the CREATE VIEW man page, which
> I've updated. I also added a section to typeconv.sgml to document
> the behavior.
>
>> 3. We need to look at whether pg_upgrade is affected. I think we
>> might be able to let it just ignore the issue for views, since they'd
>> get properly updated during the dump and reload anyway. But if someone
>> had an actual table (or matview) with an "unknown" column, that would
>> be a pg_upgrade hazard, because "unknown" doesn't store like "text".
>
> I tested and found that simple views with unknown columns seem to update
> sanely if we just let pg_upgrade dump and restore them. So I suggest we
> allow that to happen. There might be cases where dependent views behave
> unexpectedly after such a conversion, but I think that would be rare
> enough that it's not worth forcing users to fix these views by hand before
> updating. However, tables with unknown columns would fail the upgrade
> (since we'd reject the CREATE TABLE command) while matviews would be
> accepted but then the DDL wouldn't match the physical data storage.
> So I added code to pg_upgrade to check for those cases and refuse to
> proceed. This is almost a straight copy-and-paste of the existing
> pg_upgrade code for checking for "line" columns.
>

Following error message might be misleading,

postgres=# create table t1 (a unknown);
ERROR: column "a" has pseudo-type unknown

UNKNOWN is not exactly a pseudo-type.

postgres=# select typname, typtype from pg_type where typname =
'unknown' or typname = 'any';
typname | typtype
---------+---------
unknown | b
any | p
(2 rows)

In your earlier mail, you had raised the point about marking unknown
as a pseudo-type. But till we actually do that, it would be better not
to mention 'unknown' as a pseudo-type.

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