Re: Fixing contrib/isn for float8-pass-by-value

Greg Stark <greg.stark@enterprisedb.com> writes:
> I was going to say something like that but couldn't come up with a
> nice syntax. The syntax you gave seems obvious in retrospect.

Here's a draft patch for this.  It actually works out pretty well --
the bulk of the diff is just to allow processing the CREATE TYPE
parameters in an order that's independent of the syntax.

> I wonder if this should be tied in some way with creating binary-
> compatible casts or anything. Probably not since the data type can
> just make them itself.

Yeah.  I thought for awhile about copying *all* the create-type
parameters from the LIKE type, but soon decided it was a bad idea.
We should keep the inheritance to a minimal level, just the basic
type-storage parameters.

            regards, tom lane

Index: contrib/isn/isn.sql.in
===================================================================
RCS file: /cvsroot/pgsql/contrib/isn/isn.sql.in,v
retrieving revision 1.8
diff -c -r1.8 isn.sql.in
*** contrib/isn/isn.sql.in    13 Nov 2007 04:24:28 -0000    1.8
--- contrib/isn/isn.sql.in    28 Nov 2008 22:03:45 -0000
***************
*** 28,36 ****
  CREATE TYPE ean13 (
      INPUT = ean13_in,
      OUTPUT = ean13_out,
!     INTERNALLENGTH = 8,
!     ALIGNMENT = double,
!     STORAGE = PLAIN
  );
  COMMENT ON TYPE ean13
      IS 'International European Article Number (EAN13)';
--- 28,34 ----
  CREATE TYPE ean13 (
      INPUT = ean13_in,
      OUTPUT = ean13_out,
!     LIKE = pg_catalog.int8
  );
  COMMENT ON TYPE ean13
      IS 'International European Article Number (EAN13)';
***************
*** 48,56 ****
  CREATE TYPE isbn13 (
      INPUT = isbn13_in,
      OUTPUT = ean13_out,
!     INTERNALLENGTH = 8,
!     ALIGNMENT = double,
!     STORAGE = PLAIN
  );
  COMMENT ON TYPE isbn13
      IS 'International Standard Book Number 13 (ISBN13)';
--- 46,52 ----
  CREATE TYPE isbn13 (
      INPUT = isbn13_in,
      OUTPUT = ean13_out,
!     LIKE = pg_catalog.int8
  );
  COMMENT ON TYPE isbn13
      IS 'International Standard Book Number 13 (ISBN13)';
***************
*** 68,76 ****
  CREATE TYPE ismn13 (
      INPUT = ismn13_in,
      OUTPUT = ean13_out,
!     INTERNALLENGTH = 8,
!     ALIGNMENT = double,
!     STORAGE = PLAIN
  );
  COMMENT ON TYPE ismn13
      IS 'International Standard Music Number 13 (ISMN13)';
--- 64,70 ----
  CREATE TYPE ismn13 (
      INPUT = ismn13_in,
      OUTPUT = ean13_out,
!     LIKE = pg_catalog.int8
  );
  COMMENT ON TYPE ismn13
      IS 'International Standard Music Number 13 (ISMN13)';
***************
*** 88,96 ****
  CREATE TYPE issn13 (
      INPUT = issn13_in,
      OUTPUT = ean13_out,
!     INTERNALLENGTH = 8,
!     ALIGNMENT = double,
!     STORAGE = PLAIN
  );
  COMMENT ON TYPE issn13
      IS 'International Standard Serial Number 13 (ISSN13)';
--- 82,88 ----
  CREATE TYPE issn13 (
      INPUT = issn13_in,
      OUTPUT = ean13_out,
!     LIKE = pg_catalog.int8
  );
  COMMENT ON TYPE issn13
      IS 'International Standard Serial Number 13 (ISSN13)';
***************
*** 110,118 ****
  CREATE TYPE isbn (
      INPUT = isbn_in,
      OUTPUT = isn_out,
!     INTERNALLENGTH = 8,
!     ALIGNMENT = double,
!     STORAGE = PLAIN
  );
  COMMENT ON TYPE isbn
      IS 'International Standard Book Number (ISBN)';
