| From: | David Rowley <dgrowleyml(at)gmail(dot)com> |
|---|---|
| To: | Andres Freund <andres(at)anarazel(dot)de> |
| Cc: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: More speedups for tuple deformation |
| Date: | 2026-01-21 05:00:21 |
| Message-ID: | CAApHDvpDxDFatUskuOfuM7A3VESrx8U7MtYnU_HiB0QLAg94zg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, 21 Jan 2026 at 07:38, Andres Freund <andres(at)anarazel(dot)de> wrote:
> I wonder if it's possible to split the patch - it's big enough to be
> nontrivial to review... Perhaps the finalization could be introduced
> separately from the patch actually making use of it?
Seems reasonable. I've done that in the attached 0001, which contains
a dummy macro for TupleDescFinalize() and all the required calls to
it.
> I wonder if we should somehow change the API of tupledesc creation, to make
> old code that doesn't have TupleDescFinalize() fail to compile, instead of
> just warn...
I don't have any ideas on how to do that. I could maybe imagine some
preprocessor magic if we always expected a CreateTupleDesc() and
TupleDescFinalize() in the same function, but the TupleDescFinalize()
may be required after any modification to the TupleDesc that could
invalidate the processing that's done within that function.
> Think it'd be worth adding an assertion to BlessTupleDesc that
> TupleDescFinalize has been called, I think that'll lead to easier to
> understand backtraces in a lot of cases. Particularly if you consider cases
> where BlessTupleDesc() will create a tupdesc in shared memory, that could then
> trigger an assertion failure in a parallel worker or such.
Modified.
> Maybe add an assert for cattr->attbyval? Just to avoid a bad debugging
> experience if somebody tries to extend this logic to
> e.g. non-null-fixed-width-byref columns?
I ended up removing the OPTIMIZE_BYVAL code in the attached. Over all
the machines I tested on, with the benchmark results I previously
shared, it seemed to cause a slowdown rather than a speedup. Perhaps
it can be refined and tried again later, but I've removed it for now
to reduce complexity.
> I also wonder if we could have assert-only crosschecking of the "real" offsets
> against the cached ones?
I've modified the code to do that. v5 patches attached.
Thanks for reviewing.
David
| Attachment | Content-Type | Size |
|---|---|---|
| v5-0001-Add-empty-TupleDescFinalize-function.patch | text/plain | 29.0 KB |
| v5-0002-Precalculate-CompactAttribute-s-attcacheoff.patch | text/plain | 49.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | jian he | 2026-01-21 05:02:27 | Re: CREATE TABLE LIKE INCLUDING POLICIES |
| Previous Message | Tatsuro Yamada | 2026-01-21 04:40:31 | Re: [PATCH] psql: add \dcs to list all constraints |