Thread: Problem with Numerics multiplication in C-function

Hi, I have a strange problem with Numeric multiplication in C-function.

There are 2 functions getRate and getVAT which return Numeric. In 3rd function calculateService I try to multiply the results of getRate and getVAT. After execution I have two types of error, some time without messages and with message "Invalid memory alloc ... ".
If I initialize the Numeric variables inside calculateService and multiply, function numeric_mul works fine.

PostgreSQL 8.3.7

Datum getRate(PG_FUNCTION_ARGS) {
    int32 accountId = PG_GETARG_INT32(0);
    int16 serviceId = PG_GETARG_INT16(1);
    DateADT date = PG_GETARG_DATEADT(2);

    bool isNull;
    char command[QUERY_MAX_SIZE];
    char message[MESSAGE_MAX_SIZE];

    Numeric rate = DatumGetNumeric(DirectFunctionCall3(numeric_in,
            CStringGetDatum(RATE_ERROR_CSTRING_VALUE), 0, -1));

    //Build SQL query
    snprintf(command, sizeof (command), "...");

    SPI_connect();
    SPI_execute(command, true, 0);
    rate = DatumGetNumeric(SPI_getbinval(SPI_tuptable->vals[0],
                     SPI_tuptable->tupdesc,
                     1,
                     &isNull));

#ifdef PR_DEBUG
    snprintf(message, sizeof (message), "<DEBUG> getRate: Returns rate = %s.",
            DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(rate))));
    elog(INFO, message);
#endif
    SPI_finish();
    PG_RETURN_NUMERIC(rate);
}

Datum calculateService(PG_FUNCTION_ARGS) {
    // Like the getRate
}

Datum calculateService(PG_FUNCTION_ARGS) {
    int32   accountId = PG_GETARG_INT32(0);
    int16   serviceId = PG_GETARG_INT16(1);
    DateADT date = PG_GETARG_DATEADT(2);
    int32   transactionRegisterId = PG_GETARG_INT32(3);

    Numeric rate;
    Numeric vat;
    Numeric amount;

    rate = DatumGetNumeric(DirectFunctionCall3(getRate,
            Int32GetDatum(accountId),
            Int16GetDatum(serviceId),
            DateADTGetDatum(date)));
    vat = DatumGetNumeric(DirectFunctionCall1(getVAT, DateADTGetDatum(date)));

#ifdef PR_DEBUG
    snprintf(message, sizeof (message), "<DEBUG> calculateService: rate = %s, vat",
            DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(rate))));
    elog(INFO, message);
#endif

    amount = DatumGetNumeric(DirectFunctionCall2(numeric_mul,
            NumericGetDatum(rate),
            NumericGetDatum(vat)));

    // ERROR
    ...
}


--
Best regards,
Ilya Urikh.

On Mon, Aug 03, 2009 at 01:45:53PM +1100, Ilya Urikh wrote:
> Hi, I have a strange problem with Numeric multiplication in C-function.
>
> There are 2 functions getRate and getVAT which return Numeric. In 3rd
> function calculateService I try to multiply the results of getRate and
> getVAT. After execution I have two types of error, some time without
> messages and with message "Invalid memory alloc ... ".
> If I initialize the Numeric variables inside calculateService and multiply,
> function numeric_mul works fine.

Not entirly sure, but I think it's because SPI_finish destroys the SPI
memory context, including the data in it, which is the data you are
trying to return. See this page in the documentation:

http://www.postgresql.org/docs/8.3/interactive/spi-memory.html

What you need to do is copy the Datum you wish to return into the upper
memory context. I'm not seeing a helpful SPI utility function or
obvious example here you help you, but I'm sure it's something with
MemoryContextSwitchTo() and datumCopy().

Hope this helps,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Please line up in a tree and maintain the heap invariant while
> boarding. Thank you for flying nlogn airlines.

Martijn van Oosterhout <kleptog@svana.org> writes:
> What you need to do is copy the Datum you wish to return into the upper
> memory context. I'm not seeing a helpful SPI utility function or
> obvious example here you help you, but I'm sure it's something with
> MemoryContextSwitchTo() and datumCopy().

SPI_palloc is what to use to allocate the result in the right place.

My guess is that the reason the function appeared to work (most of the
time) before is that it wasn't being tested in an assert-enabled build.
CLOBBER_FREED_MEMORY is exposing the mistake.

            regards, tom lane

On Mon, Aug 03, 2009 at 10:20:36AM -0400, Tom Lane wrote:
> Martijn van Oosterhout <kleptog@svana.org> writes:
> > What you need to do is copy the Datum you wish to return into the upper
> > memory context. I'm not seeing a helpful SPI utility function or
> > obvious example here you help you, but I'm sure it's something with
> > MemoryContextSwitchTo() and datumCopy().
>
> SPI_palloc is what to use to allocate the result in the right place.

Ofcourse, and then a memcpy(). I was distracted by the fact there is an
SPI_copy/returntuple(), but no SPI_returndatum(). It might actually be
helpful to include in the documentation an example of returning from a
function a result from SPI, since it's not entirely obvious.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Please line up in a tree and maintain the heap invariant while
> boarding. Thank you for flying nlogn airlines.

Martijn van Oosterhout <kleptog@svana.org> writes:
> On Mon, Aug 03, 2009 at 10:20:36AM -0400, Tom Lane wrote:
>> SPI_palloc is what to use to allocate the result in the right place.

> Ofcourse, and then a memcpy(). I was distracted by the fact there is an
> SPI_copy/returntuple(), but no SPI_returndatum().

Yeah, I was just thinking that that seems like an oversight.
plpgsql does this:

            if (!fcinfo->isnull && !func->fn_retbyval)
            {
                Size        len;
                void       *tmp;

                len = datumGetSize(estate.retval, false, func->fn_rettyplen);
                tmp = SPI_palloc(len);
                memcpy(tmp, DatumGetPointer(estate.retval), len);
                estate.retval = PointerGetDatum(tmp);
            }

but it seems like it'd be reasonable to provide SPI_datumcopy or
something like that to encapsulate this a bit more conveniently.

            regards, tom lane

Thank's a lot! That's really help. I failed to bear memory context switching in mind.
I resolved this problem with following strings in getRate:

  result = SPI_palloc(sizeof(rate));
  memcpy(result, rate, sizeof(rate));
 

P.S. I think it's a good idea to add example to documentation. Anyway when I had started to work with PostgreSQL I was very surprised at so clear and structured documentation. :)





--
Best regards,
Ilya Urikh.