On Tue, Jan 20, 2015 at 11:38 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Michael Paquier <michael.paqu...@gmail.com> writes: >> Coverity is pointing out $subject, with the following stuff in >> gbt_var_same(): >> ... >> As Heikki pointed me out on IM, the lack of crash report in this area, >> as well as similar coding style in cube/ seem to be sufficient >> arguments to simply remove those NULL checks instead of doing more >> solid checks on them. Patch is attached. > > The way to form a convincing argument that these checks are unnecessary > would be to verify that (1) the SQL-accessible functions directly calling > gbt_var_same() are all marked STRICT, and (2) the core GIST code never > passes a NULL to these support functions. I'm prepared to believe that > (1) and (2) are both true, but it merits checking.
Sure. gbt_var_same is called by those functions in btree_gist/: - gbt_bit_same - gbt_bytea_same - gbt_numeric_same - gbt_text_same =# select proname, proisstrict from pg_proc where proname in ('gbt_bit_same', 'gbt_bytea_same', 'gbt_numeric_same', 'gbt_text_same'); proname | proisstrict ------------------+------------- gbt_text_same | t gbt_bytea_same | t gbt_numeric_same | t gbt_bit_same | t (4 rows) For the second point, I have run regression tests with an assertion in gbt_var_same checking if t1 or t2 are NULL and tests worked. Also, looking at the code of gist, gbt_var_same is called through gistKeyIsEQ, which is used for an insertion or for a split. The point is that when doing an insertion, a call to gistgetadjusted is done and we look if one of the keys is NULL. If one of them is, code does not go through gistKeyIsEQ, so the NULL checks do not seem necessary in gbt_var_same. Btw, looking at the code of gist, I think that it would be interesting to add an assertion in gistKeyIsEQ like that: Assert(DatumGetPointer(a) != NULL && DatumGetPointer(b) != NULL); Thoughts? -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers