Thread: Re: [HACKERS] enum types and binary queries

Re: [HACKERS] enum types and binary queries

From
Andrew Dunstan
Date:

Tom Lane wrote:
>
> Andrew's idea of using the enum ordinal value would meet that test, but
> at least with the current layout of pg_enum it would be quite expensive
> to do the conversion in either direction --- you'd have to fetch
> multiple catalog rows.  I think we'd have to add another column showing
> the ordinal value, and put an index on it, to make I/O reasonably fast.
> Doesn't really seem worth it.
>
>
>

Yeah. I think we should treat enums just as we do text, for this purpose.

Here's a patch (minus catalog bump) which I think does that.

cheers

andrew
Index: src/backend/commands/typecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/typecmds.c,v
retrieving revision 1.106
diff -c -r1.106 typecmds.c
*** src/backend/commands/typecmds.c    20 Jun 2007 18:15:49 -0000    1.106
--- src/backend/commands/typecmds.c    31 Aug 2007 14:31:05 -0000
***************
*** 1039,1046 ****
                     DEFAULT_TYPDELIM,    /* array element delimiter */
                     F_ENUM_IN,        /* input procedure */
                     F_ENUM_OUT,        /* output procedure */
!                    InvalidOid,        /* receive procedure - none */
!                    InvalidOid,        /* send procedure - none */
                     InvalidOid,        /* typmodin procedure - none */
                     InvalidOid,        /* typmodout procedure - none */
                     InvalidOid,        /* analyze procedure - default */
--- 1039,1046 ----
                     DEFAULT_TYPDELIM,    /* array element delimiter */
                     F_ENUM_IN,        /* input procedure */
                     F_ENUM_OUT,        /* output procedure */
!                    F_ENUM_RECV,        /* receive procedure */
!                    F_ENUM_SEND,        /* send procedure */
                     InvalidOid,        /* typmodin procedure - none */
                     InvalidOid,        /* typmodout procedure - none */
                     InvalidOid,        /* analyze procedure - default */
Index: src/backend/utils/adt/enum.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/enum.c,v
retrieving revision 1.3
diff -c -r1.3 enum.c
*** src/backend/utils/adt/enum.c    5 Jun 2007 21:31:06 -0000    1.3
--- src/backend/utils/adt/enum.c    31 Aug 2007 14:31:05 -0000
***************
*** 19,24 ****
--- 19,26 ----
  #include "utils/builtins.h"
  #include "utils/lsyscache.h"
  #include "utils/syscache.h"
+ #include "libpq/pqformat.h"
+ #include "miscadmin.h"


  static ArrayType *enum_range_internal(Oid enumtypoid, Oid lower, Oid upper);
***************
*** 86,91 ****
--- 88,160 ----
      PG_RETURN_CSTRING(result);
  }

+ /* Binary I/O support */
+ Datum
+ enum_recv(PG_FUNCTION_ARGS)
+ {
+     StringInfo  buf = (StringInfo) PG_GETARG_POINTER(0);
+     Oid enumtypoid = PG_GETARG_OID(1);
+     Oid enumoid;
+     HeapTuple tup;
+     char       *name;
+     int         nbytes;
+
+     name = pq_getmsgtext(buf, buf->len - buf->cursor, &nbytes);
+
+     /* must check length to prevent Assert failure within SearchSysCache */
+     if (strlen(name) >= NAMEDATALEN)
+         ereport(ERROR,
+                 (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                  errmsg("invalid input value for enum %s: \"%s\"",
+                         format_type_be(enumtypoid),
+                         name)));
+
+     tup = SearchSysCache(ENUMTYPOIDNAME,
+                          ObjectIdGetDatum(enumtypoid),
+                          CStringGetDatum(name),
+                          0, 0);
+     if (!HeapTupleIsValid(tup))
+         ereport(ERROR,
+                 (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                  errmsg("invalid input value for enum %s: \"%s\"",
+                         format_type_be(enumtypoid),
+                         name)));
+
+     enumoid = HeapTupleGetOid(tup);
+
+     ReleaseSysCache(tup);
+
+     pfree(name);
+
+     PG_RETURN_OID(enumoid);
+ }
+
+ Datum
+ enum_send(PG_FUNCTION_ARGS)
+ {
+     Oid enumval = PG_GETARG_OID(0);
+     StringInfoData buf;
+     HeapTuple tup;
+     Form_pg_enum en;
+
+     tup = SearchSysCache(ENUMOID,
+                          ObjectIdGetDatum(enumval),
+                          0, 0, 0);
+     if (!HeapTupleIsValid(tup))
+         ereport(ERROR,
+                 (errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
+                  errmsg("invalid internal value for enum: %u",
+                         enumval)));
+     en = (Form_pg_enum) GETSTRUCT(tup);
+
+     pq_begintypsend(&buf);
+     pq_sendtext(&buf, NameStr(en->enumlabel), strlen(NameStr(en->enumlabel)));
+
+     ReleaseSysCache(tup);
+
+     PG_RETURN_BYTEA_P(pq_endtypsend(&buf));
+ }
+
  /* Comparison functions and related */

  Datum
