Neil Conway wrote:
Bruce Momjian wrote:

+ /* +  * Return the length of a datum, possibly compressed
+  */
+ Datum
+ pg_column_size(PG_FUNCTION_ARGS)
+ {
+     Datum            value = PG_GETARG_DATUM(0);
+     int                result;
+ + /* fn_extra stores the fixed column length, or -1 for varlena. */
+     if (fcinfo->flinfo->fn_extra == NULL)    /* first call? */
+     {
+ /* On the first call lookup the datatype of the supplied argument */


[...]

Is this optimization worth the code complexity?


I didn't performance test it, but the idea of hammering the catalogs for
each value to be processed seemed a bad thing.

+         tp = SearchSysCache(TYPEOID,
+                             ObjectIdGetDatum(argtypeid),
+                             0, 0, 0);
+         if (!HeapTupleIsValid(tp))
+         {
+             /* Oid not in pg_type, should never happen. */
+             ereport(ERROR,
+                     (errcode(ERRCODE_INTERNAL_ERROR),
+                      errmsg("invalid typid: %u", argtypeid)));
+         }


elog(ERROR) is usually used for "can't happen" errors; also, the usual error message in this scenario is "cache lookup failed [...]". Perhaps better to use get_typlen() here, anyway.


Ahem,..ah yes, get_typlen()! (ducks)- that is clearly much better!

I have attached a little change to varlena.c that uses it. I left the
ereport as it was, but am not fussed about it either way.

BTW - Bruce, I like the changes, makes the beast more friendly to use!

Best wishes

Mark


Index: src/backend/utils/adt/varlena.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/varlena.c,v
retrieving revision 1.125
diff -c -r1.125 varlena.c
*** src/backend/utils/adt/varlena.c     6 Jul 2005 19:02:52 -0000       1.125
--- src/backend/utils/adt/varlena.c     7 Jul 2005 02:52:50 -0000
***************
*** 28,34 ****
  #include "utils/builtins.h"
  #include "utils/lsyscache.h"
  #include "utils/pg_locale.h"
- #include "utils/syscache.h"
  
  
  typedef struct varlena unknown;
--- 28,33 ----
***************
*** 2364,2385 ****
        {
                /* On the first call lookup the datatype of the supplied 
argument */
                Oid                             argtypeid = 
get_fn_expr_argtype(fcinfo->flinfo, 0);
!               HeapTuple               tp;
!               int                             typlen;
  
!               tp = SearchSysCache(TYPEOID,
!                                                       
ObjectIdGetDatum(argtypeid),
!                                                       0, 0, 0);
!               if (!HeapTupleIsValid(tp))
                {
                        /* Oid not in pg_type, should never happen. */
                        ereport(ERROR,
                                        (errcode(ERRCODE_INTERNAL_ERROR),
!                                        errmsg("invalid typid: %u", 
argtypeid)));
                }
!               
!               typlen = ((Form_pg_type)GETSTRUCT(tp))->typlen;
!               ReleaseSysCache(tp);
                fcinfo->flinfo->fn_extra = 
MemoryContextAlloc(fcinfo->flinfo->fn_mcxt,
                                                                                
                          sizeof(int));
                *(int *)fcinfo->flinfo->fn_extra = typlen;
--- 2363,2379 ----
        {
                /* On the first call lookup the datatype of the supplied 
argument */
                Oid                             argtypeid = 
get_fn_expr_argtype(fcinfo->flinfo, 0);
!               int                             typlen    = 
get_typlen(argtypeid);
  
!               
!               if (typlen == 0)
                {
                        /* Oid not in pg_type, should never happen. */
                        ereport(ERROR,
                                        (errcode(ERRCODE_INTERNAL_ERROR),
!                                        errmsg("cache lookup failed for type 
%u", argtypeid)));
                }
! 
                fcinfo->flinfo->fn_extra = 
MemoryContextAlloc(fcinfo->flinfo->fn_mcxt,
                                                                                
                          sizeof(int));
                *(int *)fcinfo->flinfo->fn_extra = typlen;

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]

Reply via email to