Обсуждение: Catching missing Datum conversions

Поиск
Список
Период
Сортировка

Catching missing Datum conversions

От
Thomas Munro
Дата:
Hi,

When reviewing a recent patch, I missed a place where Datum was being
converted to another type implicitly (ie without going though a
DatumGetXXX() macro).  Thanks to Jeff for fixing that (commit
b538c90b), but I was curious to see if I could convince my compiler to
tell me about that sort of thing.  Here's an experimental hack that
makes Datum a struct (unfortunately defined in two places, but like I
said it's a hack), and then fixes all the resulting compile errors.
The main categories of change are:

1. Many cases of replacing "(Datum) 0" with a new macro "NullDatum"
and adjusting code that compares with 0/NULL, so you can pretty much
ignore that noise.  Likewise code that compares datums directly
without apparent knowledge of the expected type.

2.  VARDATA etc macros taking a Datum instead of a varlena *.  I think
the interface is suppose to be able to take both, so I think you can
pretty much ignore that noise too, I just couldn't immediately think
of a trick that would make that polymorphism work so I added
DatumGetPointer(x) wherever a Datum x was given directly to those
macros.

3.  Many cases of object IDs being converted implicitly, for example
in syscache calls.  A few cases of implicit use as booleans.

4.  Various confusions about the types involved in PG_RETURN_XXX and
PG_GETARGS_XXX macros, and failure to convert values from
Datum-returning functions, or unnecessary conversion of results (eg
makeArrayResult).

I should probably split this into "actionable" (categories 3 and 4)
and "noise and scaffolding" patches.

-- 
Thomas Munro
https://enterprisedb.com
Вложения

Re: Catching missing Datum conversions

От
Corey Huinker
Дата:
I should probably split this into "actionable" (categories 3 and 4)
and "noise and scaffolding" patches.

Breaking down the noise-and-scaffolding into some subgroups might make the rather long patches more palatable/exceedingly-obvious:
* (Datum) 0 ---> NullDatum
* 0 ----> NullDatum
* The DatumGetPointer(allParameterTypes) null tests

Having said that, everything you did seems really straightforward, except for 
src/backend/rewrite/rewriteDefine.c
src/backend/statistics/mcv.c
src/backend/tsearch/ts_parse.c
and those seem like cases where the DatumGetXXX was a no-op before Datum was a struct.

Re: Catching missing Datum conversions

От
Robert Haas
Дата:
On Fri, Jul 19, 2019 at 7:55 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> When reviewing a recent patch, I missed a place where Datum was being
> converted to another type implicitly (ie without going though a
> DatumGetXXX() macro).  Thanks to Jeff for fixing that (commit
> b538c90b), but I was curious to see if I could convince my compiler to
> tell me about that sort of thing.

This is a very easy mistake to make, so if you ever feel like
returning to this topic in earnest, I think it could be a worthwhile
expenditure of time.

-- 
Robert Haas
EDB: http://www.enterprisedb.com