Index: src/include/catalog/pg_proc.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.465
diff -c -r1.465 pg_proc.h
*** src/include/catalog/pg_proc.h    27 Aug 2007 01:39:24 -0000    1.465
--- src/include/catalog/pg_proc.h    31 Aug 2007 14:31:06 -0000
***************
*** 4134,4139 ****
--- 4134,4141 ----
  DATA(insert OID = 3529 (  enum_last        PGNSP PGUID 12 1 0 f f f f s 1 3500 "3500" _null_ _null_ _null_ enum_last
-_null_ )); 
  DATA(insert OID = 3530 (  enum_range    PGNSP PGUID 12 1 0 f f f f s 2 2277 "3500 3500" _null_ _null_ _null_
enum_range_bounds- _null_ )); 
  DATA(insert OID = 3531 (  enum_range    PGNSP PGUID 12 1 0 f f f f s 1 2277 "3500" _null_ _null_ _null_
enum_range_all- _null_ )); 
+ DATA(insert OID = 3532 (  enum_recv        PGNSP PGUID 12 1 0 f f t f s 2 3500 "2275 26" _null_ _null_ _null_
enum_recv- _null_ )); 
+ DATA(insert OID = 3533 (  enum_send        PGNSP PGUID 12 1 0 f f t f s 1 17   "3500" _null_ _null_ _null_ enum_send
-_null_ )); 

  /* text search stuff */
  DATA(insert OID =  3610 (  tsvectorin            PGNSP PGUID 12 1 0 f f t f i 1 3614 "2275" _null_ _null_ _null_
tsvectorin- _null_ )); 
Index: src/include/utils/builtins.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/utils/builtins.h,v
retrieving revision 1.301
diff -c -r1.301 builtins.h
*** src/include/utils/builtins.h    27 Aug 2007 01:39:25 -0000    1.301
--- src/include/utils/builtins.h    31 Aug 2007 14:31:07 -0000
***************
*** 107,112 ****
--- 107,114 ----
  /* enum.c */
  extern Datum enum_in(PG_FUNCTION_ARGS);
  extern Datum enum_out(PG_FUNCTION_ARGS);
+ extern Datum enum_recv(PG_FUNCTION_ARGS);
+ extern Datum enum_send(PG_FUNCTION_ARGS);
  extern Datum enum_lt(PG_FUNCTION_ARGS);
  extern Datum enum_le(PG_FUNCTION_ARGS);
  extern Datum enum_eq(PG_FUNCTION_ARGS);

Re: [HACKERS] enum types and binary queries

From
Tom Lane
Date:
Andrew Dunstan <andrew@dunslane.net> writes:
> Here's a patch (minus catalog bump) which I think does that.

Looks sane in a very quick once-over, but I didn't test it.

            regards, tom lane

Re: [HACKERS] enum types and binary queries

From
"Merlin Moncure"
Date:
On 8/31/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > Here's a patch (minus catalog bump) which I think does that.
>
> Looks sane in a very quick once-over, but I didn't test it.

works fine (here was my test). thanks for quick resolution to this
issue. strings returned in binary format is IMO ok.

enum.c:
include "libpq-fe.h"
#include "string.h"
#include <stdlib.h>


int main(int argc, char **argv)
{
  PGconn *c = PQconnectdb("user=postgres");
  PGresult *r;
  r = PQexecParams(c, "select 'foo'::foo", 0, NULL, NULL, NULL, NULL, 1);

  ExecStatusType t = PQresultStatus(r);

  if(t != PGRES_COMMAND_OK & t != PGRES_TUPLES_OK)
  {
    printf("%s", PQresultErrorMessage(r));
    exit(1);
  }

  char* f = PQgetvalue(r,0,0);
  int len = 3;
  int format = 1;
  PQclear(r);

  r = PQexecParams(c, "select $1::foo", 1, NULL, (const char* const
*)&f, &len, &format, 1);

  if(t != PGRES_COMMAND_OK & t != PGRES_TUPLES_OK)
  {
    printf("%s", PQresultErrorMessage(r));
    exit(1);
  }

  PQfinish(c);
}

Re: [HACKERS] enum types and binary queries

From
"Merlin Moncure"
Date:
On 8/31/07, Merlin Moncure <mmoncure@gmail.com> wrote:
> On 8/31/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Andrew Dunstan <andrew@dunslane.net> writes:
> > > Here's a patch (minus catalog bump) which I think does that.
> >
> > Looks sane in a very quick once-over, but I didn't test it.
>
> works fine (here was my test). thanks for quick resolution to this
> issue. strings returned in binary format is IMO ok.
>   if(t != PGRES_COMMAND_OK & t != PGRES_TUPLES_OK)

oops, this line was wrong. the enum is fine though.

merlin

Re: [HACKERS] enum types and binary queries

From
Andrew Dunstan
Date:

Merlin Moncure wrote:
> On 8/31/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> Andrew Dunstan <andrew@dunslane.net> writes:
>>
>>> Here's a patch (minus catalog bump) which I think does that.
>>>
>> Looks sane in a very quick once-over, but I didn't test it.
>>
>
> works fine (here was my test). thanks for quick resolution to this
> issue. strings returned in binary format is IMO ok.
>
>
>

Patch applied (with catalog bump).

cheers

andrew