[HACKERS] Bug when dumping "empty" operator classes - Mailing list pgsql-hackers

From Daniel Gustafsson
Subject [HACKERS] Bug when dumping "empty" operator classes
Date
Msg-id D9E5FC64-7A37-4F3D-B946-7E4FB468F88A@yesql.se
Whole thread Raw
Responses Re: [HACKERS] Bug when dumping "empty" operator classes  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
While hacking on pg_upgrade in downstream Greenplum I ran into an error which
seems like an old, and obscure, bug in pg_dump (unrelated to pg_upgrade).
pg_dump generates incorrect SQL for an operator class which has no operators or
procedures, and which has the same column and storage types.  While it’s
arguably a pretty uninteresting operator class, it is allowed since we don’t
validate that all required operators/procedures are present.

The alter_generic test suite contains examples of opclasses which trigger this
behavior.  The below operator class:

  CREATE OPERATOR CLASS alt_opc1 FOR TYPE uuid USING hash AS STORAGE uuid;

..is dumped like this (after an ALTER .. RENAME, thus the new name):

  CREATE OPERATOR CLASS alt_opc3
      FOR TYPE uuid USING hash FAMILY alt_opc1 AS
      ;

The reason why this hasn’t been caught by the PostgreSQL pg_upgrade test is
because the schema in which the operator classes are created is dropped at the
end of the suite, removing the DROP cause pg_upgrade to fail with:

  pg_restore: [archiver (db)] could not execute query: ERROR:  syntax error at or near ";"
  LINE 3:     ;
              ^
      Command was: CREATE OPERATOR CLASS "alt_opc2"
      FOR TYPE "macaddr" USING "hash" FAMILY "alt_opc2" AS
      ;

The attached patch adds a belts-and-suspenders check in dumpOpclass() which
appends the STORAGE clause in case nothing had been added.  While adding
storage when it’s identical to the column type goes against the documentation,
it’s valid SQL and won’t break restore (and is handled by DefineOpClass()).
Validating the options fully would of course ensure that the dump always has
enough to render, but it also adds a lot of complexity (a quick look in the
archives doesn’t turn up many reports of it being a big problem).  The DROP in
alter_generic is also removed to exercise the code path, being able to
pg_upgrade what is executed in regression seem like a good idea.

cheers ./daniel


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: retry shm attach for windows (WAS: Re: [HACKERS] OK, so culicidaeis *still* broken)
Next
From: Robins Tharakan
Date:
Subject: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump