From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Jaime Casanova <jaime(dot)casanova(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de> |
Subject: | Re: segmentation fault using currtid and partitioned tables |
Date: | 2020-04-09 06:22:52 |
Message-ID: | 20200409062252.GP1606@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
tid-funcs-fixes-v1.patch | text/x-diff | 12.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2020-04-09 06:23:48 | Re: Vacuum o/p with (full 1, parallel 0) option throwing an error |
Previous Message | Julien Rouhaud | 2020-04-09 06:17:53 | Re: Commitfest 2020-03 Now in Progress |