On 2023-Feb-15, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> > Here are two patches that refactor the mostly repetitive "${object} is
> > visible" and get_${object}_oid() functions in namespace.c. This uses
> > the functions in objectaddress.c to look up the appropriate per-catalog
> > system caches and attribute numbers, similar to other refactoring
> > patches I have posted recently.
>
> This does not look like a simple refactoring patch to me. I have
> very serious concerns first about whether it even preserves the
> existing semantics, and second about whether there is a performance
> penalty.
I suppose there are two possible questionable angles from a performance
POV:
1. the new code uses get_object_property_data() when previously there
was a straight dereference after casting to the right struct type. So
how expensive is that? I think the answer to that is not great, because
it does a linear array scan on ObjectProperty. Maybe we need a better
answer.
2. other accesses to the data are done using SysCacheGetAttr instead of
straight struct access dereferences. I expect that most of the fields
being accessed have attcacheoff set, which allows pretty fast access to
the field in question, so it's not *that* bad. (For cases where
attcacheoff is not set, then the original code would also have to deform
the tuple.) Still, it's going to have nontrivial impact in any
microbenchmarking.
That said, I think most of this code is invoked for DDL, where
performance is not so critical; probably just fixing
get_object_property_data to not be so naïve would suffice.
Queries are another matter. I can't think of a way to determine which
of these paths are used for queries, so that we can optimize more (IOW:
just plain not rewrite those); manually going through the callers seems
a bit laborious, but perhaps doable.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
Thou shalt check the array bounds of all strings (indeed, all arrays), for
surely where thou typest "foo" someone someday shall type
"supercalifragilisticexpialidocious" (5th Commandment for C programmers)