--- 102,108 ----
  CREATE TYPE isbn (
      INPUT = isbn_in,
      OUTPUT = isn_out,
!     LIKE = pg_catalog.int8
  );
  COMMENT ON TYPE isbn
      IS 'International Standard Book Number (ISBN)';
***************
*** 130,138 ****
  CREATE TYPE ismn (
      INPUT = ismn_in,
      OUTPUT = isn_out,
!     INTERNALLENGTH = 8,
!     ALIGNMENT = double,
!     STORAGE = PLAIN
  );
  COMMENT ON TYPE ismn
      IS 'International Standard Music Number (ISMN)';
--- 120,126 ----
  CREATE TYPE ismn (
      INPUT = ismn_in,
      OUTPUT = isn_out,
!     LIKE = pg_catalog.int8
  );
  COMMENT ON TYPE ismn
      IS 'International Standard Music Number (ISMN)';
***************
*** 150,158 ****
  CREATE TYPE issn (
      INPUT = issn_in,
      OUTPUT = isn_out,
!     INTERNALLENGTH = 8,
!     ALIGNMENT = double,
!     STORAGE = PLAIN
  );
  COMMENT ON TYPE issn
      IS 'International Standard Serial Number (ISSN)';
--- 138,144 ----
  CREATE TYPE issn (
      INPUT = issn_in,
      OUTPUT = isn_out,
!     LIKE = pg_catalog.int8
  );
  COMMENT ON TYPE issn
      IS 'International Standard Serial Number (ISSN)';
***************
*** 170,178 ****
  CREATE TYPE upc (
      INPUT = upc_in,
      OUTPUT = isn_out,
!     INTERNALLENGTH = 8,
!     ALIGNMENT = double,
!     STORAGE = PLAIN
  );
  COMMENT ON TYPE upc
      IS 'Universal Product Code (UPC)';
--- 156,162 ----
  CREATE TYPE upc (
      INPUT = upc_in,
      OUTPUT = isn_out,
!     LIKE = pg_catalog.int8
  );
  COMMENT ON TYPE upc
      IS 'Universal Product Code (UPC)';
Index: doc/src/sgml/ref/create_type.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/create_type.sgml,v
retrieving revision 1.78
diff -c -r1.78 create_type.sgml
*** doc/src/sgml/ref/create_type.sgml    14 Nov 2008 10:22:46 -0000    1.78
--- doc/src/sgml/ref/create_type.sgml    28 Nov 2008 22:03:45 -0000
***************
*** 39,44 ****
--- 39,45 ----
      [ , PASSEDBYVALUE ]
      [ , ALIGNMENT = <replaceable class="parameter">alignment</replaceable> ]
      [ , STORAGE = <replaceable class="parameter">storage</replaceable> ]
+     [ , LIKE = <replaceable class="parameter">like_type</replaceable> ]
      [ , CATEGORY = <replaceable class="parameter">category</replaceable> ]
      [ , PREFERRED = <replaceable class="parameter">preferred</replaceable> ]
      [ , DEFAULT = <replaceable class="parameter">default</replaceable> ]
***************
*** 291,296 ****
--- 292,312 ----
    </para>

    <para>
+    The <replaceable class="parameter">like_type</replaceable> parameter
+    provides an alternative method for specifying the basic representation
+    properties of a data type: copy them from some existing type. The values of
+    <replaceable class="parameter">internallength</replaceable>,
+    <replaceable class="parameter">passedbyvalue</replaceable>,
+    <replaceable class="parameter">alignment</replaceable>, and
+    <replaceable class="parameter">storage</replaceable> are copied from the
+    named type.  (It is possible, though usually undesirable, to override
+    some of these values by specifying them along with the <literal>LIKE</>
+    clause.)  Specifying representation this way is especially useful when
+    the low-level implementation of the new type <quote>piggybacks</> on an
+    existing type in some fashion.
+   </para>
+
+   <para>
     The <replaceable class="parameter">category</replaceable> and
     <replaceable class="parameter">preferred</replaceable> parameters can be
     used to help control which implicit cast will be applied in ambiguous
