to_char() dumps core

On Fri, 20 Oct 2000, Tatsuo Ishii wrote:

In 7.0.2

select to_char(sum(n),'999') from t1;

causes backend dump a core if n is a float/numeric ...data type AND if
sum(n) returns NULL. This seems due to a bad null pointer handling for
aruguments of pass-by-reference data types. I think just a simple
null pointer checking at very top of each function (for example
float4_to_char()) would solve the problem. Comments?

In the 7.1devel it's correct, but here it's bug, IMHO it bear on changes
in the 7.1's fmgr, because code is same in both versions for this. On Monday,
I try fix it for 7.0.3

Karel

On Fri, 20 Oct 2000, Karel Zak wrote:

On Fri, 20 Oct 2000, Tatsuo Ishii wrote:

In 7.0.2

select to_char(sum(n),'999') from t1;

causes backend dump a core if n is a float/numeric ...data type AND if
sum(n) returns NULL. This seems due to a bad null pointer handling for
aruguments of pass-by-reference data types. I think just a simple
null pointer checking at very top of each function (for example
float4_to_char()) would solve the problem. Comments?

In the 7.1devel it's correct, but here it's bug, IMHO it bear on changes
in the 7.1's fmgr, because code is same in both versions for this. On Monday,
I try fix it for 7.0.3

Not, monday .. just now :-)

The patch is attached... Bruce, it's again to 7.0.3!

Thanks for bug report

Karel

test=# create table t1 (f4 float4, f8 float8, n numeric, i4 int4, i8 int8);
CREATE
test=# select to_char(sum(f4), '9'), to_char(sum(f8), '9'), to_char(sum(n),
'9'), to_char(sum(i4), '9'), to_char(sum(i8), '9') from t1;
to_char | to_char | to_char | to_char | to_char
---------+---------+---------+---------+---------
| | | |
(1 row)

causes backend dump a core if n is a float/numeric ...data type AND if
sum(n) returns NULL. This seems due to a bad null pointer handling for
aruguments of pass-by-reference data types. I think just a simple
null pointer checking at very top of each function (for example
float4_to_char()) would solve the problem. Comments?

In the 7.1devel it's correct, but here it's bug, IMHO it bear on changes
in the 7.1's fmgr, because code is same in both versions for this. On Monday,
I try fix it for 7.0.3

Not, monday .. just now :-)

The patch is attached... Bruce, it's again to 7.0.3!

Got it. You don't have to hit me over head all the time (just most of
the time). :-)

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

*/

On Fri, 20 Oct 2000, Bruce Momjian wrote:

The patch is attached... Bruce, it's again to 7.0.3!

Got it. You don't have to hit me over head all the time (just most of
the time). :-)

Oh no, I want pull up your head from 7.1 cycle only :-)

Thanks
Karel

In the 7.1devel it's correct, but here it's bug, IMHO it bear on changes
in the 7.1's fmgr, because code is same in both versions for this. On Monday,
I try fix it for 7.0.3

Not, monday .. just now :-)

The patch is attached... Bruce, it's again to 7.0.3!

Thanks for bug report

Karel

Thank for your qucik fix!
--
Tatsuo Ishii

Tatsuo Ishii <t-ishii@sra.co.jp> writes:

In 7.0.2
select to_char(sum(n),'999') from t1;

causes backend dump a core if n is a float/numeric ...data type AND if
sum(n) returns NULL. This seems due to a bad null pointer handling for
aruguments of pass-by-reference data types. I think just a simple
null pointer checking at very top of each function (for example
float4_to_char()) would solve the problem. Comments?

Just a note to remind everyone, since I haven't yet updated the
documentation for the new-fmgr changes: under the 7.1 fmgr it is *no
longer necessary* to check for NULL pointer in function execution
routines, assuming that your function is marked "strict" in pg_proc
(as nearly all built-in functions are). The fmgr will not call such
a function in the first place, if any of its inputs are NULLs.

So, while adding the NULL-pointer checks is an OK patch for 7.0.*,
don't stick such checks into current sources.

(Also, if you do want to check for a NULL input in current sources,
looking for a NULL pointer is the wrong way to code it anyway;
PG_ARGISNULL(n) is the right way.)

regards, tom lane

Karel Zak <zakkr@zf.jcu.cz> writes:

In the 7.1devel it's correct, but here it's bug, IMHO it bear on changes
in the 7.1's fmgr, because code is same in both versions for this. On Monday,
I try fix it for 7.0.3

Applied to REL7_0_PATCHES branch (only). Thanks.

regards, tom lane

(Also, if you do want to check for a NULL input in current sources,
looking for a NULL pointer is the wrong way to code it anyway;
PG_ARGISNULL(n) is the right way.)

For pass by reference datatypes setting the reference to a null pointer
for a NULL value imho would be a fine thing in addition to the indicator,
no ?

Andreas

Zeugswetter Andreas SB <ZeugswetterA@wien.spardat.at> writes:

For pass by reference datatypes setting the reference to a null pointer
for a NULL value imho would be a fine thing in addition to the indicator,
no ?

At the moment it generally will be, but that's not certain to be true
forever. I believe we've had discussions in the past about supporting
multiple kinds of NULL. (I had the idea this was actually required by
SQL99, in fact, though I can't find anything about it at the moment.)

The obvious way to do that is to commandeer the otherwise unused
contents of a Datum when the associated null-flag is true. At that
point checking the null-flag will be the only valid way to check for
NULL.

Assuming that the null-kind values are small integers, attempts to
dereference them will still SEGV on reasonable systems, so I don't
think any error checking is lost. Just don't do "if (datum == NULL)".

regards, tom lane