| From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
|---|---|
| To: | Amit Langote <amitlan(at)postgresql(dot)org> |
| Cc: | pgsql-committers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: pgsql: Invalidate RI fast-path metadata on operator family changes |
| Date: | 2026-09-19 09:14:28 |
| Message-ID: | CA+HiwqHO2xu3eR6bZ99qSQgCTnoS9VC4e0KW25bxE5kmkRMGyQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-committers |
On Sat, Sep 19, 2026 at 5:39 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Sat, Sep 19, 2026 at 3:59 PM Amit Langote <amitlan(at)postgresql(dot)org> wrote:
> >
> > Invalidate RI fast-path metadata on operator family changes
> >
> > The RI fast path checks a foreign key by probing the referenced
> > unique index directly, using the equality operator recorded for
> > the constraint. Whether the fast path can be used is decided once
> > and cached in RI_ConstraintInfo, and that cache is invalidated on
> > pg_constraint changes but not on pg_amop. So after an ALTER
> > OPERATOR FAMILY drops the recorded operator and adds another in its
> > place, the cached decision is stale, and the next fast-path check
> > probes the index with an operator no longer in the opfamily and
> > errors out with "operator XXX is not a member of opfamily XXX".
> > The SPI path is unaffected, because the planner just stops matching
> > the index.
> >
> > To fix, register an AMOPOPID syscache callback to flush the RI
> > cache on pg_amop changes, and have ri_check_fastpath_index()
> > recheck that the recorded operator is still the equality member of
> > the index's opfamily, falling back to SPI when it is not.
> >
> > Add regression test coverage.
> >
> > Reported-by: Nikolay Samokhvalov <nik(at)postgres(dot)ai>
> > Author: Nikolay Samokhvalov <nik(at)postgres(dot)ai>
> > Discussion: https://www.postgr.es/m/CAM527d9PzFzagr67N0%3DEx2ng1p5HzrcAszy3j5OoZKHXQMARXA%40mail.gmail.com
> > Backpatch-through: 19
> >
> > Branch
> > ------
> > master
> >
> > Details
> > -------
> > https://git.postgresql.org/pg/commitdiff/c62b330912e2095dc8dee2f749adf7e5d94ca611
> >
> > Modified Files
> > --------------
> > src/backend/utils/adt/ri_triggers.c | 45 ++++++++++++++++++++-
> > src/test/regress/expected/foreign_key.out | 67 +++++++++++++++++++++++++++++++
> > src/test/regress/sql/foreign_key.sql | 56 ++++++++++++++++++++++++++
> > 3 files changed, 166 insertions(+), 2 deletions(-)
>
> I noticed a few BF failures, after pushing the above changes:
>
> sifaka / master:
>
> # diff -U3 /Users/buildfarm/bf-data/HEAD/pgsql.build/src/test/regress/expected/window.out
> /Users/buildfarm/bf-data/HEAD/pgsql.build/src/test/regress/results/window.out
> # --- /Users/buildfarm/bf-data/HEAD/pgsql.build/src/test/regress/expected/window.out
> 2026-09-19 02:53:30
> # +++ /Users/buildfarm/bf-data/HEAD/pgsql.build/src/test/regress/results/window.out
> 2026-09-19 03:22:01
> # @@ -5658,16 +5658,19 @@
> # SELECT COUNT(*) OVER (ORDER BY t1.unique1)
> # FROM tenk1 t1 INNER JOIN tenk1 t2 ON t1.unique1 = t2.tenthous
> # LIMIT 1;
> # - QUERY PLAN
> # ---------------------------------------------------------------------------
> # + QUERY PLAN
> # +--------------------------------------------------------------------------------------
> # Limit
> # -> WindowAgg
> # Window: w1 AS (ORDER BY t1.unique1)
> # - -> Nested Loop
> # - -> Index Only Scan using tenk1_unique1 on tenk1 t1
> # - -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2
> # - Index Cond: (tenthous = t1.unique1)
> # -(7 rows)
> # + -> Sort
> # + Sort Key: t1.unique1
> # + -> Hash Join
> # + Hash Cond: (t1.unique1 = t2.tenthous)
> # + -> Index Only Scan using tenk1_unique1 on tenk1 t1
> # + -> Hash
> # + -> Index Only Scan using
> tenk1_thous_tenthous on tenk1 t2
> # +(10 rows)
> #
> # -- Ensure we get a cheap total plan. Lack of ORDER BY in the WindowClause
> # -- means that all rows must be read from the join, so a cheap startup plan
>
> sifaka / REL_19_STABLE:
>
> # diff -U3 /Users/buildfarm/bf-data/REL_19_STABLE/pgsql.build/src/test/regress/expected/window.out
> /Users/buildfarm/bf-data/REL_19_STABLE/pgsql.build/src/test/modules/test_plan_advice/tmp_check/results/window.out
> # --- /Users/buildfarm/bf-data/REL_19_STABLE/pgsql.build/src/test/regress/expected/window.out
> 2026-09-19 02:53:07
> # +++ /Users/buildfarm/bf-data/REL_19_STABLE/pgsql.build/src/test/modules/test_plan_advice/tmp_check/results/window.out
> 2026-09-19 03:09:56
> # @@ -5658,16 +5658,19 @@
> # SELECT COUNT(*) OVER (ORDER BY t1.unique1)
> # FROM tenk1 t1 INNER JOIN tenk1 t2 ON t1.unique1 = t2.tenthous
> # LIMIT 1;
> # - QUERY PLAN
> # ---------------------------------------------------------------------------
> # + QUERY PLAN
> # +--------------------------------------------------------------------------------------
> # Limit
> # -> WindowAgg
> # Window: w1 AS (ORDER BY t1.unique1)
> # - -> Nested Loop
> # - -> Index Only Scan using tenk1_unique1 on tenk1 t1
> # - -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2
> # - Index Cond: (tenthous = t1.unique1)
> # -(7 rows)
> # + -> Sort
> # + Sort Key: t1.unique1
> # + -> Hash Join
> # + Hash Cond: (t1.unique1 = t2.tenthous)
> # + -> Index Only Scan using tenk1_unique1 on tenk1 t1
> # + -> Hash
> # + -> Index Only Scan using
> tenk1_thous_tenthous on tenk1 t2
> # +(10 rows)
> #
> # -- Ensure we get a cheap total plan. Lack of ORDER BY in the WindowClause
> # -- means that all rows must be read from the join, so a cheap startup plan
>
> prion / REL_19_STABLE:
>
> diff -U3 /home/ec2-user/bf/root/REL_19_STABLE/pgsql/src/test/regress/expected/window.out
> /home/ec2-user/bf/root/REL_19_STABLE/pgsql.build/src/test/modules/test_plan_advice/tmp_check/results/window.out
> --- /home/ec2-user/bf/root/REL_19_STABLE/pgsql/src/test/regress/expected/window.out
> 2026-09-19 07:23:03.537952198 +0000
> +++ /home/ec2-user/bf/root/REL_19_STABLE/pgsql.build/src/test/modules/test_plan_advice/tmp_check/results/window.out
> 2026-09-19 08:16:48.579365765 +0000
> @@ -3634,7 +3634,7 @@
> WindowAgg
> Window: w1 AS (PARTITION BY f1 ORDER BY f2 RANGE BETWEEN
> '1'::bigint PRECEDING AND '1'::bigint FOLLOWING)
> -> Sort
> - Sort Key: f1
> + Sort Key: f1, f2
> -> Seq Scan on t1
> Filter: (f1 = f2)
> (6 rows)
> @@ -3681,7 +3681,7 @@
> WindowAgg
> Window: w1 AS (PARTITION BY f1 ORDER BY f2 GROUPS BETWEEN
> '1'::bigint PRECEDING AND '1'::bigint FOLLOWING)
> -> Sort
> - Sort Key: f1
> + Sort Key: f1, f2
> -> Seq Scan on t1
> Filter: (f1 = f2)
> (6 rows)
>
> turaca / master:
>
> # # diff -U3 /mnt/data/buildfarm/buildroot/HEAD/pgsql.build/src/test/regress/expected/window.out
> /mnt/data/buildfarm/buildroot/HEAD/pgsql.build/src/bin/pg_upgrade/tmp_check/results/window.out
> # # --- /mnt/data/buildfarm/buildroot/HEAD/pgsql.build/src/test/regress/expected/window.out
> 2026-09-19 07:53:30.000000000 +0100
> # # +++ /mnt/data/buildfarm/buildroot/HEAD/pgsql.build/src/bin/pg_upgrade/tmp_check/results/window.out
> 2026-09-19 09:00:04.647674224 +0100
> # # @@ -3634,7 +3634,7 @@
> # # WindowAgg
> # # Window: w1 AS (PARTITION BY f1 ORDER BY f2 RANGE BETWEEN
> '1'::bigint PRECEDING AND '1'::bigint FOLLOWING)
> # # -> Sort
> # # - Sort Key: f1
> # # + Sort Key: f1, f1
> # # -> Seq Scan on t1
> # # Filter: (f1 = f2)
> # # (6 rows)
> # # 1 of 239 tests failed.
> # # The differences that caused some tests to fail can be viewed in
> the file "/mnt/data/buildfarm/buildroot/HEAD/pgsql.build/src/bin/pg_upgrade/tmp_check/regression.diffs".
> # # A copy of the test summary that you see above is saved in the file
> "/mnt/data/buildfarm/buildroot/HEAD/pgsql.build/src/bin/pg_upgrade/tmp_check/regression.out".
>
> I don't see any relation between these failures and my commit. window
> runs concurrently with foreign_key, but this commit's additions to the
> latter are self-contained (a private operator family in its own
> schema), so I don't see why window's plans would be affected.
>
> Does anyone see it differently?
Forgot to add that the code change is isolated too in that it only
changes what ri_triggers.c does on a pg_amop invalidation during an RI
fast-path check, not anything the planner consults. It doesn't touch
pg_constraint or how FKs feed planning, so I don't see a route from
this commit's changes to these window plans. Any effect would be
timing at most. I would not claim that I have figured this out.
--
Thanks, Amit Langote
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-09-19 11:16:01 | pgsql: Fix PASSING of toasted text values in JSON query functions |
| Previous Message | Amit Langote | 2026-09-19 08:39:21 | Re: pgsql: Invalidate RI fast-path metadata on operator family changes |