***************
*** 525,530 ****
--- 541,562 ----
     </varlistentry>

     <varlistentry>
+     <term><replaceable class="parameter">like_type</replaceable></term>
+     <listitem>
+      <para>
+       The name of an existing data type that the new type will have the
+       same representation as.  The values of
+       <replaceable class="parameter">internallength</replaceable>,
+       <replaceable class="parameter">passedbyvalue</replaceable>,
+       <replaceable class="parameter">alignment</replaceable>, and
+       <replaceable class="parameter">storage</replaceable>
+       are copied from that type, unless overridden by explicit
+       specification elsewhere in this <command>CREATE TYPE</> command.
+      </para>
+     </listitem>
+    </varlistentry>
+
+    <varlistentry>
      <term><replaceable class="parameter">category</replaceable></term>
      <listitem>
       <para>
Index: src/backend/commands/typecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/typecmds.c,v
retrieving revision 1.126
diff -c -r1.126 typecmds.c
*** src/backend/commands/typecmds.c    2 Nov 2008 01:45:28 -0000    1.126
--- src/backend/commands/typecmds.c    28 Nov 2008 22:03:45 -0000
***************
*** 100,106 ****
      char       *typeName;
      Oid            typeNamespace;
      int16        internalLength = -1;    /* default: variable-length */
-     Oid            elemType = InvalidOid;
      List       *inputName = NIL;
      List       *outputName = NIL;
      List       *receiveName = NIL;
--- 100,105 ----
***************
*** 108,120 ****
      List       *typmodinName = NIL;
      List       *typmodoutName = NIL;
      List       *analyzeName = NIL;
-     char       *defaultValue = NULL;
-     bool        byValue = false;
      char        category = TYPCATEGORY_USER;
      bool        preferred = false;
      char        delimiter = DEFAULT_TYPDELIM;
      char        alignment = 'i';    /* default alignment */
      char        storage = 'p';    /* default TOAST storage method */
      Oid            inputOid;
      Oid            outputOid;
      Oid            receiveOid = InvalidOid;
--- 107,137 ----
      List       *typmodinName = NIL;
      List       *typmodoutName = NIL;
      List       *analyzeName = NIL;
      char        category = TYPCATEGORY_USER;
      bool        preferred = false;
      char        delimiter = DEFAULT_TYPDELIM;
+     Oid            elemType = InvalidOid;
+     char       *defaultValue = NULL;
+     bool        byValue = false;
      char        alignment = 'i';    /* default alignment */
      char        storage = 'p';    /* default TOAST storage method */
+     DefElem       *likeTypeEl = NULL;
+     DefElem       *internalLengthEl = NULL;
+     DefElem       *inputNameEl = NULL;
+     DefElem       *outputNameEl = NULL;
+     DefElem       *receiveNameEl = NULL;
+     DefElem       *sendNameEl = NULL;
+     DefElem       *typmodinNameEl = NULL;
+     DefElem       *typmodoutNameEl = NULL;
+     DefElem       *analyzeNameEl = NULL;
+     DefElem       *categoryEl = NULL;
+     DefElem       *preferredEl = NULL;
+     DefElem       *delimiterEl = NULL;
+     DefElem       *elemTypeEl = NULL;
+     DefElem       *defaultValueEl = NULL;
+     DefElem       *byValueEl = NULL;
+     DefElem       *alignmentEl = NULL;
+     DefElem       *storageEl = NULL;
      Oid            inputOid;
      Oid            outputOid;
      Oid            receiveOid = InvalidOid;
***************
*** 124,133 ****
      Oid            analyzeOid = InvalidOid;
      char       *array_type;
      Oid            array_oid;
