Read/write expanded objects versus domains and CASE

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Read/write expanded objects versus domains and CASE
Date: 2016-12-22 18:33:39
Message-ID: 15225.1482431619@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I looked into the problem reported in bug #14472,
https://www.postgresql.org/message-id/20161221214744.25622.71454@wrigleys.postgresql.org
Although the submitter thought it was related to bug #14414, it isn't
particularly. The failure scenario is that the input value to a
CoerceToDomain execution node is a read-write expanded datum. We were
blindly passing that to any CHECK constraint expressions for the domain
type, which leaves called functions at liberty to modify or even delete
the expanded object. Correct behavior is to pass a read-only pointer to
the CHECK expressions and then return the original read-write pointer as
the expression result.

I nosed around for other occurrences of the same problem and soon
realized that CASE with an "arg" expression has a similar issue,
since the "arg" value may get passed to multiple test expressions.
It'd be substantially harder to make a test case for that in the
current state of the code --- to get a failure, you'd need a
plpgsql function to be the equality operator for some data type ---
but it's surely not impossible.

Also, domain_check() could in principle be called with a r/w datum,
so it should also protect against this.

The fix for this is nominally simple, to call MakeExpandedObjectReadOnly
at the appropriate places; but that requires having the data type's typlen
at hand, which we don't in these places.

In the domain cases, the typlen is cheaply accessible through the domain's
typcache entry, which we could get at as long as we don't mind using
DomainConstraintRef.tcache, which had been documented as private to
typcache.c. I don't see any particularly strong reason not to allow
callers to use it, though.

In the CASE case, there seems no help for it except to expend a
get_typlen() syscache lookup during executor setup. It's kind of annoying
to do that to support a corner case that may very well never occur in the
field, but I don't see another alternative.

So I'm proposing the attached patch (sans test cases as yet).
Any objections?

regards, tom lane

Attachment Content-Type Size
fix-rw-objects-in-domains-and-CASE.patch text/x-diff 5.9 KB

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-12-22 19:20:52 Re: Ilegal type cast in _hash_doinsert()
Previous Message Claudio Freire 2016-12-22 18:18:07 Re: Vacuum: allow usage of more than 1GB of work mem