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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: Rahila Syed <rahilasyed90(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Assignment of valid collation for SET operations on queries with UNKNOWN types.
Date: 2017-01-22 22:53:20
Message-ID: 20227.1485125600@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

> 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.

I think this is committable now; the other loose ends can be dealt
with in follow-on patches. Does anyone want to review it?

regards, tom lane

Attachment Content-Type Size
no-more-UNKNOWN-output-columns-2.patch text/x-diff 33.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Nasby 2017-01-22 22:59:44 Re: Online enabling of page level checksums
Previous Message Jim Nasby 2017-01-22 22:51:53 Re: Protect syscache from bloating with negative cache entries