PG 11 JIT deform failure
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t40717psql -h localhost -U postgresBuilt from patchset v5 (message #5), July 28, 2026 at 04:03 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t40717_5 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t40717_5 && git checkout t40717_5Patchset v5 (message #5) is on t40717_5
Hi,
JIT slot_compile_deform assumes there's at least 'natts' in TupleDesc, eg
/*
* Iterate over each attribute that needs to be deformed, build code to
* deform it.
*/
for (attnum = 0; attnum < natts; attnum++)
{
Form_pg_attribute att = TupleDescAttr(desc, attnum);
but a new TupleDesc has no attribute and the caller only tests
TupleDesc is not null.
Attachments:
jit_expr.patchtext/x-patch; charset=US-ASCII; name=jit_expr.patchDownload+1-1
didier <did447@gmail.com> writes:
JIT slot_compile_deform assumes there's at least 'natts' in TupleDesc, eg
/*
* Iterate over each attribute that needs to be deformed, build code to
* deform it.
*/
for (attnum = 0; attnum < natts; attnum++)
{
Form_pg_attribute att = TupleDescAttr(desc, attnum);
but a new TupleDesc has no attribute and the caller only tests
TupleDesc is not null.
I looked at this, but I find it quite unconvincing. Under what
circumstances would we not have a correctly filled-in tupdesc here?
regards, tom lane
Extensions can do it, timescaledb in this case with:
INSERT INTO ... RETURNING *;
Or replacing the test in llvm_compile_expr with an Assert in
slot_compile_deform ?
Hi,
On June 13, 2019 11:08:15 AM PDT, didier <did447@gmail.com> wrote:
Extensions can do it, timescaledb in this case with:
INSERT INTO ... RETURNING *;Or replacing the test in llvm_compile_expr with an Assert in
slot_compile_deform ?
In that case we ought to never generate a deform expression step - core code doesn't afair. That's only done I'd there's actually something to deform. I'm fine with adding an assert tough
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Hi,
I searched the mailing list but found nothing. Any reason why
TupleDescAttr is a macro and not a static inline?
Rather than adding an Assert attached POC replace TupleDescAttr macro
by a static inline function with AssertArg.
It:
- Factorize Assert.
- Trigger an Assert in JIT_deform if natts is wrong.
- Currently In HEAD
src/backend/access/common/tupdesc.c:TupleDescCopyEntry() compiler can
optimize out AssertArg(PointerIsValid(...)), no idea
if compiling with both cassert and -O2 make sense though).
- Remove two UB in memcpy when natts is zero.
Note:
Comment line 1480 in ../contrib/tablefunc/tablefunc.c is wrong it's
the fourth column.
Regards
Didier
Show quoted text
On Thu, Jun 13, 2019 at 8:35 PM Andres Freund <andres@anarazel.de> wrote:
Hi,
On June 13, 2019 11:08:15 AM PDT, didier <did447@gmail.com> wrote:
Extensions can do it, timescaledb in this case with:
INSERT INTO ... RETURNING *;Or replacing the test in llvm_compile_expr with an Assert in
slot_compile_deform ?In that case we ought to never generate a deform expression step - core code doesn't afair. That's only done I'd there's actually something to deform. I'm fine with adding an assert tough
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Hi,
I still haven't heard an explanation why you see a problem here.
On 2019-06-27 15:54:28 +0200, didier wrote:
I searched the mailing list but found nothing. Any reason why
TupleDescAttr is a macro and not a static inline?
It's present in branches that can't rely on static inlines being
present. Obviously we can still change it in HEAD, because there we rely
on static inlien functions working (althoug we might need to surround it
with #ifndef FRONTEND, if tupdesc.h is included from other headers
legitimately needed from frontend code).
Rather than adding an Assert attached POC replace TupleDescAttr macro
by a static inline function with AssertArg.
It:
- Factorize Assert.- Trigger an Assert in JIT_deform if natts is wrong.
- Currently In HEAD
src/backend/access/common/tupdesc.c:TupleDescCopyEntry() compiler can
optimize out AssertArg(PointerIsValid(...)), no idea
if compiling with both cassert and -O2 make sense though).
It's not important.
- Remove two UB in memcpy when natts is zero.
I don't think it matters, but I'm not actually sure this is actually
UB. It's IIRC legal to form a pointer to one after the end of an array
(but not dereference, obviously), and memcpy with a 0 length byte also
is legal.
Note:
Comment line 1480 in ../contrib/tablefunc/tablefunc.c is wrong it's
the fourth column.
Huh, this is of very long-standing vintage. Think it's been introduced
in
commit a265b7f70aa01a34ae30554186ee8c2089e035d8
Author: Bruce Momjian <bruce@momjian.us>
Date: 2003-07-27 03:51:59 +0000
Greetings,
Andres Freund