From 31e5b502920351ddcf8f5efa7f2dc4a60ecdca3a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 17 Jul 2022 18:11:22 -0400 Subject: [PATCH] postgres_fdw: be more wary about shippability of reg* constants. Don't consider a constant of regconfig or other reg* types to be shippable unless it refers to a built-in object, or an object in an extension that's been marked shippable. Without this restriction, we're too likely to send a constant that will fail to parse on the remote server. For the regconfig type only, consider OIDs up to 16383 to be "built in", rather than the normal cutoff of 9999. Otherwise the initdb-created text search configurations will be considered unshippable, which is unlikely to make anyone happy. It's possible that this new restriction will de-optimize queries that were working satisfactorily before. Users can restore any lost performance by making sure that objects that can be expected to exist on the remote side are in shippable extensions. However, that's not a change that people are likely to be happy about having to make after a minor-release update. Between that consideration and the lack of field complaints, let's just change this in HEAD. Noted while fixing bug #17483, although this is not precisely the problem that that report complained about. Discussion: https://postgr.es/m/1423433.1652722406@sss.pgh.pa.us --- contrib/postgres_fdw/deparse.c | 72 +++++++++++++++++++ .../postgres_fdw/expected/postgres_fdw.out | 25 ++++++- contrib/postgres_fdw/sql/postgres_fdw.sql | 12 +++- 3 files changed, 107 insertions(+), 2 deletions(-) diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index 8f4d8a5022..a9766f9734 100644 --- a/contrib/postgres_fdw/deparse.c +++ b/contrib/postgres_fdw/deparse.c @@ -37,11 +37,14 @@ #include "access/sysattr.h" #include "access/table.h" #include "catalog/pg_aggregate.h" +#include "catalog/pg_authid.h" #include "catalog/pg_collation.h" #include "catalog/pg_namespace.h" #include "catalog/pg_operator.h" #include "catalog/pg_opfamily.h" #include "catalog/pg_proc.h" +#include "catalog/pg_ts_config.h" +#include "catalog/pg_ts_dict.h" #include "catalog/pg_type.h" #include "commands/defrem.h" #include "nodes/makefuncs.h" @@ -384,6 +387,75 @@ foreign_expr_walker(Node *node, { Const *c = (Const *) node; + /* + * Constants of regproc and related types can't be shipped + * unless the referenced object is shippable. But NULL's ok. + * (See also the related code in dependency.c.) + */ + if (!c->constisnull) + { + switch (c->consttype) + { + case REGPROCOID: + case REGPROCEDUREOID: + if (!is_shippable(DatumGetObjectId(c->constvalue), + ProcedureRelationId, fpinfo)) + return false; + break; + case REGOPEROID: + case REGOPERATOROID: + if (!is_shippable(DatumGetObjectId(c->constvalue), + OperatorRelationId, fpinfo)) + return false; + break; + case REGCLASSOID: + if (!is_shippable(DatumGetObjectId(c->constvalue), + RelationRelationId, fpinfo)) + return false; + break; + case REGTYPEOID: + if (!is_shippable(DatumGetObjectId(c->constvalue), + TypeRelationId, fpinfo)) + return false; + break; + case REGCOLLATIONOID: + if (!is_shippable(DatumGetObjectId(c->constvalue), + CollationRelationId, fpinfo)) + return false; + break; + case REGCONFIGOID: + + /* + * For text search objects only, we weaken the + * normal shippability criterion to allow all OIDs + * below FirstNormalObjectId. Without this, none + * of the initdb-installed TS configurations would + * be shippable, which would be quite annoying. + */ + if (DatumGetObjectId(c->constvalue) >= FirstNormalObjectId && + !is_shippable(DatumGetObjectId(c->constvalue), + TSConfigRelationId, fpinfo)) + return false; + break; + case REGDICTIONARYOID: + if (DatumGetObjectId(c->constvalue) >= FirstNormalObjectId && + !is_shippable(DatumGetObjectId(c->constvalue), + TSDictionaryRelationId, fpinfo)) + return false; + break; + case REGNAMESPACEOID: + if (!is_shippable(DatumGetObjectId(c->constvalue), + NamespaceRelationId, fpinfo)) + return false; + break; + case REGROLEOID: + if (!is_shippable(DatumGetObjectId(c->constvalue), + AuthIdRelationId, fpinfo)) + return false; + break; + } + } + /* * If the constant has nondefault collation, either it's of a * non-builtin type, or it reflects folding of a CollateExpr. diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index a6c1b16275..ebf9ea3598 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -1163,11 +1163,34 @@ SELECT * FROM ft1 WHERE CASE c3 COLLATE "C" WHEN c6 THEN true ELSE c3 < 'bar' EN Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" (4 rows) --- check schema-qualification of regconfig constant +-- a regconfig constant referring to this text search configuration +-- is initially unshippable CREATE TEXT SEARCH CONFIGURATION public.custom_search (COPY = pg_catalog.english); EXPLAIN (VERBOSE, COSTS OFF) SELECT c1, to_tsvector('custom_search'::regconfig, c3) FROM ft1 +WHERE c1 = 642 AND length(to_tsvector('custom_search'::regconfig, c3)) > 0; + QUERY PLAN +------------------------------------------------------------------------- + Foreign Scan on public.ft1 + Output: c1, to_tsvector('custom_search'::regconfig, c3) + Filter: (length(to_tsvector('custom_search'::regconfig, ft1.c3)) > 0) + Remote SQL: SELECT "C 1", c3 FROM "S 1"."T 1" WHERE (("C 1" = 642)) +(4 rows) + +SELECT c1, to_tsvector('custom_search'::regconfig, c3) FROM ft1 +WHERE c1 = 642 AND length(to_tsvector('custom_search'::regconfig, c3)) > 0; + c1 | to_tsvector +-----+------------- + 642 | '00642':1 +(1 row) + +-- but if it's in a shippable extension, it can be shipped +ALTER EXTENSION postgres_fdw ADD TEXT SEARCH CONFIGURATION public.custom_search; +-- however, that doesn't flush the shippability cache, so do a quick reconnect +\c - +EXPLAIN (VERBOSE, COSTS OFF) +SELECT c1, to_tsvector('custom_search'::regconfig, c3) FROM ft1 WHERE c1 = 642 AND length(to_tsvector('custom_search'::regconfig, c3)) > 0; QUERY PLAN ---------------------------------------------------------------------------------------------------------------------------------------------- diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index a4f988fdb8..b7817c5a41 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -442,7 +442,8 @@ SELECT * FROM ft1 WHERE CASE c3 WHEN c6 THEN true ELSE c3 < 'bar' END; EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 WHERE CASE c3 COLLATE "C" WHEN c6 THEN true ELSE c3 < 'bar' END; --- check schema-qualification of regconfig constant +-- a regconfig constant referring to this text search configuration +-- is initially unshippable CREATE TEXT SEARCH CONFIGURATION public.custom_search (COPY = pg_catalog.english); EXPLAIN (VERBOSE, COSTS OFF) @@ -450,6 +451,15 @@ SELECT c1, to_tsvector('custom_search'::regconfig, c3) FROM ft1 WHERE c1 = 642 AND length(to_tsvector('custom_search'::regconfig, c3)) > 0; SELECT c1, to_tsvector('custom_search'::regconfig, c3) FROM ft1 WHERE c1 = 642 AND length(to_tsvector('custom_search'::regconfig, c3)) > 0; +-- but if it's in a shippable extension, it can be shipped +ALTER EXTENSION postgres_fdw ADD TEXT SEARCH CONFIGURATION public.custom_search; +-- however, that doesn't flush the shippability cache, so do a quick reconnect +\c - +EXPLAIN (VERBOSE, COSTS OFF) +SELECT c1, to_tsvector('custom_search'::regconfig, c3) FROM ft1 +WHERE c1 = 642 AND length(to_tsvector('custom_search'::regconfig, c3)) > 0; +SELECT c1, to_tsvector('custom_search'::regconfig, c3) FROM ft1 +WHERE c1 = 642 AND length(to_tsvector('custom_search'::regconfig, c3)) > 0; -- =================================================================== -- JOIN queries