-     ListCell   *pl;
      Oid            typoid;
      Oid            resulttype;
      Relation    pg_type;

      /*
       * As of Postgres 8.4, we require superuser privilege to create a base
--- 141,150 ----
      Oid            analyzeOid = InvalidOid;
      char       *array_type;
      Oid            array_oid;
      Oid            typoid;
      Oid            resulttype;
      Relation    pg_type;
+     ListCell   *pl;

      /*
       * As of Postgres 8.4, we require superuser privilege to create a base
***************
*** 202,312 ****
                       errmsg("type \"%s\" already exists", typeName)));
      }

      foreach(pl, parameters)
      {
          DefElem    *defel = (DefElem *) lfirst(pl);

!         if (pg_strcasecmp(defel->defname, "internallength") == 0)
!             internalLength = defGetTypeLength(defel);
          else if (pg_strcasecmp(defel->defname, "input") == 0)
!             inputName = defGetQualifiedName(defel);
          else if (pg_strcasecmp(defel->defname, "output") == 0)
!             outputName = defGetQualifiedName(defel);
          else if (pg_strcasecmp(defel->defname, "receive") == 0)
!             receiveName = defGetQualifiedName(defel);
          else if (pg_strcasecmp(defel->defname, "send") == 0)
!             sendName = defGetQualifiedName(defel);
          else if (pg_strcasecmp(defel->defname, "typmod_in") == 0)
!             typmodinName = defGetQualifiedName(defel);
          else if (pg_strcasecmp(defel->defname, "typmod_out") == 0)
!             typmodoutName = defGetQualifiedName(defel);
          else if (pg_strcasecmp(defel->defname, "analyze") == 0 ||
                   pg_strcasecmp(defel->defname, "analyse") == 0)
!             analyzeName = defGetQualifiedName(defel);
          else if (pg_strcasecmp(defel->defname, "category") == 0)
!         {
!             char       *p = defGetString(defel);
!
!             category = p[0];
!             /* restrict to non-control ASCII */
!             if (category < 32 || category > 126)
!                 ereport(ERROR,
!                         (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
!                          errmsg("invalid type category \"%s\": must be simple ASCII",
!                                 p)));
!         }
          else if (pg_strcasecmp(defel->defname, "preferred") == 0)
!             preferred = defGetBoolean(defel);
          else if (pg_strcasecmp(defel->defname, "delimiter") == 0)
!         {
!             char       *p = defGetString(defel);
!
!             delimiter = p[0];
!             /* XXX shouldn't we restrict the delimiter? */
!         }
          else if (pg_strcasecmp(defel->defname, "element") == 0)
!         {
!             elemType = typenameTypeId(NULL, defGetTypeName(defel), NULL);
!             /* disallow arrays of pseudotypes */
!             if (get_typtype(elemType) == TYPTYPE_PSEUDO)
!                 ereport(ERROR,
!                         (errcode(ERRCODE_DATATYPE_MISMATCH),
!                          errmsg("array element type cannot be %s",
!                                 format_type_be(elemType))));
!         }
          else if (pg_strcasecmp(defel->defname, "default") == 0)
!             defaultValue = defGetString(defel);
          else if (pg_strcasecmp(defel->defname, "passedbyvalue") == 0)
!             byValue = defGetBoolean(defel);
          else if (pg_strcasecmp(defel->defname, "alignment") == 0)
!         {
!             char       *a = defGetString(defel);
!
!             /*
!              * Note: if argument was an unquoted identifier, parser will have
!              * applied translations to it, so be prepared to recognize
!              * translated type names as well as the nominal form.
!              */
!             if (pg_strcasecmp(a, "double") == 0 ||
!                 pg_strcasecmp(a, "float8") == 0 ||
!                 pg_strcasecmp(a, "pg_catalog.float8") == 0)
!                 alignment = 'd';
!             else if (pg_strcasecmp(a, "int4") == 0 ||
!                      pg_strcasecmp(a, "pg_catalog.int4") == 0)
!                 alignment = 'i';
!             else if (pg_strcasecmp(a, "int2") == 0 ||
!                      pg_strcasecmp(a, "pg_catalog.int2") == 0)
!                 alignment = 's';
!             else if (pg_strcasecmp(a, "char") == 0 ||
!                      pg_strcasecmp(a, "pg_catalog.bpchar") == 0)
!                 alignment = 'c';
!             else
!                 ereport(ERROR,
!                         (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
!                          errmsg("alignment \"%s\" not recognized", a)));
!         }
          else if (pg_strcasecmp(defel->defname, "storage") == 0)
