postgres_fdw versus regconfig and similar constants

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: postgres_fdw versus regconfig and similar constants
Date: 2022-05-16 17:33:26
Message-ID: 1423433.1652722406@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Bug #17483 points out that postgres_fdw falls down pretty badly when
a potentially shippable clause contains a "regconfig" constant [1].
It doesn't check whether the constant refers to an object that's
likely to exist on the remote side, and it fails to ensure that
the printed name is properly schema-qualified. The same flaws apply
to constants of other OID alias types. Below are some draft patches
to address this.

0001 deals with the lack-of-schema-qualification issue by forcing
search_path to be just "pg_catalog" while we're deparsing constants.
This seems straightforward, if annoyingly expensive, and it's enough
to fix the bug as presented.

0002 tightens deparse.c's rules to only consider an OID alias constant
as shippable if the object it refers to is shippable. This seems
obvious in hindsight; I wonder how come we've not realized it before?
However, this has one rather nasty problem for regconfig in particular:
with our standard shippability rules none of the built-in text search
configurations would be treated as shippable, because initdb gives them
non-fixed OIDs above 9999. That seems like a performance hit we don't
want to take. In the attached, I hacked around that by making a special
exception for OIDs up to 16383, but that seems like a fairly ugly kluge.
Anybody have a better idea?

While using find_expr_references() as a reference for writing the new code
in 0002, I was dismayed to find that it omitted handling regcollation;
and a quick search showed that other places that specially process REG*
types hadn't been updated for that addition either. 0003 closes those
oversights.

I've split this into three parts partially because they probably should be
back-patched differently. It seems like 0001 should go into all branches.
0003 should go back to v13 where regcollation was added. But I wonder if
0002 should get back-patched at all: it seems like we're more likely to
get performance complaints about quals no longer being shipped than we are
to get kudos for not mistakenly shipping an unportable tsconfig reference.
People could fix such performance issues by putting the config into an
extension marked safe-to-ship, but they probably won't want to have to
deal with that in a minor release.

I've not done anything about a regression test yet.

Thoughts?

regards, tom lane

[1] https://www.postgresql.org/message-id/17483-795757fa99607659%40postgresql.org

Attachment Content-Type Size
0001-print-qualified-regconfig-values.patch text/x-diff 636 bytes
0002-ship-only-shippable-regfoo-consts.patch text/x-diff 3.1 KB
0003-fix-regcollation-oversights.patch text/x-diff 1.8 KB

Browse pgsql-hackers by date

  From Date Subject
Next Message Erik Rijkers 2022-05-16 17:52:43 Re: JSON Functions and Operators Docs for v15
Previous Message Bruce Momjian 2022-05-16 16:42:57 Re: First draft of the PG 15 release notes