Thread: funny (cache (?)) bug in postgres (7.x tested)

Hello!

I found a funny bug in postgres with c functions. (or feature??)
Let's say we have got an function like this:
CREATE FUNCTION hupper(text)
RETURNS text
AS '/fun.so'
LANGUAGE 'c';

and fun.c:
#include <postgresql/postgres.h>
#include <postgresql/utils/elog.h>
#include <postgresql/libpq/libpq-fs.h>

text *hupper (text *a) {   int hossz,i;
   hossz=a->vl_len;   for (i=0;i<hossz;i++)   {       char ch;       ch=a->vl_dat[i];       if ((ch>=97)&(ch<=122))
ch=ch-32;      else if (ch=='á') ch='Á';       else if (ch=='é') ch='É';       else if (ch=='í') ch='Í';       else if
(ch=='ó')ch='Ó';       else if (ch=='ő') ch='Ő';       else if (ch=='ö') ch='Ö';       else if (ch=='ú') ch='Ú';
elseif (ch=='ű') ch='Ű';       else if (ch=='ü') ch='Ü';       a->vl_dat[i]=ch;   }
 
   return a;
}

We use this to make hungarian upper (=Hupper).
And two select:
gergo=> select mire from mamapenz;mire
-------betetebedebedebedebedebed
(6 rows)

gergo=> select hupper(mire) from mamapenz;hupper
--------BETETEBEDEBEDEBEDEBEDEBED
(6 rows)

this is good, and now:
gergo=> select mire from mamapenz;        ^^^^^^^^^^^^^^^^^^^^^mire
-------BETETEBEDEBEDEBEDEBEDEBED
(6 rows)

After once hupper run on the table it will be upper case even I don't use hupper.
It can be fixed with a postgres restart or with 10-20 minutes of waiting.

If this is documented, sorry (but please point out where).

Thanks,
RISKO Gergely

RISKO Gergely wrote:
[Charset iso-8859-1,iso- unsupported, skipping...]

> Hello!
>
> I found a funny bug in postgres with c functions. (or feature??)
> Let's say we have got an function like this:
> CREATE FUNCTION hupper(text)
> RETURNS text
> AS '/fun.so'
> LANGUAGE 'c';
   This  is  actually  neither a feature, nor a bug in Postgres.   Your function is violating some coding rules.
   The text argument handed into is actually residing inside the   shared buffer cache. So you're not supposed to
changeit!
 
   Your function isn't safe for compressed or toasted values. If   you accidentially have big data  in  that  column,
it might   crash the backend completely.
 
   A  function  returning  text has to allocate the return value   with palloc().
   Look into utils/adt/*.c for examples how  to  deal  correctly   with text attributes.


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#================================================== JanWieck@Yahoo.com #



_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com

On Thu, 28 Jun 2001, RISKO Gergely wrote:

> text *hupper (text *a) {
>     int hossz,i;
> 
>     hossz=a->vl_len;
>     for (i=0;i<hossz;i++)
>     {
>         char ch;
>         ch=a->vl_dat[i];
>         if ((ch>=97)&(ch<=122)) ch=ch-32;
>         else if (ch=='�') ch='�';
>         else if (ch=='�') ch='�';
>         else if (ch=='�') ch='�';
>         else if (ch=='�') ch='�';
>         else if (ch=='�') ch='�';
>         else if (ch=='�') ch='�';
>         else if (ch=='�') ch='�';
>         else if (ch=='�') ch='�';
>         else if (ch=='�') ch='�';
>         a->vl_dat[i]=ch;
>     }
> 
>     return a;
> }
(Rest snipped).

You are not supposed to write directly to the argument you are given. You
must construct a new text value (by allocating space via palloc) and
copy your string there. By overwriting existing values, you potentially
corrupt postgres' cache, resulting in your behaviour.

RISKO Gergely <risko@atom.hu> writes:
> I found a funny bug in postgres with c functions. (or feature??)

Scribbling on your input datum is verboten.  palloc a new value
to return.
        regards, tom lane

RISKO Gergely <risko@atom.hu> writes:

> text *hupper (text *a) {
>     int hossz,i;
> 
>     hossz=a->vl_len;
>     for (i=0;i<hossz;i++)
>     {
>         char ch;
>         ch=a->vl_dat[i];
>         if ((ch>=97)&(ch<=122)) ch=ch-32;
>         else if (ch=='á') ch='Á';
>         else if (ch=='é') ch='É';
>         else if (ch=='í') ch='Í';
>         else if (ch=='ó') ch='Ó';
>         else if (ch=='õ') ch='Õ';
>         else if (ch=='ö') ch='Ö';
>         else if (ch=='ú') ch='Ú';
>         else if (ch=='û') ch='Û';
>         else if (ch=='ü') ch='Ü';
>         a->vl_dat[i]=ch;
>     }
> 
>     return a;
> }

I think you need to allocate a new TEXT datum and return it.  You're
modifying the cached data in place, which is a no-no AFAIK.

-Doug
-- 
The rain man gave me two cures; he said jump right in,
The first was Texas medicine--the second was just railroad gin,
And like a fool I mixed them, and it strangled up my mind,
Now people just get uglier, and I got no sense of time...          --Dylan