Recent MemSet change to DirectFunctionCall1 - Mailing list pgsql-patches

Following the instructions in doc/bug.template here is my bug report and a
possible patch for fixing it.  I hope I am correct in sending it here first
and not to another mailing list.

Cheers
Mark Halliwell


Your name        :    Mark Halliwell
Your email address    :    mark@transportservices.com.au


System Configuration
---------------------
  Architecture (example: Intel Pentium)      :    Intel Pentium

  Operating System (example: Linux 2.0.26 ELF)     :    Linux 2.4.18

  PostgreSQL version (example: PostgreSQL-7.3):   PostgreSQL-7.3

  Compiler used (example:  gcc 2.95.2)        :    gcc 2.96


Please enter a FULL description of your problem:
------------------------------------------------

I recently upgraded from 7.2.1 to 7.3 and found that one of my triggers
stopped working; it would report:
ERROR:  TIMESTAMP(-1073746888) precision must be between 0 and 6
The trigger is used to set the time a record changes and does the following
call:
DirectFunctionCall1(timestamp_in, CStringGetDatum("now"));
(My trigger was based upon the code in contrib/spi/moddatetime.c)

I had thought the timestamp_in (from backend/utils/adt/timestamp.c) function
should take just 1 argument, but if a third argument is passed, it will be
used for the precision.

DirectFunctionCall1 was recently changed to not MemSet the
FunctionCallInfoData
structure, and as a result, arguments after the first one will be garbage.
Thus, when timestamp_in is invoked by my trigger, it has a bad value for the
precision and therefore fails.


Please describe a way to repeat the problem.   Please try to provide a
concise reproducible example, if at all possible:
----------------------------------------------------------------------

A trigger which does:
DirectFunctionCall1(timestamp_in, CStringGetDatum("now"));
such as the example in contrib/spi/moddatetime.c should cause an error
something like this:
ERROR:  TIMESTAMP(-1073746888) precision must be between 0 and 6


If you know how this problem might be fixed, list the solution below:
---------------------------------------------------------------------

Now, if this is just a bug in that DirectFunctionCall3 should be used instead
of DirectFunctionCall1 both in contrib/spi/moddatetime.c and my own trigger,
please ignore the rest of this.

I understand from the comments in backend/utils/fmgr/fmgr.c and the cvs log
that the decision to remove the MemSet was quite deliberate (mainly, it seems,
for a speed increase).  If I put the MemSet back in, it solves my particular
problem.

However, I have come up with a possibly better solution.  As I am not very
familiar with the source I may be completely wrong here, but, it would seem
that the macro that return the arguments (PG_GETARG_DATUM(n)) should check
the nargs value in the FunctionCallInfoData structure.  I have tried out this
following patch and it seems to work OK.  (Output from diff -c)

*** /tmp/fmgr.h Sat Oct 26 08:17:32 2002
--- src/include/fmgr.h  Thu Jan  9 12:43:02 2003
***************
*** 167,173 ****

  /* Macros for fetching arguments of standard types */

! #define PG_GETARG_DATUM(n)     (fcinfo->arg[n])
  #define PG_GETARG_INT32(n)     DatumGetInt32(PG_GETARG_DATUM(n))
  #define PG_GETARG_UINT32(n)  DatumGetUInt32(PG_GETARG_DATUM(n))
  #define PG_GETARG_INT16(n)     DatumGetInt16(PG_GETARG_DATUM(n))
--- 167,173 ----

  /* Macros for fetching arguments of standard types */

! #define PG_GETARG_DATUM(n)       (n >= fcinfo->nargs ? NULL:fcinfo->arg[n])
  #define PG_GETARG_INT32(n)     DatumGetInt32(PG_GETARG_DATUM(n))
  #define PG_GETARG_UINT32(n)  DatumGetUInt32(PG_GETARG_DATUM(n))
  #define PG_GETARG_INT16(n)     DatumGetInt16(PG_GETARG_DATUM(n))