Re: BUG #19476: Segmentation fault in contrib/spi

On Wed, May 13, 2026 at 12:57:47AM +0530, Ayush Tiwari wrote:
> I have rebased the minimal fix on current master. It is essentially
> the same shape as the snippet you suggested -- emit the NULL keyword
> directly when SPI_getvalue() returns NULL, otherwise pass through
> quote_literal_cstr() as today. Attached as v2-0001.
>
> I dropped my earlier 0002 patch. The CVE fix already addressed the
> quoting/escaping concerns that motivated half of it.

I'm confused why you dropped 0002. Reusing the new key values for
subsequent updates seems like a bug worth fixing. However, note that the
parameter symbol approach doesn't work well for cases like this:

CREATE EXTENSION refint;

CREATE TABLE p (a int);
CREATE TABLE f (a xid);
CREATE TRIGGER t
AFTER DELETE OR UPDATE ON p
FOR EACH ROW EXECUTE PROCEDURE
check_foreign_key(1, 'c', 'a', 'f', 'a');

INSERT INTO p VALUES (1);
UPDATE p SET a = 2;

With a rebased version of 0002 applied, the UPDATE statement fails like
this:

ERROR: column "a" is of type xid but expression is of type integer
LINE 1: update f set a = $2 where a = $1
^
HINT: You will need to rewrite or cast the expression.
QUERY: update f set a = $2 where a = $1

Presumably the problem is that we're using the wrong argument type for the
foreign key. I'm not sure this is trivial to fix; it seems like we'd need
to provide that information in CREATE TRIGGER or look up the foreign key
type within the trigger function itself. Perhaps the best we can do is to
avoid caching a plan in this case.

Regarding 0001, note that the refint docs state the following:

Note that the primary/unique key columns should be marked NOT NULL and
should have a unique index.

So maybe we could alternatively teach check_foreign_key() to either ERROR
or do nothing instead. On the other hand, given this case seemed to
accidentally work before the CVE fix, it's arguably worth fixing.

--
nathan