Thread: pgsql: Remove dependency on HeapTuple from predicate locking functions.
Remove dependency on HeapTuple from predicate locking functions. The following changes make the predicate locking functions more generic and suitable for use by future access methods: - PredicateLockTuple() is renamed to PredicateLockTID(). It takes ItemPointer and inserting transaction ID instead of HeapTuple. - CheckForSerializableConflictIn() takes blocknum instead of buffer. - CheckForSerializableConflictOut() no longer takes HeapTuple or buffer. Author: Ashwin Agrawal Reviewed-by: Andres Freund, Kuntal Ghosh, Thomas Munro Discussion: https://postgr.es/m/CALfoeiv0k3hkEb3Oqk%3DziWqtyk2Jys1UOK5hwRBNeANT_yX%2Bng%40mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/6f38d4dac381b5b8bead302a0b4f81761042cd25 Modified Files -------------- src/backend/access/gin/ginbtree.c | 2 +- src/backend/access/gin/ginfast.c | 2 +- src/backend/access/gin/gininsert.c | 6 +- src/backend/access/gist/gist.c | 2 +- src/backend/access/hash/hashinsert.c | 2 +- src/backend/access/heap/heapam.c | 125 ++++++++++++++++++++++++---- src/backend/access/heap/heapam_handler.c | 11 +-- src/backend/access/index/indexam.c | 4 +- src/backend/access/nbtree/nbtinsert.c | 4 +- src/backend/storage/lmgr/predicate.c | 137 ++++++++++--------------------- src/include/access/heapam.h | 2 + src/include/storage/predicate.h | 9 +- 12 files changed, 176 insertions(+), 130 deletions(-)
Thomas Munro <tmunro@postgresql.org> writes:
> Remove dependency on HeapTuple from predicate locking functions.
anole's not terribly pleased with this:
"heapam.c", line 9137: error #2118: a void function may not return a value
return CheckForSerializableConflictOut(relation, xid, snapshot);
^
regards, tom lane
On Tue, Jan 28, 2020 at 6:59 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Thomas Munro <tmunro@postgresql.org> writes: > > Remove dependency on HeapTuple from predicate locking functions. > > anole's not terribly pleased with this: > > "heapam.c", line 9137: error #2118: a void function may not return a value > return CheckForSerializableConflictOut(relation, xid, snapshot); > ^ Thanks. I pushed a fix. Wow, HP C spits out a lot of warnings.
Thomas Munro <thomas.munro@gmail.com> writes:
> On Tue, Jan 28, 2020 at 6:59 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> anole's not terribly pleased with this:
>> "heapam.c", line 9137: error #2118: a void function may not return a value
>> return CheckForSerializableConflictOut(relation, xid, snapshot);
> Thanks. I pushed a fix.
> Wow, HP C spits out a lot of warnings.
It's pretty noisy, and most of 'em are useless :-(. But as for this
particular complaint, I don't really understand why gcc lets it slide.
The C99 standard is not exactly ambiguous about this:
6.8.6.4 The return statement
Constraints
[#1] A return statement with an expression shall not appear
in a function whose return type is void. A return statement
without an expression shall only appear in a function whose
return type is void.
There is absolutely no question that the original coding is illegal
per spec, and it isn't even a particularly useful shorthand; so why
can't we get even a warning about it?
regards, tom lane
On Wed, Jan 29, 2020 at 4:05 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
> > On Tue, Jan 28, 2020 at 6:59 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> anole's not terribly pleased with this:
> >> "heapam.c", line 9137: error #2118: a void function may not return a value
> >> return CheckForSerializableConflictOut(relation, xid, snapshot);
>
> > Thanks. I pushed a fix.
> > Wow, HP C spits out a lot of warnings.
>
> It's pretty noisy, and most of 'em are useless :-(. But as for this
> particular complaint, I don't really understand why gcc lets it slide.
Maybe because it's allowed in C++, and pretty harmless.
> There is absolutely no question that the original coding is illegal
> per spec, and it isn't even a particularly useful shorthand; so why
> can't we get even a warning about it?
$ cat test.c
void f() {}
void g() { return f(); }
$ cc -c -Wall test.c
$ cc -c -Wpedantic test.c
test.c:2:12: warning: void function 'g' should not return void
expression [-Wpedantic]
void g() { return f(); }
^ ~~~
1 warning generated.
Many other constructs in PostgreSQL are rejected by that switch,
though, and I don't see a way to ask for just that one warning.
Thomas Munro <thomas.munro@gmail.com> writes:
> On Wed, Jan 29, 2020 at 4:05 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> There is absolutely no question that the original coding is illegal
>> per spec, and it isn't even a particularly useful shorthand; so why
>> can't we get even a warning about it?
> $ cc -c -Wpedantic test.c
> test.c:2:12: warning: void function 'g' should not return void
> expression [-Wpedantic]
> void g() { return f(); }
> ^ ~~~
> 1 warning generated.
> Many other constructs in PostgreSQL are rejected by that switch,
> though, and I don't see a way to ask for just that one warning.
Yeah, -Wpedantic is a little *too* pedantic I'm afraid. Oh well.
regards, tom lane