Another bug(?) turned up by the llvm optimization checker

From: Greg Stark <stark(at)mit(dot)edu>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Another bug(?) turned up by the llvm optimization checker
Date: 2013-10-08 16:17:51
Message-ID: CAM-w4HNqwbVBJ45DAm58+G7j_w4bZraDdXmPzyb1UcwYXyoS7Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The commit below, specifically the change mentioned in the last paragraph
to fix isLockedRel broke the following comment in addRangeTableEntry:

* If pstate is NULL, we just build an RTE and return it without adding it
* to an rtable list.

In fact isLockedRefname() will seg fault promptly if pstate is NULL. I'm
not clear why this behaviour is needed though since as far as I can tell
nowhere in the code calls addRangeTableEntry or any of its derivatives with
pstate==NULL. I'm inclined to just remove the comment and the test for
pstate==NULL lower down but I don't really know what the motivation for
this promised behaviour was in the first place so I'm hesitant to do it on
my own.

commit 61e532820824504aa92ad93c427722d3fa9c1632
Author: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Date: Tue Oct 27 17:11:18 2009 +0000

Make FOR UPDATE/SHARE in the primary query not propagate into WITH
queries;
for example in
WITH w AS (SELECT * FROM foo) SELECT * FROM w, bar ... FOR UPDATE
the FOR UPDATE will now affect bar but not foo. This is more useful and
consistent than the original 8.4 behavior, which tried to propagate FOR
UPDATE
into the WITH query but always failed due to assorted implementation
restrictions. Even though we are in process of removing those
restrictions,
it seems correct on philosophical grounds to not let the outer query's
FOR UPDATE affect the WITH query.

In passing, fix isLockedRel which frequently got things wrong in
nested-subquery cases: "FOR UPDATE OF foo" applies to an alias foo in
the
current query level, not subqueries. This has been broken for a long
time,
but it doesn't seem worth back-patching further than 8.4 because the
actual
consequences are minimal. At worst the parser would sometimes get
RowShareLock on a relation when it should be AccessShareLock or vice
versa.
That would only make a difference if someone were using ExclusiveLock
concurrently, which no standard operation does, and anyway FOR UPDATE
doesn't result in visible changes so it's not clear that the someone
would
notice any problem. Between that and the fact that FOR UPDATE barely
works
with subqueries at all in existing releases, I'm not excited about
worrying
about it.

--
greg

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2013-10-08 16:20:22 Re: logical changeset generation v6.1
Previous Message Bruce Momjian 2013-10-08 16:13:54 Re: proposal: Set effective_cache_size to greater of .conf value, shared_buffers