Re: segmentation fault using currtid and partitioned tables
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Jaime Casanova <jaime.casanova@2ndquadrant.com>, Pg Hackers <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>
Date: 2020-04-09T06:22:52Z
Lists: pgsql-hackers
Attachments
- tid-funcs-fixes-v1.patch (text/x-diff) patch v1
On Wed, Apr 08, 2020 at 04:13:31PM +0900, Michael Paquier wrote: > I have been looking at the tree and the use of the table AM APIs, and > those TID lookups are really a particular case compared to the other > callers of the table AM callbacks. Anyway, I have not spotted similar > problems, so I find very tempting the option to just add some > RELKIND_HAS_STORAGE() to tid.c where it matters and call it a day. Playing more with this stuff, it happens that we have zero code coverage for currtid() and currtid2(), and the main user of those functions I can find around is the ODBC driver: https://coverage.postgresql.org/src/backend/utils/adt/tid.c.gcov.html There are multiple cases to consider, particularly for views: - Case of a view with ctid as attribute taken from table. - Case of a view with ctid as attribute with incorrect attribute type. It is worth noting that all those code paths can trigger various elog() errors, which is not something that a user should be able to do using a SQL-callable function. There are also two code paths for cases where a view has no or more-than-one SELECT rules, which cannot normally be reached. All in that, I propose something like the attached to patch the surroundings with tests to cover everything I could think of, which I guess had better be backpatched? While on it, I have noticed that we lack coverage for max(tid) and min(tid), so I have included a bonus test. Another issue is that we don't have any documentation for those functions, in which case the best fit is a subsection for TID operators under "Functions and Operators"? For now, I am adding a patch to next CF so as we don't forget about this set of issues. Any thoughts? -- Michael
Commits
-
Fix use-after-release mistake in currtid() and currtid2() for views
- a8c6eb5b4c4c 9.5.23 landed
- e2fa9732fddc 9.6.19 landed
- a36f216209b8 10.14 landed
- 8bc74490df25 11.9 landed
- 894041eb262f 12.4 landed
- ce1c5b9ae87b 13.0 landed
-
Fix crashes with currtid() and currtid2()
- 95e389b3c2ea 12.4 landed
- e786be5fcb25 13.0 landed