On 08.08.25 22:55, Tom Lane wrote:
> FWIW, I don't love the "DummyDatum" name either. Maybe InvalidDatum?
I specifically did not use InvalidDatum because, for example, the result
of Int32GetDatum(0) is by no means "invalid". Maybe something like
NullDatum or VoidDatum?
A few related thoughts:
The calls to PG_RETURN_DUMMY() in my patch are in set-returning
functions. I thought maybe a real API macro would be nice here, like
PG_RETURN_SRF_DONE().
Many changes are in the arguments of on_*_exit() functions. I always
thought it was weird layering to use Datum at that level. I mean, would
on_proc_exit(foo, Int64GetDatum(val)) even work correctly on 32-bit
platforms?
Another common coding pattern is something like
if (isnull)
{
d = (Datum) 0;
n = true;
}
else
{
d = actualvalue;
n = false;
}
sometimes with a comment /* keep compiler quiet */ or /* redundant, but
safe */.
I wonder whether it would in general be better to not initialize "d" in
the first case, allowing perhaps static analyzers to detect invalid
accesses later on. On the other hand, this might confuse regular
compilers into warnings, as the comments suggest.
So maybe finding some higher-level changes in these areas could reduce
the churn in the remaining patch significantly.
