pgsql: Add defenses against putting expanded objects into Const nodes. - Mailing list pgsql-committers

From Tom Lane
Subject pgsql: Add defenses against putting expanded objects into Const nodes.
Date
Msg-id E1aMJT8-0007Gu-2b@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Add defenses against putting expanded objects into Const nodes.

Putting a reference to an expanded-format value into a Const node would be
a bad idea for a couple of reasons.  It'd be possible for the supposedly
immutable Const to change value, if something modified the referenced
variable ... in fact, if the Const's reference were R/W, any function that
has the Const as argument might itself change it at runtime.  Also, because
datumIsEqual() is pretty simplistic, the Const might fail to compare equal
to other Consts that it should compare equal to, notably including copies
of itself.  This could lead to unexpected planner behavior, such as "could
not find pathkey item to sort" errors or inferior plans.

I have not been able to find any way to get an expanded value into a Const
within the existing core code; but Paul Ramsey was able to trigger the
problem by writing a datatype input function that returns an expanded
value.

The best fix seems to be to establish a rule that varlena values being
placed into Const nodes should be passed through pg_detoast_datum().
That will do nothing (and cost little) in normal cases, but it will flatten
expanded values and thereby avoid the above problems.  Also, it will
convert short-header or compressed values into canonical format, which will
avoid possible unexpected lack-of-equality issues for those cases too.
And it provides a last-ditch defense against putting a toasted value into
a Const, which we already knew was dangerous, cf commit 2b0c86b66563cf2f.
(In the light of this discussion, I'm no longer sure that that commit
provided 100% protection against such cases, but this fix should do it.)

The test added in commit 65c3d05e18e7c530 to catch datatype input functions
with unstable results would fail for functions that returned expanded
values; but it seems a bit uncharitable to deem a result unstable just
because it's expressed in expanded form, so revise the coding so that we
check for bitwise equality only after applying pg_detoast_datum().  That's
a sufficient condition anyway given the new rule about detoasting when
forming a Const.

Back-patch to 9.5 where the expanded-object facility was added.  It's
possible that this should go back further; but in the absence of clear
evidence that there's any live bug in older branches, I'll refrain for now.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/b99551832e79c915e4d877cf0a072120bd248748

Modified Files
--------------
src/backend/nodes/makefuncs.c        |    9 ++++++++
src/backend/optimizer/util/clauses.c |    3 ++-
src/backend/parser/parse_coerce.c    |   38 ++++++++++++++++++++++++++++++++++
src/backend/parser/parse_type.c      |   31 +--------------------------
src/include/nodes/primnodes.h        |    5 +++++
5 files changed, 55 insertions(+), 31 deletions(-)


pgsql-committers by date:

Previous
From: Fujii Masao
Date:
Subject: pgsql: Remove unused argument from ginInsertCleanup()
Next
From: Tom Lane
Date:
Subject: pgsql: Add defenses against putting expanded objects into Const nodes.