!         {
!             char       *a = defGetString(defel);
!
!             if (pg_strcasecmp(a, "plain") == 0)
!                 storage = 'p';
!             else if (pg_strcasecmp(a, "external") == 0)
!                 storage = 'e';
!             else if (pg_strcasecmp(a, "extended") == 0)
!                 storage = 'x';
!             else if (pg_strcasecmp(a, "main") == 0)
!                 storage = 'm';
!             else
!                 ereport(ERROR,
!                         (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
!                          errmsg("storage \"%s\" not recognized", a)));
!         }
          else
              ereport(WARNING,
                      (errcode(ERRCODE_SYNTAX_ERROR),
                       errmsg("type attribute \"%s\" not recognized",
                              defel->defname)));
      }

      /*
--- 219,393 ----
                       errmsg("type \"%s\" already exists", typeName)));
      }

+     /* Extract the parameters from the parameter list */
      foreach(pl, parameters)
      {
          DefElem    *defel = (DefElem *) lfirst(pl);
+         DefElem   **defelp;

!         if (pg_strcasecmp(defel->defname, "like") == 0)
!             defelp = &likeTypeEl;
!         else if (pg_strcasecmp(defel->defname, "internallength") == 0)
!             defelp = &internalLengthEl;
          else if (pg_strcasecmp(defel->defname, "input") == 0)
!             defelp = &inputNameEl;
          else if (pg_strcasecmp(defel->defname, "output") == 0)
!             defelp = &outputNameEl;
          else if (pg_strcasecmp(defel->defname, "receive") == 0)
!             defelp = &receiveNameEl;
          else if (pg_strcasecmp(defel->defname, "send") == 0)
!             defelp = &sendNameEl;
          else if (pg_strcasecmp(defel->defname, "typmod_in") == 0)
!             defelp = &typmodinNameEl;
          else if (pg_strcasecmp(defel->defname, "typmod_out") == 0)
!             defelp = &typmodoutNameEl;
          else if (pg_strcasecmp(defel->defname, "analyze") == 0 ||
                   pg_strcasecmp(defel->defname, "analyse") == 0)
!             defelp = &analyzeNameEl;
          else if (pg_strcasecmp(defel->defname, "category") == 0)
!             defelp = &categoryEl;
          else if (pg_strcasecmp(defel->defname, "preferred") == 0)
!             defelp = &preferredEl;
          else if (pg_strcasecmp(defel->defname, "delimiter") == 0)
!             defelp = &delimiterEl;
          else if (pg_strcasecmp(defel->defname, "element") == 0)
!             defelp = &elemTypeEl;
          else if (pg_strcasecmp(defel->defname, "default") == 0)
!             defelp = &defaultValueEl;
          else if (pg_strcasecmp(defel->defname, "passedbyvalue") == 0)
!             defelp = &byValueEl;
          else if (pg_strcasecmp(defel->defname, "alignment") == 0)
!             defelp = &alignmentEl;
          else if (pg_strcasecmp(defel->defname, "storage") == 0)
!             defelp = &storageEl;
          else
+         {
+             /* WARNING, not ERROR, for historical backwards-compatibility */
              ereport(WARNING,
                      (errcode(ERRCODE_SYNTAX_ERROR),
                       errmsg("type attribute \"%s\" not recognized",
                              defel->defname)));
+             continue;
+         }
+         if (*defelp != NULL)
+             ereport(ERROR,
+                     (errcode(ERRCODE_SYNTAX_ERROR),
+                      errmsg("conflicting or redundant options")));
+         *defelp = defel;
+     }
+
+     /*
+      * Now interpret the options; we do this separately so that LIKE can
+      * be overridden by other options regardless of the ordering in the
+      * parameter list.
+      */
+     if (likeTypeEl)
+     {
+         Type     likeType;
+         Form_pg_type likeForm;
+
+         likeType = typenameType(NULL, defGetTypeName(likeTypeEl), NULL);
+         likeForm = (Form_pg_type) GETSTRUCT(likeType);
+         internalLength = likeForm->typlen;
+         byValue = likeForm->typbyval;
+         alignment = likeForm->typalign;
+         storage = likeForm->typstorage;
+         ReleaseSysCache(likeType);
+     }
+     if (internalLengthEl)
+         internalLength = defGetTypeLength(internalLengthEl);
+     if (inputNameEl)
+         inputName = defGetQualifiedName(inputNameEl);
+     if (outputNameEl)
+         outputName = defGetQualifiedName(outputNameEl);
+     if (receiveNameEl)
+         receiveName = defGetQualifiedName(receiveNameEl);
+     if (sendNameEl)
+         sendName = defGetQualifiedName(sendNameEl);
+     if (typmodinNameEl)
+         typmodinName = defGetQualifiedName(typmodinNameEl);
+     if (typmodoutNameEl)
+         typmodoutName = defGetQualifiedName(typmodoutNameEl);
+     if (analyzeNameEl)
+         analyzeName = defGetQualifiedName(analyzeNameEl);
+     if (categoryEl)
+     {
+         char       *p = defGetString(categoryEl);
+
+         category = p[0];
+         /* restrict to non-control ASCII */
+         if (category < 32 || category > 126)
+             ereport(ERROR,
+                     (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                      errmsg("invalid type category \"%s\": must be simple ASCII",
+                             p)));
+     }
+     if (preferredEl)
+         preferred = defGetBoolean(preferredEl);
+     if (delimiterEl)
+     {
+         char       *p = defGetString(delimiterEl);
+
+         delimiter = p[0];
+         /* XXX shouldn't we restrict the delimiter? */
+     }
+     if (elemTypeEl)
+     {
+         elemType = typenameTypeId(NULL, defGetTypeName(elemTypeEl), NULL);
+         /* disallow arrays of pseudotypes */
+         if (get_typtype(elemType) == TYPTYPE_PSEUDO)
+             ereport(ERROR,
+                     (errcode(ERRCODE_DATATYPE_MISMATCH),
+                      errmsg("array element type cannot be %s",
+                             format_type_be(elemType))));
+     }
+     if (defaultValueEl)
+         defaultValue = defGetString(defaultValueEl);
+     if (byValueEl)
+         byValue = defGetBoolean(byValueEl);
+     if (alignmentEl)
+     {
+         char       *a = defGetString(alignmentEl);
+
+         /*
+          * Note: if argument was an unquoted identifier, parser will have
+          * applied translations to it, so be prepared to recognize
+          * translated type names as well as the nominal form.
+          */
+         if (pg_strcasecmp(a, "double") == 0 ||
+             pg_strcasecmp(a, "float8") == 0 ||
+             pg_strcasecmp(a, "pg_catalog.float8") == 0)
+             alignment = 'd';
+         else if (pg_strcasecmp(a, "int4") == 0 ||
+                  pg_strcasecmp(a, "pg_catalog.int4") == 0)
+             alignment = 'i';
+         else if (pg_strcasecmp(a, "int2") == 0 ||
+                  pg_strcasecmp(a, "pg_catalog.int2") == 0)
+             alignment = 's';
+         else if (pg_strcasecmp(a, "char") == 0 ||
+                  pg_strcasecmp(a, "pg_catalog.bpchar") == 0)
+             alignment = 'c';
+         else
+             ereport(ERROR,
+                     (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                      errmsg("alignment \"%s\" not recognized", a)));
+     }
+     if (storageEl)
+     {
+         char       *a = defGetString(storageEl);
+
+         if (pg_strcasecmp(a, "plain") == 0)
+             storage = 'p';
+         else if (pg_strcasecmp(a, "external") == 0)
+             storage = 'e';
+         else if (pg_strcasecmp(a, "extended") == 0)
+             storage = 'x';
+         else if (pg_strcasecmp(a, "main") == 0)
+             storage = 'm';
+         else
+             ereport(ERROR,
+                     (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                      errmsg("storage \"%s\" not recognized", a)));
      }

      /*