On Mon, Jan 23, 2017 at 4:23 AM, Tom Lane <t...@sss.pgh.pa.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
