From 85fe8d9cac341cea26637ef4d335753f36b7e23b Mon Sep 17 00:00:00 2001 From: himmel Date: Tue, 8 Sep 2026 01:52:45 +0000 Subject: [PATCH 1/3] Add a statistics-aware selectivity estimator for agtype @> and @>> A MATCH property constraint such as (n:Label {key: value}) is compiled to `properties @> '{"key": value}'`. Neither stock estimator can see through that. contsel returns a fixed 0.001. matchingsel, the binding before #2356, consults the statistics of the whole properties column, which say nothing about the distribution of any single key, and it is expensive: it calls agtype_contains() once per MCV and histogram entry, which is why 639c8d5a reverted it. Both give {person_id: } and {city: } the same estimate. On a multi-hop MATCH the resulting overestimate of the start vertex pushes the planner away from per-vertex index probes and toward a full scan of every edge label table joined with a hash or merge join. On a 3M-vertex graph with three 3M-row edge tables, `(a:VTABLE {person_id: '...'})-->()-->(c)` estimates the start at 3000 rows instead of 1 and degenerates into a Parallel Seq Scan of all three edge tables: cost 156134, 691 ms. With this estimator the start estimates at 1, the second hop becomes a parameterized Bitmap Index Scan on start_id, and the same query costs 25932 and runs in 0.64 ms. agtype_contains_sel() decomposes the containment constant exactly the way the parser does when age.enable_containment = off (transform_map_to_ind_recursive for @>, transform_map_to_ind_top_level for @>>): one equality per leaf on agtype_access_operator(VARIADIC ARRAY[properties, '"key"', ...]) That is the expression users index or attach extended statistics to. build_access_expr() synthesizes the same node shape transform_A_Indirection produces, so examine_variable() matches an expression index or a statistics object by structural equality, and var_eq_const() turns the statistics into the selectivity an explicit WHERE n.key = value gets. The two ways of writing the filter now estimate identically. Nested keys extend the array; per-leaf selectivities are multiplied. The properties column's own statistics are never read and agtype_contains() is never called at plan time. Fallback contract, so that the #2356 planning regression cannot recur: the estimator returns the 0.001 contsel produced when age.enable_containment_statistics is off, when the operand is not a constant non-empty object, when the relation has neither an expression index nor extended statistics, or when no leaf finds statistics. The relation gate only inspects rel->statlist and rel->indexlist, both already in memory, so a relation without expression statistics performs no node synthesis at all and gets byte-identical plans. Only the RESTRICT bindings of @> and @>> change. JOIN selectivity stays contjoinsel, and <@, <<@, ?, ?| and ?& are untouched. Also adds age.enable_containment_statistics (boolean, PGC_USERSET, default on) so the estimator can be disabled per session, and extends the containment_selectivity regression test: the existing guard that no matchingsel estimate leaks is kept, and assertions are added that an inline map estimates the same as the equivalent WHERE clause for unique, low-cardinality, nested and mixed constraints, and that a relation without statistics, an unknown key, and the GUC turned off all still produce the contsel estimate. --- Makefile | 1 + age--1.8.0--y.y.y.sql | 34 ++ regress/expected/containment_selectivity.out | 276 +++++++++--- regress/sql/containment_selectivity.sql | 179 ++++++-- sql/agtype_operators.sql | 19 +- src/backend/utils/adt/agtype_selfuncs.c | 424 +++++++++++++++++++ src/backend/utils/ag_guc.c | 11 + src/include/utils/ag_guc.h | 1 + 8 files changed, 861 insertions(+), 84 deletions(-) create mode 100644 src/backend/utils/adt/agtype_selfuncs.c diff --git a/Makefile b/Makefile index 2ea8f64ed..f2f819b9a 100644 --- a/Makefile +++ b/Makefile @@ -142,6 +142,7 @@ OBJS = src/backend/age.o \ src/backend/utils/adt/agtype.o \ src/backend/utils/adt/agtype_ext.o \ src/backend/utils/adt/agtype_gin.o \ + src/backend/utils/adt/agtype_selfuncs.o \ src/backend/utils/adt/agtype_ops.o \ src/backend/utils/adt/agtype_parser.o \ src/backend/utils/adt/agtype_util.o \ diff --git a/age--1.8.0--y.y.y.sql b/age--1.8.0--y.y.y.sql index 4c85b28ca..e7f1ecbb3 100644 --- a/age--1.8.0--y.y.y.sql +++ b/age--1.8.0--y.y.y.sql @@ -33,3 +33,37 @@ --* Please add all additions, deletions, and modifications to the end of this --* file. We need to keep the order of these changes. --* REMOVE ALL LINES ABOVE, and this one, that start with --* + +-- +-- Statistics-aware restriction selectivity for the agtype containment +-- operators. +-- +-- @> and @>> were bound to contsel, which returns a fixed 0.001 without +-- reading statistics. On a MATCH with an inline property map that makes the +-- start vertex look like "0.1% of the table" regardless of the data, and on +-- multi-hop patterns the overestimate pushes the planner from per-vertex +-- index probes to a full scan of every edge table plus a hash or merge join. +-- +-- agtype_contains_sel decomposes the constant into per-key equalities on +-- agtype_access_operator() and uses the expression statistics attached to +-- that expression (expression index or CREATE STATISTICS). With no such +-- statistics it returns the same 0.001 contsel did, so plans are unchanged +-- for installations that have not created any. +-- +-- The JOIN estimator stays contjoinsel. <@, <<@ and the key-existence +-- operators are unchanged. +-- + +CREATE FUNCTION ag_catalog.agtype_contains_sel(internal, oid, internal, integer) + RETURNS float8 + LANGUAGE c + STABLE + STRICT + PARALLEL SAFE +AS 'MODULE_PATHNAME'; + +ALTER OPERATOR ag_catalog.@> (agtype, agtype) + SET (RESTRICT = ag_catalog.agtype_contains_sel); + +ALTER OPERATOR ag_catalog.@>> (agtype, agtype) + SET (RESTRICT = ag_catalog.agtype_contains_sel); diff --git a/regress/expected/containment_selectivity.out b/regress/expected/containment_selectivity.out index a0626d554..c2e62dee8 100644 --- a/regress/expected/containment_selectivity.out +++ b/regress/expected/containment_selectivity.out @@ -17,18 +17,35 @@ * under the License. */ /* - * Regression coverage for issue #2356: - * The containment (@>, <@, @>>, <<@) and key-existence (?, ?|, ?&) - * operators on agtype must be bound to the lightweight selectivity - * helpers contsel / contjoinsel during planning. Earlier PG14+ - * branches used matchingsel / matchingjoinsel, which caused planning - * to invoke agtype_contains() against pg_statistic MCVs and produced - * a 30%+ planning-time regression on point queries (severe TPS drop - * reported on the PG18 branch). + * Selectivity bindings for the agtype containment (@>, <@, @>>, <<@) and + * key-existence (?, ?|, ?&) operators, and behaviour of the statistics-aware + * estimator bound to @> and @>>. * - * This test pins the bindings by querying pg_operator directly. If - * someone re-introduces matchingsel here, the test diff is loud and - * precise. + * History: + * - Before #2356 all seven operators used matchingsel / matchingjoinsel. + * matchingsel probes the statistics of the whole properties column and + * calls agtype_contains() once per MCV / histogram entry at plan time, + * which produced a 30%+ planning-time regression on point queries. Those + * whole-column statistics also carry no information about any one key, + * so the estimate was wrong anyway (it bottoms out at PostgreSQL's 1e-4 + * floor). #2356 rebound everything to contsel / contjoinsel. + * - contsel returns a fixed 0.001. On a multi-hop MATCH the resulting + * overestimate of a selective start vertex pushes the planner from + * per-vertex index probes to a full scan of every edge table. @> and @>> + * are now bound to agtype_contains_sel, which decomposes the constant + * into per-key equalities on agtype_access_operator(properties, '"key"') + * and reads the expression statistics users attach to that expression. + * It never touches the properties column's statistics and never calls + * agtype_contains() at plan time, so the #2356 regression cannot recur. + * Without expression statistics it returns exactly what contsel did. + * + * This file pins the bindings via pg_operator so that a re-introduction of + * matchingsel is loud, and asserts the estimator's two contracts: + * 1. an inline property map and the equivalent WHERE clause estimate the + * same number of rows once expression statistics exist; + * 2. without expression statistics, or with + * age.enable_containment_statistics = off, the estimate is the one + * contsel produced. */ LOAD 'age'; SET search_path TO ag_catalog; @@ -43,12 +60,12 @@ JOIN pg_catalog.pg_namespace n ON n.oid = o.oprnamespace WHERE n.nspname = 'ag_catalog' AND o.oprname IN ('@>', '<@', '@>>', '<<@') ORDER BY o.oprname, lhs, rhs; - oprname | lhs | rhs | restrict_fn | join_fn ----------+--------+--------+-------------+------------- - <<@ | agtype | agtype | contsel | contjoinsel - <@ | agtype | agtype | contsel | contjoinsel - @> | agtype | agtype | contsel | contjoinsel - @>> | agtype | agtype | contsel | contjoinsel + oprname | lhs | rhs | restrict_fn | join_fn +---------+--------+--------+---------------------+------------- + <<@ | agtype | agtype | contsel | contjoinsel + <@ | agtype | agtype | contsel | contjoinsel + @> | agtype | agtype | agtype_contains_sel | contjoinsel + @>> | agtype | agtype | agtype_contains_sel | contjoinsel (4 rows) -- Selectivity helpers for all key-existence operator overloads @@ -73,12 +90,9 @@ ORDER BY o.oprname, lhs, rhs; ?| | agtype | text[] | contsel | contjoinsel (6 rows) --- Scoped guard for issue #2356: assert that none of the specific containment --- and key-existence operators on agtype are bound to matchingsel / --- matchingjoinsel. We deliberately limit the check to these operator names --- (rather than every operator in ag_catalog) so unrelated operators that --- legitimately use matchingsel for their own semantics are not affected by --- this regression test. +-- Scoped guard for issue #2356: none of these operators may be bound to +-- matchingsel / matchingjoinsel. The check is limited to these operator names +-- so unrelated operators that legitimately use matchingsel are not affected. SELECT COUNT(*) AS leaked_matchingsel_bindings FROM pg_catalog.pg_operator o JOIN pg_catalog.pg_namespace n ON n.oid = o.oprnamespace @@ -154,22 +168,179 @@ SELECT '{"a":1,"b":2}'::agtype ?& '["a","b"]'::agtype AS exists_all_a t (1 row) --- Upgrade-path assertion for issue #2356. -- --- The checks above cover a FRESH install: contsel / contjoinsel come straight --- from agtype_operators.sql and agtype_exists.sql. Existing installs instead --- pick up the fix from the ALTER OPERATOR ... SET (RESTRICT, JOIN) block that --- age--1.7.0--y.y.y.sql ships and "ALTER EXTENSION age UPDATE" replays. Nothing --- above exercises that block, so a silent regression in it would go unnoticed. +-- Estimator behaviour. +-- +-- csel_plan_rows() returns the planner's row estimate for the scan of the +-- given relation inside EXPLAIN output. Comparing two estimates (rather than +-- printing them) keeps the expected output free of magic numbers that would +-- drift with statistics targets. +-- +CREATE FUNCTION csel_plan_rows(q text, rel text) RETURNS numeric +LANGUAGE plpgsql AS $fn$ +DECLARE + j json; +BEGIN + EXECUTE 'EXPLAIN (FORMAT JSON, COSTS ON) ' || q INTO j; + RETURN (pg_catalog.jsonb_path_query_first( + j::jsonb, + ('$.** ? (@."Relation Name" == "' || rel || '")."Plan Rows"')::jsonpath + ))::text::numeric; +END +$fn$; +SELECT create_graph('csel'); +NOTICE: graph "csel" has been created + create_graph +-------------- + +(1 row) + +SELECT create_vlabel('csel', 'V'); +NOTICE: VLabel "V" has been created + create_vlabel +--------------- + +(1 row) + +-- 3000 vertices: uid unique, city one of 30, grp one of 4, addr.zip one of 10. +INSERT INTO csel."V" (id, properties) +SELECT ag_catalog._graphid((SELECT l.id FROM ag_catalog.ag_label l + JOIN ag_catalog.ag_graph g ON g.graphid = l.graph + WHERE g.name = 'csel' AND l.name = 'V'), i::bigint), + ('{"uid": ' || i || ', "city": "c' || (i % 30) || '", "grp": ' || + (i % 4) || ', "addr": {"zip": ' || (i % 10) || '}}')::agtype +FROM pg_catalog.generate_series(1, 3000) AS i; +ANALYZE csel."V"; +-- Before any expression statistics exist, the inline map must estimate +-- exactly as it did under contsel: a fixed 0.001 of the table. +CREATE TEMP TABLE csel_baseline AS +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {uid: 7}) RETURN n $$) AS (n agtype)$q$, 'V') AS rows_no_stats; +SELECT rows_no_stats = pg_catalog.round(3000 * 0.001) AS no_stats_matches_contsel +FROM csel_baseline; + no_stats_matches_contsel +-------------------------- + t +(1 row) + +-- Attach expression statistics: an index on uid and city, a statistics object +-- (no index) on the nested addr.zip. grp deliberately gets none. +CREATE INDEX csel_v_uid_idx ON csel."V" (agtype_access_operator(properties, '"uid"'::agtype)); +CREATE INDEX csel_v_city_idx ON csel."V" (agtype_access_operator(properties, '"city"'::agtype)); +CREATE STATISTICS csel_v_zip_stat ON (agtype_access_operator(properties, '"addr"'::agtype, '"zip"'::agtype)) FROM csel."V"; +ANALYZE csel."V"; +-- Contract 1: inline map == WHERE clause, per key shape. +-- unique key +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {uid: 7}) RETURN n $$) AS (n agtype)$q$, 'V') + = csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V) WHERE n.uid = 7 RETURN n $$) AS (n agtype)$q$, 'V') + AS unique_key_equal, + csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {uid: 7}) RETURN n $$) AS (n agtype)$q$, 'V') + AS unique_key_rows; + unique_key_equal | unique_key_rows +------------------+----------------- + t | 1 +(1 row) + +-- low-cardinality key (MCV hit) +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {city: 'c3'}) RETURN n $$) AS (n agtype)$q$, 'V') + = csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V) WHERE n.city = 'c3' RETURN n $$) AS (n agtype)$q$, 'V') + AS lowcard_key_equal, + csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {city: 'c3'}) RETURN n $$) AS (n agtype)$q$, 'V') + AS lowcard_key_rows; + lowcard_key_equal | lowcard_key_rows +-------------------+------------------ + t | 100 +(1 row) + +-- two keys, one with statistics and one without +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {city: 'c3', grp: 1}) RETURN n $$) AS (n agtype)$q$, 'V') + = csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V) WHERE n.city = 'c3' AND n.grp = 1 RETURN n $$) AS (n agtype)$q$, 'V') + AS mixed_keys_equal; + mixed_keys_equal +------------------ + t +(1 row) + +-- nested key backed by CREATE STATISTICS rather than an index +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {addr: {zip: 5}}) RETURN n $$) AS (n agtype)$q$, 'V') + = csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V) WHERE n.addr.zip = 5 RETURN n $$) AS (n agtype)$q$, 'V') + AS nested_key_equal; + nested_key_equal +------------------ + t +(1 row) + +-- top-level containment form (=properties) on a scalar key +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V ={uid: 7}) RETURN n $$) AS (n agtype)$q$, 'V') + = csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V) WHERE n.uid = 7 RETURN n $$) AS (n agtype)$q$, 'V') + AS top_level_equal; + top_level_equal +----------------- + t +(1 row) + +-- a key that has no statistics at all still estimates as contsel did +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {grp: 1}) RETURN n $$) AS (n agtype)$q$, 'V') + = rows_no_stats AS unknown_key_matches_contsel +FROM csel_baseline; + unknown_key_matches_contsel +----------------------------- + t +(1 row) + +-- Contract 2: the GUC restores the contsel estimate. +SET age.enable_containment_statistics = off; +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {uid: 7}) RETURN n $$) AS (n agtype)$q$, 'V') + = rows_no_stats AS guc_off_matches_contsel +FROM csel_baseline; + guc_off_matches_contsel +------------------------- + t +(1 row) + +RESET age.enable_containment_statistics; +-- Results are unaffected by the estimator. +SELECT * FROM cypher('csel', $$ MATCH (n:V {uid: 7}) RETURN n.city $$) AS (city agtype); + city +------ + "c7" +(1 row) + +SELECT * FROM cypher('csel', $$ MATCH (n:V {city: 'c3', grp: 1}) RETURN count(n) $$) AS (c agtype); + c +---- + 50 +(1 row) + +SELECT * FROM cypher('csel', $$ MATCH (n:V {addr: {zip: 5}}) RETURN count(n) $$) AS (c agtype); + c +----- + 300 +(1 row) + +SELECT drop_graph('csel', true); +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to table csel._ag_label_vertex +drop cascades to table csel._ag_label_edge +drop cascades to table csel."V" +NOTICE: graph "csel" has been dropped + drop_graph +------------ + +(1 row) + +DROP FUNCTION csel_plan_rows(text, text); +-- +-- Upgrade-path assertion. -- --- We replay the shipped ALTER OPERATOR statements directly rather than running --- ALTER EXTENSION age UPDATE: the dev upgrade script targets the placeholder --- version "y.y.y" and is not a stable version-chain target inside the --- regression harness. The whole section runs in a transaction that is rolled --- back, so it observes the flip without permanently mutating the operator --- catalog (PostgreSQL DDL is transactional). +-- The checks above cover a FRESH install. Existing installs pick up the +-- bindings from the ALTER OPERATOR blocks shipped in the upgrade scripts and +-- replayed by "ALTER EXTENSION age UPDATE". We replay those statements +-- directly rather than running ALTER EXTENSION: the dev upgrade script targets +-- the placeholder version "y.y.y" and is not a stable version-chain target +-- inside the regression harness. The section runs in a transaction that is +-- rolled back, so the operator catalog is not permanently mutated. BEGIN; --- Simulate a stale (pre-fix) install: force all ten overloads back onto +-- Simulate a stale (pre-#2356) install: force all ten overloads back onto -- matchingsel / matchingjoinsel. ALTER OPERATOR ag_catalog.@>(agtype, agtype) SET (RESTRICT = matchingsel, JOIN = matchingjoinsel); ALTER OPERATOR ag_catalog.<@(agtype, agtype) SET (RESTRICT = matchingsel, JOIN = matchingjoinsel); @@ -206,7 +377,7 @@ ORDER BY o.oprname, lhs, rhs; @>> | agtype | agtype | matchingsel | matchingjoinsel (10 rows) --- Replay the exact ALTER OPERATOR block shipped in age--1.7.0--y.y.y.sql. +-- Replay the ALTER OPERATOR block shipped in age--1.7.0--1.8.0.sql (#2356). ALTER OPERATOR ag_catalog.@>(agtype, agtype) SET (RESTRICT = contsel, JOIN = contjoinsel); ALTER OPERATOR ag_catalog.<@(agtype, agtype) SET (RESTRICT = contsel, JOIN = contjoinsel); ALTER OPERATOR ag_catalog.@>>(agtype, agtype) SET (RESTRICT = contsel, JOIN = contjoinsel); @@ -217,7 +388,12 @@ ALTER OPERATOR ag_catalog.?|(agtype, text[]) SET (RESTRICT = contsel, JOIN = c ALTER OPERATOR ag_catalog.?|(agtype, agtype) SET (RESTRICT = contsel, JOIN = contjoinsel); ALTER OPERATOR ag_catalog.?&(agtype, text[]) SET (RESTRICT = contsel, JOIN = contjoinsel); ALTER OPERATOR ag_catalog.?&(agtype, agtype) SET (RESTRICT = contsel, JOIN = contjoinsel); --- After the upgrade replay every overload is back on contsel / contjoinsel. +-- Then the block shipped in age--1.8.0--y.y.y.sql: @> and @>> move to the +-- statistics-aware estimator; the JOIN estimator and all other operators +-- stay where #2356 put them. +ALTER OPERATOR ag_catalog.@> (agtype, agtype) SET (RESTRICT = ag_catalog.agtype_contains_sel); +ALTER OPERATOR ag_catalog.@>> (agtype, agtype) SET (RESTRICT = ag_catalog.agtype_contains_sel); +-- After the upgrade replay the bindings match a fresh install. SELECT o.oprname, pg_catalog.format_type(o.oprleft, NULL) AS lhs, pg_catalog.format_type(o.oprright, NULL) AS rhs, @@ -228,18 +404,18 @@ JOIN pg_catalog.pg_namespace n ON n.oid = o.oprnamespace WHERE n.nspname = 'ag_catalog' AND o.oprname IN ('@>', '<@', '@>>', '<<@', '?', '?|', '?&') ORDER BY o.oprname, lhs, rhs; - oprname | lhs | rhs | restrict_fn | join_fn ----------+--------+--------+-------------+------------- - <<@ | agtype | agtype | contsel | contjoinsel - <@ | agtype | agtype | contsel | contjoinsel - ? | agtype | agtype | contsel | contjoinsel - ? | agtype | text | contsel | contjoinsel - ?& | agtype | agtype | contsel | contjoinsel - ?& | agtype | text[] | contsel | contjoinsel - ?| | agtype | agtype | contsel | contjoinsel - ?| | agtype | text[] | contsel | contjoinsel - @> | agtype | agtype | contsel | contjoinsel - @>> | agtype | agtype | contsel | contjoinsel + oprname | lhs | rhs | restrict_fn | join_fn +---------+--------+--------+---------------------+------------- + <<@ | agtype | agtype | contsel | contjoinsel + <@ | agtype | agtype | contsel | contjoinsel + ? | agtype | agtype | contsel | contjoinsel + ? | agtype | text | contsel | contjoinsel + ?& | agtype | agtype | contsel | contjoinsel + ?& | agtype | text[] | contsel | contjoinsel + ?| | agtype | agtype | contsel | contjoinsel + ?| | agtype | text[] | contsel | contjoinsel + @> | agtype | agtype | agtype_contains_sel | contjoinsel + @>> | agtype | agtype | agtype_contains_sel | contjoinsel (10 rows) ROLLBACK; diff --git a/regress/sql/containment_selectivity.sql b/regress/sql/containment_selectivity.sql index c35ad3fc8..501ebdb25 100755 --- a/regress/sql/containment_selectivity.sql +++ b/regress/sql/containment_selectivity.sql @@ -18,18 +18,35 @@ */ /* - * Regression coverage for issue #2356: - * The containment (@>, <@, @>>, <<@) and key-existence (?, ?|, ?&) - * operators on agtype must be bound to the lightweight selectivity - * helpers contsel / contjoinsel during planning. Earlier PG14+ - * branches used matchingsel / matchingjoinsel, which caused planning - * to invoke agtype_contains() against pg_statistic MCVs and produced - * a 30%+ planning-time regression on point queries (severe TPS drop - * reported on the PG18 branch). + * Selectivity bindings for the agtype containment (@>, <@, @>>, <<@) and + * key-existence (?, ?|, ?&) operators, and behaviour of the statistics-aware + * estimator bound to @> and @>>. * - * This test pins the bindings by querying pg_operator directly. If - * someone re-introduces matchingsel here, the test diff is loud and - * precise. + * History: + * - Before #2356 all seven operators used matchingsel / matchingjoinsel. + * matchingsel probes the statistics of the whole properties column and + * calls agtype_contains() once per MCV / histogram entry at plan time, + * which produced a 30%+ planning-time regression on point queries. Those + * whole-column statistics also carry no information about any one key, + * so the estimate was wrong anyway (it bottoms out at PostgreSQL's 1e-4 + * floor). #2356 rebound everything to contsel / contjoinsel. + * - contsel returns a fixed 0.001. On a multi-hop MATCH the resulting + * overestimate of a selective start vertex pushes the planner from + * per-vertex index probes to a full scan of every edge table. @> and @>> + * are now bound to agtype_contains_sel, which decomposes the constant + * into per-key equalities on agtype_access_operator(properties, '"key"') + * and reads the expression statistics users attach to that expression. + * It never touches the properties column's statistics and never calls + * agtype_contains() at plan time, so the #2356 regression cannot recur. + * Without expression statistics it returns exactly what contsel did. + * + * This file pins the bindings via pg_operator so that a re-introduction of + * matchingsel is loud, and asserts the estimator's two contracts: + * 1. an inline property map and the equivalent WHERE clause estimate the + * same number of rows once expression statistics exist; + * 2. without expression statistics, or with + * age.enable_containment_statistics = off, the estimate is the one + * contsel produced. */ LOAD 'age'; @@ -60,12 +77,9 @@ WHERE n.nspname = 'ag_catalog' AND o.oprname IN ('?', '?|', '?&') ORDER BY o.oprname, lhs, rhs; --- Scoped guard for issue #2356: assert that none of the specific containment --- and key-existence operators on agtype are bound to matchingsel / --- matchingjoinsel. We deliberately limit the check to these operator names --- (rather than every operator in ag_catalog) so unrelated operators that --- legitimately use matchingsel for their own semantics are not affected by --- this regression test. +-- Scoped guard for issue #2356: none of these operators may be bound to +-- matchingsel / matchingjoinsel. The check is limited to these operator names +-- so unrelated operators that legitimately use matchingsel are not affected. SELECT COUNT(*) AS leaked_matchingsel_bindings FROM pg_catalog.pg_operator o JOIN pg_catalog.pg_namespace n ON n.oid = o.oprnamespace @@ -88,23 +102,118 @@ SELECT '{"a":1,"b":2}'::agtype ?| '["a","c"]'::agtype AS exists_any_a SELECT '{"a":1,"b":2}'::agtype ?& ARRAY['a','b'] AS exists_all_text_yes; SELECT '{"a":1,"b":2}'::agtype ?& '["a","b"]'::agtype AS exists_all_agtype_yes; --- Upgrade-path assertion for issue #2356. -- --- The checks above cover a FRESH install: contsel / contjoinsel come straight --- from agtype_operators.sql and agtype_exists.sql. Existing installs instead --- pick up the fix from the ALTER OPERATOR ... SET (RESTRICT, JOIN) block that --- age--1.7.0--y.y.y.sql ships and "ALTER EXTENSION age UPDATE" replays. Nothing --- above exercises that block, so a silent regression in it would go unnoticed. +-- Estimator behaviour. +-- +-- csel_plan_rows() returns the planner's row estimate for the scan of the +-- given relation inside EXPLAIN output. Comparing two estimates (rather than +-- printing them) keeps the expected output free of magic numbers that would +-- drift with statistics targets. +-- +CREATE FUNCTION csel_plan_rows(q text, rel text) RETURNS numeric +LANGUAGE plpgsql AS $fn$ +DECLARE + j json; +BEGIN + EXECUTE 'EXPLAIN (FORMAT JSON, COSTS ON) ' || q INTO j; + RETURN (pg_catalog.jsonb_path_query_first( + j::jsonb, + ('$.** ? (@."Relation Name" == "' || rel || '")."Plan Rows"')::jsonpath + ))::text::numeric; +END +$fn$; + +SELECT create_graph('csel'); +SELECT create_vlabel('csel', 'V'); + +-- 3000 vertices: uid unique, city one of 30, grp one of 4, addr.zip one of 10. +INSERT INTO csel."V" (id, properties) +SELECT ag_catalog._graphid((SELECT l.id FROM ag_catalog.ag_label l + JOIN ag_catalog.ag_graph g ON g.graphid = l.graph + WHERE g.name = 'csel' AND l.name = 'V'), i::bigint), + ('{"uid": ' || i || ', "city": "c' || (i % 30) || '", "grp": ' || + (i % 4) || ', "addr": {"zip": ' || (i % 10) || '}}')::agtype +FROM pg_catalog.generate_series(1, 3000) AS i; +ANALYZE csel."V"; + +-- Before any expression statistics exist, the inline map must estimate +-- exactly as it did under contsel: a fixed 0.001 of the table. +CREATE TEMP TABLE csel_baseline AS +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {uid: 7}) RETURN n $$) AS (n agtype)$q$, 'V') AS rows_no_stats; + +SELECT rows_no_stats = pg_catalog.round(3000 * 0.001) AS no_stats_matches_contsel +FROM csel_baseline; + +-- Attach expression statistics: an index on uid and city, a statistics object +-- (no index) on the nested addr.zip. grp deliberately gets none. +CREATE INDEX csel_v_uid_idx ON csel."V" (agtype_access_operator(properties, '"uid"'::agtype)); +CREATE INDEX csel_v_city_idx ON csel."V" (agtype_access_operator(properties, '"city"'::agtype)); +CREATE STATISTICS csel_v_zip_stat ON (agtype_access_operator(properties, '"addr"'::agtype, '"zip"'::agtype)) FROM csel."V"; +ANALYZE csel."V"; + +-- Contract 1: inline map == WHERE clause, per key shape. +-- unique key +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {uid: 7}) RETURN n $$) AS (n agtype)$q$, 'V') + = csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V) WHERE n.uid = 7 RETURN n $$) AS (n agtype)$q$, 'V') + AS unique_key_equal, + csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {uid: 7}) RETURN n $$) AS (n agtype)$q$, 'V') + AS unique_key_rows; + +-- low-cardinality key (MCV hit) +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {city: 'c3'}) RETURN n $$) AS (n agtype)$q$, 'V') + = csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V) WHERE n.city = 'c3' RETURN n $$) AS (n agtype)$q$, 'V') + AS lowcard_key_equal, + csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {city: 'c3'}) RETURN n $$) AS (n agtype)$q$, 'V') + AS lowcard_key_rows; + +-- two keys, one with statistics and one without +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {city: 'c3', grp: 1}) RETURN n $$) AS (n agtype)$q$, 'V') + = csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V) WHERE n.city = 'c3' AND n.grp = 1 RETURN n $$) AS (n agtype)$q$, 'V') + AS mixed_keys_equal; + +-- nested key backed by CREATE STATISTICS rather than an index +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {addr: {zip: 5}}) RETURN n $$) AS (n agtype)$q$, 'V') + = csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V) WHERE n.addr.zip = 5 RETURN n $$) AS (n agtype)$q$, 'V') + AS nested_key_equal; + +-- top-level containment form (=properties) on a scalar key +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V ={uid: 7}) RETURN n $$) AS (n agtype)$q$, 'V') + = csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V) WHERE n.uid = 7 RETURN n $$) AS (n agtype)$q$, 'V') + AS top_level_equal; + +-- a key that has no statistics at all still estimates as contsel did +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {grp: 1}) RETURN n $$) AS (n agtype)$q$, 'V') + = rows_no_stats AS unknown_key_matches_contsel +FROM csel_baseline; + +-- Contract 2: the GUC restores the contsel estimate. +SET age.enable_containment_statistics = off; +SELECT csel_plan_rows($q$SELECT * FROM cypher('csel', $$ MATCH (n:V {uid: 7}) RETURN n $$) AS (n agtype)$q$, 'V') + = rows_no_stats AS guc_off_matches_contsel +FROM csel_baseline; +RESET age.enable_containment_statistics; + +-- Results are unaffected by the estimator. +SELECT * FROM cypher('csel', $$ MATCH (n:V {uid: 7}) RETURN n.city $$) AS (city agtype); +SELECT * FROM cypher('csel', $$ MATCH (n:V {city: 'c3', grp: 1}) RETURN count(n) $$) AS (c agtype); +SELECT * FROM cypher('csel', $$ MATCH (n:V {addr: {zip: 5}}) RETURN count(n) $$) AS (c agtype); + +SELECT drop_graph('csel', true); +DROP FUNCTION csel_plan_rows(text, text); + +-- +-- Upgrade-path assertion. -- --- We replay the shipped ALTER OPERATOR statements directly rather than running --- ALTER EXTENSION age UPDATE: the dev upgrade script targets the placeholder --- version "y.y.y" and is not a stable version-chain target inside the --- regression harness. The whole section runs in a transaction that is rolled --- back, so it observes the flip without permanently mutating the operator --- catalog (PostgreSQL DDL is transactional). +-- The checks above cover a FRESH install. Existing installs pick up the +-- bindings from the ALTER OPERATOR blocks shipped in the upgrade scripts and +-- replayed by "ALTER EXTENSION age UPDATE". We replay those statements +-- directly rather than running ALTER EXTENSION: the dev upgrade script targets +-- the placeholder version "y.y.y" and is not a stable version-chain target +-- inside the regression harness. The section runs in a transaction that is +-- rolled back, so the operator catalog is not permanently mutated. BEGIN; --- Simulate a stale (pre-fix) install: force all ten overloads back onto +-- Simulate a stale (pre-#2356) install: force all ten overloads back onto -- matchingsel / matchingjoinsel. ALTER OPERATOR ag_catalog.@>(agtype, agtype) SET (RESTRICT = matchingsel, JOIN = matchingjoinsel); ALTER OPERATOR ag_catalog.<@(agtype, agtype) SET (RESTRICT = matchingsel, JOIN = matchingjoinsel); @@ -129,7 +238,7 @@ WHERE n.nspname = 'ag_catalog' AND o.oprname IN ('@>', '<@', '@>>', '<<@', '?', '?|', '?&') ORDER BY o.oprname, lhs, rhs; --- Replay the exact ALTER OPERATOR block shipped in age--1.7.0--y.y.y.sql. +-- Replay the ALTER OPERATOR block shipped in age--1.7.0--1.8.0.sql (#2356). ALTER OPERATOR ag_catalog.@>(agtype, agtype) SET (RESTRICT = contsel, JOIN = contjoinsel); ALTER OPERATOR ag_catalog.<@(agtype, agtype) SET (RESTRICT = contsel, JOIN = contjoinsel); ALTER OPERATOR ag_catalog.@>>(agtype, agtype) SET (RESTRICT = contsel, JOIN = contjoinsel); @@ -141,7 +250,13 @@ ALTER OPERATOR ag_catalog.?|(agtype, agtype) SET (RESTRICT = contsel, JOIN = c ALTER OPERATOR ag_catalog.?&(agtype, text[]) SET (RESTRICT = contsel, JOIN = contjoinsel); ALTER OPERATOR ag_catalog.?&(agtype, agtype) SET (RESTRICT = contsel, JOIN = contjoinsel); --- After the upgrade replay every overload is back on contsel / contjoinsel. +-- Then the block shipped in age--1.8.0--y.y.y.sql: @> and @>> move to the +-- statistics-aware estimator; the JOIN estimator and all other operators +-- stay where #2356 put them. +ALTER OPERATOR ag_catalog.@> (agtype, agtype) SET (RESTRICT = ag_catalog.agtype_contains_sel); +ALTER OPERATOR ag_catalog.@>> (agtype, agtype) SET (RESTRICT = ag_catalog.agtype_contains_sel); + +-- After the upgrade replay the bindings match a fresh install. SELECT o.oprname, pg_catalog.format_type(o.oprleft, NULL) AS lhs, pg_catalog.format_type(o.oprright, NULL) AS rhs, diff --git a/sql/agtype_operators.sql b/sql/agtype_operators.sql index 3fbc52f33..d56bc3908 100644 --- a/sql/agtype_operators.sql +++ b/sql/agtype_operators.sql @@ -28,12 +28,27 @@ RETURNS NULL ON NULL INPUT PARALLEL SAFE AS 'MODULE_PATHNAME'; +-- +-- Statistics-aware restriction selectivity for @> and @>>. Decomposes a +-- constant object into per-key equalities on agtype_access_operator() and +-- consults the expression statistics users attach to that expression; falls +-- back to the constant contsel returned when none exist. See +-- agtype_selfuncs.c. +-- +CREATE FUNCTION ag_catalog.agtype_contains_sel(internal, oid, internal, integer) + RETURNS float8 + LANGUAGE c + STABLE + STRICT + PARALLEL SAFE +AS 'MODULE_PATHNAME'; + CREATE OPERATOR @> ( LEFTARG = agtype, RIGHTARG = agtype, FUNCTION = ag_catalog.agtype_contains, COMMUTATOR = '<@', - RESTRICT = contsel, + RESTRICT = ag_catalog.agtype_contains_sel, JOIN = contjoinsel ); @@ -67,7 +82,7 @@ CREATE OPERATOR @>> ( RIGHTARG = agtype, FUNCTION = ag_catalog.agtype_contains_top_level, COMMUTATOR = '<<@', - RESTRICT = contsel, + RESTRICT = ag_catalog.agtype_contains_sel, JOIN = contjoinsel ); diff --git a/src/backend/utils/adt/agtype_selfuncs.c b/src/backend/utils/adt/agtype_selfuncs.c new file mode 100644 index 000000000..f713d7c26 --- /dev/null +++ b/src/backend/utils/adt/agtype_selfuncs.c @@ -0,0 +1,424 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/* + * Statistics-aware restriction selectivity for the agtype containment + * operators @> and @>>. + * + * A MATCH property constraint such as (n:Label {key: value}) is compiled to + * + * properties @> '{"key": value}' + * + * Neither stock estimator can see through that. contsel returns a fixed + * constant. matchingsel (the binding before #2356) consults the statistics + * of the whole properties column, which say nothing about the distribution + * of any one key, and it is expensive: it calls agtype_contains() once per + * MCV and histogram entry. Both give {person_id: } and + * {city: } the same estimate. On a multi-hop MATCH the resulting + * overestimate of the start vertex pushes the planner away from per-vertex + * index probes toward a full scan of every edge label table joined with a + * hash or merge join. + * + * This estimator decomposes the constant exactly the way the parser does + * when age.enable_containment = off (transform_map_to_ind_recursive for @>, + * transform_map_to_ind_top_level for @>>): one equality per leaf on + * + * agtype_access_operator(VARIADIC ARRAY[properties, '"key"', ...]) + * + * That is the expression users index or attach extended statistics to. + * examine_variable() finds those statistics by structural equality and + * var_eq_const() turns them into the selectivity an explicit + * WHERE n.key = value gets, so the two ways of writing the filter estimate + * identically. + * + * The properties column's own statistics are never read and + * agtype_contains() is never called at plan time. The only per-MCV work is + * inside var_eq_const() on the extracted key's short scalar values, which is + * what an equality on that key already costs. + * + * Fallback contract: when age.enable_containment_statistics is off, when + * the relation has neither an expression index nor extended statistics, + * when the operand is not a constant non-empty object, or when no leaf finds + * statistics, the result is AGTYPE_CONTAIN_DEFAULT_SEL, the value contsel + * returned. Installations without expression statistics see byte-identical + * plans. + */ + +#include "postgres.h" + +#include "catalog/namespace.h" +#include "catalog/pg_operator.h" +#include "catalog/pg_type.h" +#include "lib/stringinfo.h" +#include "miscadmin.h" +#include "nodes/makefuncs.h" +#include "nodes/nodeFuncs.h" +#include "nodes/pathnodes.h" +#include "nodes/pg_list.h" +#include "utils/elog.h" +#include "utils/lsyscache.h" +#include "utils/selfuncs.h" +#include "utils/syscache.h" + +#include "utils/ag_func.h" +#include "utils/ag_guc.h" +#include "utils/agtype.h" + +/* the constant contsel returns; see geo_selfuncs.c */ +#define AGTYPE_CONTAIN_DEFAULT_SEL 0.001 + +PG_FUNCTION_INFO_V1(agtype_contains_sel); + +typedef struct contains_sel_context +{ + PlannerInfo *root; + int varRelid; + Node *propvar; /* variable side of the operator */ + Oid eq_opoid; /* ag_catalog.=(agtype, agtype) */ + Oid access_fnoid; /* ag_catalog.agtype_access_operator(agtype[]) */ + bool top_level; /* @>> : do not descend into nested maps */ + int nleaves_with_stats; /* leaves for which statistics were found */ + double sel; +} contains_sel_context; + +static Oid cached_eq_opoid = InvalidOid; +static Oid cached_access_fnoid = InvalidOid; + +/* + * The operator and function oids are stable for the life of the extension, + * but the extension can be dropped and recreated within a backend, so the + * cached oid is revalidated against the syscache before use. + */ +static Oid get_agtype_eq_opoid(void) +{ + if (!OidIsValid(cached_eq_opoid) || + !SearchSysCacheExists1(OPEROID, ObjectIdGetDatum(cached_eq_opoid))) + { + cached_eq_opoid = OpernameGetOprid(list_make2(makeString("ag_catalog"), + makeString("=")), + AGTYPEOID, AGTYPEOID); + } + + return cached_eq_opoid; +} + +static Oid get_access_operator_fnoid(void) +{ + if (!OidIsValid(cached_access_fnoid) || + !SearchSysCacheExists1(PROCOID, ObjectIdGetDatum(cached_access_fnoid))) + { + cached_access_fnoid = get_ag_func_oid("agtype_access_operator", 1, + AGTYPEARRAYOID); + } + + return cached_access_fnoid; +} + +/* + * Cheap gate so that relations without any expression statistics never pay + * for node synthesis. Both lists are already in memory at this point. + */ +static bool rel_has_expression_statistics(RelOptInfo *rel) +{ + ListCell *lc; + + if (rel == NULL) + { + return false; + } + + if (rel->statlist != NIL) + { + return true; + } + + foreach(lc, rel->indexlist) + { + IndexOptInfo *index = (IndexOptInfo *) lfirst(lc); + + if (index->indexprs != NIL) + { + return true; + } + } + + return false; +} + +/* + * Build agtype_access_operator(VARIADIC ARRAY[, '"k1"', '"k2"', ...]) + * with the same node shape transform_A_Indirection produces, so that equal() + * matches an expression index or extended statistics object built on the + * documented CREATE INDEX ... (agtype_access_operator(properties, '"key"')) + * form. + */ +static Node *build_access_expr(contains_sel_context *ctx, List *keys) +{ + ArrayExpr *arr = makeNode(ArrayExpr); + FuncExpr *fexpr; + ListCell *lc; + + arr->elements = list_make1(copyObject(ctx->propvar)); + + foreach(lc, keys) + { + agtype_value *keyval = string_to_agtype_value((char *) lfirst(lc)); + agtype *keyagt = agtype_value_to_agtype(keyval); + Const *keyconst; + + keyconst = makeConst(AGTYPEOID, -1, InvalidOid, -1, + AGTYPE_P_GET_DATUM(keyagt), false, false); + arr->elements = lappend(arr->elements, keyconst); + } + + arr->element_typeid = AGTYPEOID; + arr->array_typeid = AGTYPEARRAYOID; + arr->multidims = false; + arr->location = -1; + + fexpr = makeFuncExpr(ctx->access_fnoid, AGTYPEOID, list_make1(arr), + InvalidOid, InvalidOid, COERCE_EXPLICIT_CALL); + fexpr->funcvariadic = true; + fexpr->location = -1; + + return (Node *) fexpr; +} + +static char *key_path_to_string(List *keys) +{ + StringInfoData buf; + ListCell *lc; + + initStringInfo(&buf); + + foreach(lc, keys) + { + if (buf.len > 0) + { + appendStringInfoChar(&buf, '.'); + } + appendStringInfoString(&buf, (char *) lfirst(lc)); + } + + return buf.data; +} + +/* + * One decomposed leaf: OP . + * + * Mirrors transform_map_to_ind_recursive / _top_level: in top-level mode + * every value is compared with =; in deep mode lists and (empty) maps are + * compared with @>, everything else with =. + */ +static void estimate_leaf(contains_sel_context *ctx, List *keys, + agtype_value *val) +{ + bool is_container = (val->type == AGTV_BINARY); + bool use_equality; + double s; + bool found = false; + + if (ctx->top_level) + { + use_equality = true; + } + else + { + use_equality = !is_container; + } + + if (use_equality) + { + VariableStatData vd; + Node *expr = build_access_expr(ctx, keys); + agtype *valagt = agtype_value_to_agtype(val); + + examine_variable(ctx->root, expr, ctx->varRelid, &vd); + + /* + * var_eq_const gives the same answer eqsel would for an explicit + * equality on this expression, including the 1/ndistinct fallback + * when no statistics exist. + */ + s = var_eq_const(&vd, ctx->eq_opoid, InvalidOid, + AGTYPE_P_GET_DATUM(valagt), false, true, false); + + found = HeapTupleIsValid(vd.statsTuple) || vd.isunique; + + ReleaseVariableStats(vd); + } + else + { + /* a nested containment: this is what contsel gave it */ + s = AGTYPE_CONTAIN_DEFAULT_SEL; + } + + if (found) + { + ctx->nleaves_with_stats++; + } + + if (message_level_is_interesting(DEBUG1)) + { + ereport(DEBUG1, + (errmsg_internal("agtype_contains_sel: key %s: %s, selectivity %g", + key_path_to_string(keys), + use_equality ? + (found ? "statistics found" : + "no statistics") : + "containment, default", + s))); + } + + ctx->sel *= s; +} + +/* + * Walk one object level of the containment constant, descending into + * non-empty nested objects when in deep (@>) mode. + */ +static void walk_object(contains_sel_context *ctx, agtype_container *agtc, + List *keys) +{ + agtype_iterator *it; + agtype_iterator_token tok; + agtype_value v; + char *key = NULL; + + check_stack_depth(); + + it = agtype_iterator_init(agtc); + + while ((tok = agtype_iterator_next(&it, &v, true)) != WAGT_DONE) + { + List *leaf_keys; + + if (tok == WAGT_KEY) + { + key = pnstrdup(v.val.string.val, v.val.string.len); + continue; + } + + if (tok != WAGT_VALUE || key == NULL) + { + continue; + } + + leaf_keys = lappend(list_copy(keys), key); + key = NULL; + + if (!ctx->top_level && + v.type == AGTV_BINARY && + AGTYPE_CONTAINER_IS_OBJECT(v.val.binary.data) && + AGTYPE_CONTAINER_SIZE(v.val.binary.data) > 0) + { + walk_object(ctx, v.val.binary.data, leaf_keys); + } + else + { + estimate_leaf(ctx, leaf_keys, &v); + } + } +} + +/* + * Restriction selectivity for agtype @> agtype and agtype @>> agtype. + * Signature: (internal, oid, internal, integer) -> float8. + */ +Datum agtype_contains_sel(PG_FUNCTION_ARGS) +{ + PlannerInfo *root = (PlannerInfo *) PG_GETARG_POINTER(0); + Oid operator = PG_GETARG_OID(1); + List *args = (List *) PG_GETARG_POINTER(2); + int varRelid = PG_GETARG_INT32(3); + VariableStatData vardata; + Node *other = NULL; + bool varonleft = false; + Const *cnst; + agtype *agt; + char *opname; + contains_sel_context ctx; + + if (!age_enable_containment_statistics || root == NULL) + { + PG_RETURN_FLOAT8(AGTYPE_CONTAIN_DEFAULT_SEL); + } + + if (!get_restriction_variable(root, args, varRelid, &vardata, &other, + &varonleft)) + { + PG_RETURN_FLOAT8(AGTYPE_CONTAIN_DEFAULT_SEL); + } + + /* need @> ; anything else keeps the old estimate */ + if (!varonleft || other == NULL || !IsA(other, Const) || + vardata.var == NULL || + !rel_has_expression_statistics(vardata.rel)) + { + ReleaseVariableStats(vardata); + PG_RETURN_FLOAT8(AGTYPE_CONTAIN_DEFAULT_SEL); + } + + cnst = (Const *) other; + + if (cnst->constisnull || cnst->consttype != AGTYPEOID) + { + ReleaseVariableStats(vardata); + PG_RETURN_FLOAT8(AGTYPE_CONTAIN_DEFAULT_SEL); + } + + agt = DATUM_GET_AGTYPE_P(cnst->constvalue); + + if (!AGT_ROOT_IS_OBJECT(agt) || AGT_ROOT_COUNT(agt) == 0) + { + ReleaseVariableStats(vardata); + PG_RETURN_FLOAT8(AGTYPE_CONTAIN_DEFAULT_SEL); + } + + ctx.root = root; + ctx.varRelid = varRelid; + ctx.propvar = vardata.var; + ctx.eq_opoid = get_agtype_eq_opoid(); + ctx.access_fnoid = get_access_operator_fnoid(); + ctx.nleaves_with_stats = 0; + ctx.sel = 1.0; + + opname = get_opname(operator); + ctx.top_level = (opname != NULL && strcmp(opname, "@>>") == 0); + + if (!OidIsValid(ctx.eq_opoid) || !OidIsValid(ctx.access_fnoid)) + { + ReleaseVariableStats(vardata); + PG_RETURN_FLOAT8(AGTYPE_CONTAIN_DEFAULT_SEL); + } + + walk_object(&ctx, &agt->root, NIL); + + ReleaseVariableStats(vardata); + + /* no leaf had statistics: keep the estimate contsel produced */ + if (ctx.nleaves_with_stats == 0) + { + PG_RETURN_FLOAT8(AGTYPE_CONTAIN_DEFAULT_SEL); + } + + CLAMP_PROBABILITY(ctx.sel); + + PG_RETURN_FLOAT8(ctx.sel); +} diff --git a/src/backend/utils/ag_guc.c b/src/backend/utils/ag_guc.c index 86b4e00bc..466abec55 100644 --- a/src/backend/utils/ag_guc.c +++ b/src/backend/utils/ag_guc.c @@ -23,6 +23,7 @@ #include "utils/ag_guc.h" bool age_enable_containment = true; +bool age_enable_containment_statistics = true; /* * Defines AGE's custom configuration parameters. @@ -42,5 +43,15 @@ void define_config_params(void) NULL, NULL, NULL); + DefineCustomBoolVariable("age.enable_containment_statistics", + "Consult expression statistics when estimating the selectivity of agtype containment (@>, @>>). When off, a fixed selectivity is used.", + NULL, + &age_enable_containment_statistics, + true, + PGC_USERSET, + 0, + NULL, + NULL, + NULL); EmitWarningsOnPlaceholders("age"); } diff --git a/src/include/utils/ag_guc.h b/src/include/utils/ag_guc.h index 52fab2b85..e399c631a 100644 --- a/src/include/utils/ag_guc.h +++ b/src/include/utils/ag_guc.h @@ -38,6 +38,7 @@ * expression index. */ extern bool age_enable_containment; +extern bool age_enable_containment_statistics; void define_config_params(void); From 6b9890f4bac01927fb1db974adbd89df42390405 Mon Sep 17 00:00:00 2001 From: himmel Date: Tue, 8 Sep 2026 02:17:30 +0000 Subject: [PATCH 2/3] Add age.max_global_graph_memory to bound the global graph cache manage_GRAPH_global_contexts() builds a full in-memory copy of every vertex and edge of a graph, once per backend, and nothing bounded it. On a 3M-vertex graph with three 3M-row edge tables the copy is about 1.4 GB of private memory per backend, so a handful of concurrent traversals is enough to have the OOM killer take a backend down, which on a busy instance means losing other sessions' work and possibly a crash recovery cycle. The cache is also not shared between backends, so every session pays the full cost again. This adds a ceiling. Exceeding it can only be an error, because every traversal consumer requires the cache and there is no uncached path to fall back to, but turning "the OOM killer takes down the backend" into "this query fails with an actionable message" is what an operator needs first. age.max_global_graph_memory is a per-backend total across every cached graph, in the spirit of temp_file_limit, and follows it in the rest of its definition too: kB units, PGC_SUSET so that a DBA sets the ceiling and users can only lower it, and -1 (unlimited) by default so that no existing workload changes behavior on upgrade. The limit is enforced in two places. Before loading, the size is estimated from pg_class.reltuples so that a load which cannot possibly fit fails in milliseconds rather than after a full scan; on the 3M-vertex graph above that is 2 ms instead of 25 s. The estimate uses a bytes-per-element constant chosen below the whole measured range (measured: 105 bytes per element at 500k vertices and 1.5M edges, 123 at 3M/9M, 159 at 500k/500k), so it can under-estimate but never rejects a graph that would have fit. During the load the limit is then enforced against what has actually been allocated, every few thousand entries so that walking the context's block list is not a per-tuple cost, and once more when the load finishes, which is what catches a graph with fewer elements than the check interval. The error carries ERRCODE_CONFIGURATION_LIMIT_EXCEEDED and reports the three terms that matter - what the load needs, what the backend's other cached graphs hold, and the limit - because an oversized graph and an accumulation of cached graphs have different remedies. Two changes were needed to make that possible. Each context now owns a private MemoryContext tree: ggctx_mcxt holds the GRAPH_global_context struct and the graph name, with vertex_mcxt and the existing edge_table_mcxt as children. The vertex hashtable is created with HASH_CONTEXT, and the load runs with vertex_mcxt current so that the per-vertex VertexEdgeArray allocations and the vertices list land there too instead of in TopMemoryContext. Accounting is then whatever the allocator already tracks for that tree, with no separate byte counting, and free_specific_GRAPH_global_context collapses from a manual walk of the vertices list to a single MemoryContextDelete. That walk could return false on a vertex missing from the hashtable, which callers turned into a "missing vertex or edge entry during free" error; it can no longer fail. The new context is also no longer linked into global_graph_contexts until the build has finished. Previously it was linked first, so an ERROR during the load - a malformed label table, and now the memory limit - left a partially built context on the list. A failed load makes no change that would bump the graph's version counter, so is_ggctx_invalid() would accept that partial context on the next call and queries would silently see a graph missing vertices or edges. Deferring the link means the context is unreachable while it is built, so the build is wrapped in PG_TRY and the error path deletes the private tree, which was verified to hold RssAnon flat across repeated failing loads. Not included, deliberately: evicting other cached graphs to make room before failing. VLE holds a GRAPH_global_context pointer and a raw GraphIdNode across SRF calls, and a single statement can traverse two graphs, so releasing a valid context that another scan is walking needs a refcount first. Narrowing the load to the labels and direction a query actually uses is also left out; it changes what a cached context contains and so needs cache-key work of its own. 43/43 installcheck on PostgreSQL 18.4. --- regress/expected/age_global_graph.out | 148 ++++++++ regress/sql/age_global_graph.sql | 86 +++++ src/backend/utils/adt/age_global_graph.c | 461 ++++++++++++++++++----- src/backend/utils/ag_guc.c | 14 + src/include/utils/ag_guc.h | 13 + 5 files changed, 633 insertions(+), 89 deletions(-) diff --git a/regress/expected/age_global_graph.out b/regress/expected/age_global_graph.out index 4833511a7..e3bea26d9 100644 --- a/regress/expected/age_global_graph.out +++ b/regress/expected/age_global_graph.out @@ -714,6 +714,154 @@ NOTICE: graph "vle_trigger_test" has been dropped (1 row) +----------------------------------------------------------------------------------------------------------------------------- +-- +-- age.max_global_graph_memory +-- +-- The global graph cache is a full in-memory copy of a graph's adjacency, +-- built once per backend. Before this GUC existed nothing bounded it, so a +-- large enough graph grew the backend until the OOM killer took it down. +-- Exceeding the limit can only be an error: every traversal consumer needs +-- the cache, so there is no uncached path to fall back to. +-- +-- These tests assert the observable contract only. They deliberately do not +-- check byte counts: the amount a graph occupies depends on the allocator +-- and on where the element count falls relative to the hashtables' +-- power-of-two sizing. +-- +----------------------------------------------------------------------------------------------------------------------------- +SELECT * FROM create_graph('ggm'); +NOTICE: graph "ggm" has been created + create_graph +-------------- + +(1 row) + +SELECT * FROM cypher('ggm', $$ + CREATE (a:V {n: 'a'})-[:E]->(b:V {n: 'b'})-[:E]->(c:V {n: 'c'}) +$$) AS (v agtype); + v +--- +(0 rows) + +ANALYZE ggm."V"; +ANALYZE ggm."E"; +-- the default is unlimited, and is a superuser-only kB setting +SELECT setting, unit, vartype, context +FROM pg_settings WHERE name = 'age.max_global_graph_memory'; + setting | unit | vartype | context +---------+------+---------+----------- + -1 | kB | integer | superuser +(1 row) + +-- unlimited: the traversal works +SELECT * FROM cypher('ggm', $$ + MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n ORDER BY x.n +$$) AS (n agtype); + n +----- + "b" + "c" +(2 rows) + +-- Start from an empty cache so that the limit applies to this graph alone, +-- and report errors terse: the detail line carries sizes that depend on the +-- allocator and on what else the backend has cached. +SELECT ag_catalog.age_delete_global_graphs(NULL); + age_delete_global_graphs +-------------------------- + t +(1 row) + +\set VERBOSITY terse +-- a limit no graph can satisfy: the load is refused, not the backend +SET age.max_global_graph_memory = '1kB'; +SELECT * FROM cypher('ggm', $$ + MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n ORDER BY x.n +$$) AS (n agtype); +ERROR: global graph cache for graph "ggm" would exceed age.max_global_graph_memory +-- the error is reported as a configuration limit, so an application can +-- recognize it by SQLSTATE rather than by message text +DO $$ +BEGIN + PERFORM * FROM cypher('ggm', $q$ + MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n + $q$) AS (n agtype); + RAISE NOTICE 'no error raised'; +EXCEPTION WHEN configuration_limit_exceeded THEN + RAISE NOTICE 'configuration_limit_exceeded'; +END +$$; +NOTICE: configuration_limit_exceeded +-- a refused load leaves nothing cached: raising the limit in the same +-- session must produce a complete graph, not a partially loaded one +RESET age.max_global_graph_memory; +SELECT * FROM cypher('ggm', $$ + MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n ORDER BY x.n +$$) AS (n agtype); + n +----- + "b" + "c" +(2 rows) + +-- the limit is a per-backend total across every cached graph, so a second +-- graph is refused once the first has consumed the budget +SELECT * FROM create_graph('ggm2'); +NOTICE: graph "ggm2" has been created + create_graph +-------------- + +(1 row) + +SELECT * FROM cypher('ggm2', $$ CREATE (a:V {n: 'a'})-[:E]->(b:V {n: 'b'}) $$) AS (v agtype); + v +--- +(0 rows) + +ANALYZE ggm2."V"; +ANALYZE ggm2."E"; +SET age.max_global_graph_memory = '1kB'; +SELECT * FROM cypher('ggm2', $$ + MATCH (a:V {n: 'a'})-[:E*1..1]->(x) RETURN x.n +$$) AS (n agtype); +ERROR: global graph cache for graph "ggm2" would exceed age.max_global_graph_memory +-- and the graph that was already cached is still usable +SELECT * FROM cypher('ggm', $$ + MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n ORDER BY x.n +$$) AS (n agtype); + n +----- + "b" + "c" +(2 rows) + +RESET age.max_global_graph_memory; +\set VERBOSITY default +SELECT * FROM drop_graph('ggm', true); +NOTICE: drop cascades to 4 other objects +DETAIL: drop cascades to table ggm._ag_label_vertex +drop cascades to table ggm._ag_label_edge +drop cascades to table ggm."V" +drop cascades to table ggm."E" +NOTICE: graph "ggm" has been dropped + drop_graph +------------ + +(1 row) + +SELECT * FROM drop_graph('ggm2', true); +NOTICE: drop cascades to 4 other objects +DETAIL: drop cascades to table ggm2._ag_label_vertex +drop cascades to table ggm2._ag_label_edge +drop cascades to table ggm2."V" +drop cascades to table ggm2."E" +NOTICE: graph "ggm2" has been dropped + drop_graph +------------ + +(1 row) + ----------------------------------------------------------------------------------------------------------------------------- -- -- End of tests diff --git a/regress/sql/age_global_graph.sql b/regress/sql/age_global_graph.sql index 9f4a1ce2d..04ddc4a87 100644 --- a/regress/sql/age_global_graph.sql +++ b/regress/sql/age_global_graph.sql @@ -335,6 +335,92 @@ $$) AS (name agtype); -- Cleanup SELECT * FROM drop_graph('vle_trigger_test', true); +----------------------------------------------------------------------------------------------------------------------------- +-- +-- age.max_global_graph_memory +-- +-- The global graph cache is a full in-memory copy of a graph's adjacency, +-- built once per backend. Before this GUC existed nothing bounded it, so a +-- large enough graph grew the backend until the OOM killer took it down. +-- Exceeding the limit can only be an error: every traversal consumer needs +-- the cache, so there is no uncached path to fall back to. +-- +-- These tests assert the observable contract only. They deliberately do not +-- check byte counts: the amount a graph occupies depends on the allocator +-- and on where the element count falls relative to the hashtables' +-- power-of-two sizing. +-- +----------------------------------------------------------------------------------------------------------------------------- + +SELECT * FROM create_graph('ggm'); +SELECT * FROM cypher('ggm', $$ + CREATE (a:V {n: 'a'})-[:E]->(b:V {n: 'b'})-[:E]->(c:V {n: 'c'}) +$$) AS (v agtype); +ANALYZE ggm."V"; +ANALYZE ggm."E"; + +-- the default is unlimited, and is a superuser-only kB setting +SELECT setting, unit, vartype, context +FROM pg_settings WHERE name = 'age.max_global_graph_memory'; + +-- unlimited: the traversal works +SELECT * FROM cypher('ggm', $$ + MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n ORDER BY x.n +$$) AS (n agtype); + +-- Start from an empty cache so that the limit applies to this graph alone, +-- and report errors terse: the detail line carries sizes that depend on the +-- allocator and on what else the backend has cached. +SELECT ag_catalog.age_delete_global_graphs(NULL); +\set VERBOSITY terse + +-- a limit no graph can satisfy: the load is refused, not the backend +SET age.max_global_graph_memory = '1kB'; +SELECT * FROM cypher('ggm', $$ + MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n ORDER BY x.n +$$) AS (n agtype); + +-- the error is reported as a configuration limit, so an application can +-- recognize it by SQLSTATE rather than by message text +DO $$ +BEGIN + PERFORM * FROM cypher('ggm', $q$ + MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n + $q$) AS (n agtype); + RAISE NOTICE 'no error raised'; +EXCEPTION WHEN configuration_limit_exceeded THEN + RAISE NOTICE 'configuration_limit_exceeded'; +END +$$; + +-- a refused load leaves nothing cached: raising the limit in the same +-- session must produce a complete graph, not a partially loaded one +RESET age.max_global_graph_memory; +SELECT * FROM cypher('ggm', $$ + MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n ORDER BY x.n +$$) AS (n agtype); + +-- the limit is a per-backend total across every cached graph, so a second +-- graph is refused once the first has consumed the budget +SELECT * FROM create_graph('ggm2'); +SELECT * FROM cypher('ggm2', $$ CREATE (a:V {n: 'a'})-[:E]->(b:V {n: 'b'}) $$) AS (v agtype); +ANALYZE ggm2."V"; +ANALYZE ggm2."E"; +SET age.max_global_graph_memory = '1kB'; +SELECT * FROM cypher('ggm2', $$ + MATCH (a:V {n: 'a'})-[:E*1..1]->(x) RETURN x.n +$$) AS (n agtype); + +-- and the graph that was already cached is still usable +SELECT * FROM cypher('ggm', $$ + MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n ORDER BY x.n +$$) AS (n agtype); +RESET age.max_global_graph_memory; +\set VERBOSITY default + +SELECT * FROM drop_graph('ggm', true); +SELECT * FROM drop_graph('ggm2', true); + ----------------------------------------------------------------------------------------------------------------------------- -- -- End of tests diff --git a/src/backend/utils/adt/age_global_graph.c b/src/backend/utils/adt/age_global_graph.c index 397e0511c..ca8ecd3c9 100644 --- a/src/backend/utils/adt/age_global_graph.c +++ b/src/backend/utils/adt/age_global_graph.c @@ -30,6 +30,7 @@ #include "utils/lsyscache.h" #include "utils/memutils.h" #include "utils/snapmgr.h" +#include "utils/syscache.h" #include "utils/builtins.h" #if PG_VERSION_NUM >= 170000 @@ -39,6 +40,7 @@ #include "storage/shmem.h" #endif +#include "utils/ag_guc.h" #include "utils/age_global_graph.h" #include "utils/agehash.h" #include "catalog/ag_graph.h" @@ -51,6 +53,34 @@ #define VERTEX_HTAB_INITIAL_SIZE 10000 #define EDGE_HTAB_INITIAL_SIZE 10000 +/* + * How many vertex or edge insertions to make between memory-limit checks + * during a load. MemoryContextMemAllocated walks the context's block list, + * so checking per tuple would be a measurable cost on a multi-million + * element graph; checking every few thousand bounds the overshoot to a few + * thousand entries' worth of memory, which is negligible against any useful + * setting of age.max_global_graph_memory. + */ +#define GGCTX_MEMCHECK_INTERVAL 8192 + +/* + * Deliberately low bytes-per-element figure for the pre-load estimate only. + * + * Measured cost, as MemoryContextMemAllocated reports it, is 105 bytes per + * element on a 500k-vertex/1.5M-edge graph, 123 on 3M/9M and 159 on + * 500k/500k. The spread comes from the power-of-two sizing of the two + * hashtables and from the fixed cost of the first edge at each vertex, so it + * depends on a graph's shape rather than on anything predictable from the row + * counts alone. + * + * This constant sits below that whole range on purpose. The pre-load check + * exists only so that a load which cannot possibly fit fails in + * milliseconds instead of after a full scan, so it must never reject a graph + * that would have fit; under-estimating just means the in-load check, which + * is authoritative, does the rejecting instead. + */ +#define GGCTX_PRECHECK_BYTES_PER_ELEMENT 64 + /* Maximum number of graphs tracked for version counting */ #define AGE_MAX_GRAPHS 128 @@ -140,7 +170,10 @@ typedef struct GRAPH_global_context Oid graph_oid; /* graph oid for searching */ HTAB *vertex_hashtable; /* hashtable to hold vertex edge lists */ AgeHashTable *edge_table; /* edge to vertex map (Robin Hood) */ - MemoryContext edge_table_mcxt; /* private context owning edge_table */ + MemoryContext ggctx_mcxt; /* parent context owning this whole context */ + MemoryContext vertex_mcxt; /* child context owning the vertex side */ + MemoryContext edge_table_mcxt; /* child context owning edge_table */ + int64 memcheck_counter; /* insertions since the last memory check */ uint64 graph_version; /* version counter for cache invalidation */ TransactionId xmin; /* snapshot fallback: transaction xmin */ TransactionId xmax; /* snapshot fallback: transaction xmax */ @@ -208,6 +241,14 @@ static void load_GRAPH_global_hashtables(GRAPH_global_context *ggctx); static void load_vertex_hashtable(GRAPH_global_context *ggctx); static void load_edge_hashtable(GRAPH_global_context *ggctx); static void freeze_GRAPH_global_hashtables(GRAPH_global_context *ggctx); +static Size ggctx_memory_used(GRAPH_global_context *ggctx); +static Size cached_graphs_memory_used(GRAPH_global_context *except); +static Size ggctx_memory_limit_bytes(void); +static void ggctx_memory_limit_error(const char *graph_name, Size required, + Size other_cached, Size limit); +static void precheck_graph_memory_limit(Oid graph_oid, char *graph_name); +static void enforce_ggctx_memory_limit(GRAPH_global_context *ggctx); +static void check_ggctx_memory_limit(GRAPH_global_context *ggctx); static List *get_ag_labels_names(Snapshot snapshot, Oid graph_oid, char label_type); static bool insert_edge_entry(GRAPH_global_context *ggctx, graphid edge_id, @@ -340,27 +381,37 @@ static void create_GRAPH_global_hashtables(GRAPH_global_context *ggctx) strcpy(vhn, VERTEX_HTAB_NAME); vhn = strncat(vhn, graph_name, glen); - /* initialize the vertex hashtable */ + /* + * Initialize the vertex hashtable inside vertex_mcxt. + * + * HASH_CONTEXT puts the HTAB header, its directory and all of its + * entries in that context; load_GRAPH_global_hashtables additionally + * runs the load with vertex_mcxt current, so the per-vertex + * VertexEdgeArray allocations and the vertices list land there too. + * The whole vertex side is then reclaimed by one MemoryContextDelete. + */ + ggctx->vertex_mcxt = AllocSetContextCreate(ggctx->ggctx_mcxt, + "AGE vertex_hashtable", + ALLOCSET_DEFAULT_SIZES); + MemSet(&vertex_ctl, 0, sizeof(vertex_ctl)); vertex_ctl.keysize = sizeof(int64); vertex_ctl.entrysize = sizeof(vertex_entry); vertex_ctl.hash = graphid_hash; + vertex_ctl.hcxt = ggctx->vertex_mcxt; ggctx->vertex_hashtable = hash_create(vhn, VERTEX_HTAB_INITIAL_SIZE, &vertex_ctl, - HASH_ELEM | HASH_FUNCTION); + HASH_ELEM | HASH_FUNCTION | + HASH_CONTEXT); pfree_if_not_null(vhn); /* - * Initialize the edge_table (agehash, INLINE mode). - * - * Owns its own MemoryContext as a child of CurrentMemoryContext (which, - * at the call site, is TopMemoryContext for the lifetime of the cached - * GRAPH_global_context). Cleanup is a single MemoryContextDelete in - * free_specific_GRAPH_global_context, so an elog during build cannot - * leak slots. + * Initialize the edge_table (agehash, INLINE mode) inside its own child + * context. Cleanup for both sides is a single MemoryContextDelete of + * ggctx_mcxt in free_specific_GRAPH_global_context. */ ggctx->edge_table_mcxt = - AllocSetContextCreate(CurrentMemoryContext, + AllocSetContextCreate(ggctx->ggctx_mcxt, "AGE edge_table", ALLOCSET_DEFAULT_SIZES); ggctx->edge_table = agehash_create_inline(ggctx->edge_table_mcxt, @@ -559,6 +610,9 @@ static bool insert_edge_entry(GRAPH_global_context *ggctx, graphid edge_id, /* increment the number of loaded edges */ ggctx->num_loaded_edges++; + /* fail the load rather than the backend if we are over the limit */ + check_ggctx_memory_limit(ggctx); + return true; } @@ -623,6 +677,9 @@ static bool insert_vertex_entry(GRAPH_global_context *ggctx, graphid vertex_id, /* increment the number of loaded vertices */ ggctx->num_loaded_vertices++; + /* fail the load rather than the backend if we are over the limit */ + check_ggctx_memory_limit(ggctx); + return true; } @@ -711,6 +768,224 @@ static bool insert_vertex_edge(GRAPH_global_context *ggctx, return false; } + +/* + * age.max_global_graph_memory support. + * + * Every cached GRAPH_global_context owns a private MemoryContext tree + * (ggctx_mcxt, with vertex_mcxt and edge_table_mcxt as children), so the + * memory a context holds is exactly what the allocator already tracks for + * that tree and no separate byte counting is needed. The limit is a + * per-backend total across every cached graph, in the spirit of + * temp_file_limit. + * + * Note that exceeding the limit can only be an error: every traversal + * consumer requires the cache, so there is no uncached path to fall back to. + * Turning "the OOM killer takes down the backend" into "this query fails" is + * the whole point. + */ + +/* bytes this context's tree currently holds */ +static Size ggctx_memory_used(GRAPH_global_context *ggctx) +{ + if (ggctx == NULL || ggctx->ggctx_mcxt == NULL) + { + return 0; + } + + return MemoryContextMemAllocated(ggctx->ggctx_mcxt, true); +} + +/* bytes every already-cached context holds, excluding the one being built */ +static Size cached_graphs_memory_used(GRAPH_global_context *except) +{ + GRAPH_global_context *curr = NULL; + Size total = 0; + + for (curr = global_graph_contexts; curr != NULL; curr = curr->next) + { + if (curr == except) + { + continue; + } + + total += ggctx_memory_used(curr); + } + + return total; +} + +/* the configured limit in bytes, or 0 when unlimited */ +static Size ggctx_memory_limit_bytes(void) +{ + if (age_max_global_graph_memory < 0) + { + return 0; + } + + return (Size) age_max_global_graph_memory * 1024; +} + +/* + * Raise the limit-exceeded error. + * + * What matters is the backend total, so the detail line breaks it into the + * three terms: what this load needs, what the backend's other cached graphs + * already hold, and the limit. That lets the reader tell an oversized graph + * from an accumulation of cached graphs, which have different remedies. + * Sizes are rounded up so that a small graph does not report 0 kB. + */ +static void ggctx_memory_limit_error(const char *graph_name, Size required, + Size other_cached, Size limit) +{ + ereport(ERROR, + (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED), + errmsg("global graph cache for graph \"%s\" would exceed age.max_global_graph_memory", + graph_name), + errdetail("This load needs about %zu kB, other graphs cached by this backend hold %zu kB, and the limit is %d kB.", + (required + 1023) / 1024, (other_cached + 1023) / 1024, + age_max_global_graph_memory), + errhint("Raise age.max_global_graph_memory, release cached graphs with age_delete_global_graphs(), or reduce the size of the graph traversed."))); +} + +/* + * Estimate the cache size for a graph from pg_class.reltuples and fail before + * loading if even the conservative estimate does not fit. This is only to + * avoid spending a full load - tens of seconds and gigabytes on a large + * graph - before failing; it uses GGCTX_PRECHECK_BYTES_PER_ELEMENT, which is + * below the whole measured range, so it never rejects a graph that would have + * fit. Tables that have never been analyzed (reltuples < 0) contribute + * nothing and are left to the in-load check. + */ +static void precheck_graph_memory_limit(Oid graph_oid, char *graph_name) +{ + Snapshot snapshot; + List *label_names = NIL; + ListCell *lc; + Oid graph_namespace_oid; + MemoryContext tmpctx; + MemoryContext oldctx; + Size limit; + Size other_cached; + double elements = 0.0; + Size required; + int i; + + limit = ggctx_memory_limit_bytes(); + + if (limit == 0) + { + return; + } + + /* + * The caller runs in TopMemoryContext, so the label name lists built + * below need a context of their own to be reclaimed from. + */ + tmpctx = AllocSetContextCreate(CurrentMemoryContext, + "AGE graph memory precheck", + ALLOCSET_SMALL_SIZES); + oldctx = MemoryContextSwitchTo(tmpctx); + + graph_namespace_oid = get_namespace_oid(graph_name, false); + snapshot = GetActiveSnapshot(); + + for (i = 0; i < 2; i++) + { + label_names = get_ag_labels_names(snapshot, graph_oid, + (i == 0) ? LABEL_TYPE_VERTEX + : LABEL_TYPE_EDGE); + + foreach (lc, label_names) + { + Oid relid; + HeapTuple tuple; + Form_pg_class reltup; + + relid = get_relname_relid((char *) lfirst(lc), + graph_namespace_oid); + + if (!OidIsValid(relid)) + { + continue; + } + + tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); + + if (!HeapTupleIsValid(tuple)) + { + continue; + } + + reltup = (Form_pg_class) GETSTRUCT(tuple); + + if (reltup->reltuples > 0) + { + elements += reltup->reltuples; + } + + ReleaseSysCache(tuple); + } + } + + MemoryContextSwitchTo(oldctx); + MemoryContextDelete(tmpctx); + + required = (Size) (elements * GGCTX_PRECHECK_BYTES_PER_ELEMENT); + other_cached = cached_graphs_memory_used(NULL); + + if (other_cached + required > limit) + { + ggctx_memory_limit_error(graph_name, required, other_cached, limit); + } +} + +/* + * Called from the insert paths every GGCTX_MEMCHECK_INTERVAL entries. This is + * the authoritative check: it measures what the load has actually allocated + * rather than estimating it. + */ +static void enforce_ggctx_memory_limit(GRAPH_global_context *ggctx) +{ + Size limit; + Size used; + Size other_cached; + + limit = ggctx_memory_limit_bytes(); + + if (limit == 0) + { + return; + } + + used = ggctx_memory_used(ggctx); + other_cached = cached_graphs_memory_used(ggctx); + + if (other_cached + used > limit) + { + ggctx_memory_limit_error(ggctx->graph_name, used, other_cached, limit); + } +} + +/* + * Called from the insert paths. Enforces the limit every + * GGCTX_MEMCHECK_INTERVAL entries so that a large load is stopped partway + * instead of running to completion; load_GRAPH_global_hashtables enforces it + * once more at the end, which is what catches a graph small enough never to + * reach the interval. + */ +static void check_ggctx_memory_limit(GRAPH_global_context *ggctx) +{ + if (++ggctx->memcheck_counter < GGCTX_MEMCHECK_INTERVAL) + { + return; + } + + ggctx->memcheck_counter = 0; + + enforce_ggctx_memory_limit(ggctx); +} + /* helper routine to load all vertices into the GRAPH global vertex hashtable */ static void load_vertex_hashtable(GRAPH_global_context *ggctx) { @@ -799,15 +1074,36 @@ static void load_vertex_hashtable(GRAPH_global_context *ggctx) */ static void load_GRAPH_global_hashtables(GRAPH_global_context *ggctx) { + MemoryContext oldctx; + /* initialize statistics */ ggctx->num_loaded_vertices = 0; ggctx->num_loaded_edges = 0; + ggctx->memcheck_counter = 0; + + /* + * Run the load with vertex_mcxt current so that the per-vertex + * VertexEdgeArray allocations made by vea_append and the vertices list + * built by append_graphid are owned by the vertex side's context rather + * than by TopMemoryContext. The edge_table allocates in its own context + * regardless of the current one. + */ + oldctx = MemoryContextSwitchTo(ggctx->vertex_mcxt); /* insert all of our vertices */ load_vertex_hashtable(ggctx); /* insert all of our edges */ load_edge_hashtable(ggctx); + + MemoryContextSwitchTo(oldctx); + + /* + * Enforce the limit on the finished context. The periodic check during + * the load only fires every GGCTX_MEMCHECK_INTERVAL entries, so a graph + * with fewer elements than that would otherwise never be checked. + */ + enforce_ggctx_memory_limit(ggctx); } /* @@ -937,78 +1233,29 @@ static void freeze_GRAPH_global_hashtables(GRAPH_global_context *ggctx) */ static bool free_specific_GRAPH_global_context(GRAPH_global_context *ggctx) { - GraphIdNode *curr_vertex = NULL; - /* don't do anything if NULL */ if (ggctx == NULL) { return true; } - /* free the graph name */ - pfree_if_not_null(ggctx->graph_name); - ggctx->graph_name = NULL; - - ggctx->graph_oid = InvalidOid; - ggctx->next = NULL; - - /* free the vertex edge lists and properties, starting with the head */ - curr_vertex = peek_stack_head(ggctx->vertices); - while (curr_vertex != NULL) - { - GraphIdNode *next_vertex = NULL; - vertex_entry *value = NULL; - bool found = false; - graphid vertex_id; - - /* get the next vertex in the list, if any */ - next_vertex = next_GraphIdNode(curr_vertex); - - /* get the current vertex id */ - vertex_id = get_graphid(curr_vertex); - - /* retrieve the vertex entry */ - value = (vertex_entry *)hash_search(ggctx->vertex_hashtable, - (void *)&vertex_id, HASH_FIND, - &found); - /* this is bad if it isn't found, but leave that to the caller */ - if (found == false) - { - return false; - } - - /* free the edge arrays associated with this vertex */ - vea_free(&value->edges_in); - vea_free(&value->edges_out); - vea_free(&value->edges_self); - - /* move to the next vertex */ - curr_vertex = next_vertex; - } - - /* free the vertices list */ - free_ListGraphId(ggctx->vertices); - ggctx->vertices = NULL; - - /* free the hashtables */ - hash_destroy(ggctx->vertex_hashtable); /* - * The edge_table and all of its slots live entirely inside - * edge_table_mcxt, so a single MemoryContextDelete reclaims them. + * Everything this context owns - the GRAPH_global_context struct itself, + * the graph name, the vertex hashtable with its entries and per-vertex + * VertexEdgeArray allocations, the vertices list, and the edge_table with + * all of its slots - lives inside ggctx_mcxt or one of its children, so a + * single delete reclaims all of it. Nothing may be read from ggctx after + * this point. + * + * hash_destroy is deliberately not called: it would pfree entries that + * the context delete reclaims anyway, and its own context handling + * assumes it owns the HTAB's context. */ - if (ggctx->edge_table_mcxt != NULL) + if (ggctx->ggctx_mcxt != NULL) { - MemoryContextDelete(ggctx->edge_table_mcxt); + MemoryContextDelete(ggctx->ggctx_mcxt); } - ggctx->vertex_hashtable = NULL; - ggctx->edge_table = NULL; - ggctx->edge_table_mcxt = NULL; - - /* free the context */ - pfree_if_not_null(ggctx); - ggctx = NULL; - return true; } @@ -1027,6 +1274,7 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name, GRAPH_global_context *new_ggctx = NULL; GRAPH_global_context *curr_ggctx = NULL; GRAPH_global_context *prev_ggctx = NULL; + MemoryContext ggctx_mcxt = NULL; MemoryContext oldctx = NULL; /* we need a higher context, or one that isn't destroyed by SRF exit */ @@ -1105,20 +1353,27 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name, curr_ggctx = curr_ggctx->next; } - /* otherwise, we need to create one and possibly attach it */ - new_ggctx = palloc0(sizeof(GRAPH_global_context)); + /* + * We need to build one. Fail early if pg_class already says the graph + * cannot fit, so that an impossible load does not first spend the time + * and memory of a full scan. + */ + precheck_graph_memory_limit(graph_oid, graph_name); - if (global_graph_contexts != NULL) - { - new_ggctx->next = global_graph_contexts; - } - else - { - new_ggctx->next = NULL; - } + /* + * The whole context - the struct, the graph name, both hashtables and + * every per-vertex allocation - lives in a private context tree so that + * its memory can be measured against age.max_global_graph_memory and + * released with one delete. + */ + ggctx_mcxt = AllocSetContextCreate(TopMemoryContext, + "AGE global graph context", + ALLOCSET_DEFAULT_SIZES); - /* set the global context variable */ - global_graph_contexts = new_ggctx; + MemoryContextSwitchTo(ggctx_mcxt); + + new_ggctx = palloc0(sizeof(GRAPH_global_context)); + new_ggctx->ggctx_mcxt = ggctx_mcxt; /* set the graph name and oid */ new_ggctx->graph_name = pstrdup(graph_name); @@ -1134,14 +1389,42 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name, /* initialize our vertices list */ new_ggctx->vertices = NULL; + new_ggctx->next = NULL; - /* build the hashtables for this graph */ - create_GRAPH_global_hashtables(new_ggctx); - load_GRAPH_global_hashtables(new_ggctx); - freeze_GRAPH_global_hashtables(new_ggctx); + /* + * Build the hashtables, then publish. + * + * The context is deliberately not linked into global_graph_contexts + * until the build has finished. A load can raise an error - a malformed + * label table, or age.max_global_graph_memory being exceeded - and a + * partially built context left on the list would be indistinguishable + * from a complete one on the next call, because a failed load makes no + * change to bump the graph's version counter, so is_ggctx_invalid() + * would accept it and queries would silently see a graph missing + * vertices or edges. + * + * Since an unpublished context is unreachable, the error path has to + * release it here, which is one delete of the private tree. + */ + PG_TRY(); + { + create_GRAPH_global_hashtables(new_ggctx); + load_GRAPH_global_hashtables(new_ggctx); + freeze_GRAPH_global_hashtables(new_ggctx); + } + PG_CATCH(); + { + MemoryContextSwitchTo(oldctx); + MemoryContextDelete(ggctx_mcxt); + PG_RE_THROW(); + } + PG_END_TRY(); + /* the context is complete: publish it */ + new_ggctx->next = global_graph_contexts; + global_graph_contexts = new_ggctx; - /* switch back to the previous memory context */ + /* switch our context back */ MemoryContextSwitchTo(oldctx); return new_ggctx; diff --git a/src/backend/utils/ag_guc.c b/src/backend/utils/ag_guc.c index 466abec55..fb9ca1397 100644 --- a/src/backend/utils/ag_guc.c +++ b/src/backend/utils/ag_guc.c @@ -19,11 +19,14 @@ #include "postgres.h" +#include + #include "utils/guc.h" #include "utils/ag_guc.h" bool age_enable_containment = true; bool age_enable_containment_statistics = true; +int age_max_global_graph_memory = -1; /* * Defines AGE's custom configuration parameters. @@ -53,5 +56,16 @@ void define_config_params(void) NULL, NULL, NULL); + DefineCustomIntVariable("age.max_global_graph_memory", + "Sets the maximum memory a backend may use for cached global graph contexts.", + "The global graph cache holds a full in-memory copy of a graph's adjacency, built once per backend for variable-length-edge and shortest-path traversal. This is the total across every graph cached by the backend. A load that would exceed the limit fails instead of growing the backend without bound. -1 means unlimited.", + &age_max_global_graph_memory, + -1, + -1, INT_MAX, + PGC_SUSET, + GUC_UNIT_KB, + NULL, + NULL, + NULL); EmitWarningsOnPlaceholders("age"); } diff --git a/src/include/utils/ag_guc.h b/src/include/utils/ag_guc.h index e399c631a..ae34e8181 100644 --- a/src/include/utils/ag_guc.h +++ b/src/include/utils/ag_guc.h @@ -40,6 +40,19 @@ extern bool age_enable_containment; extern bool age_enable_containment_statistics; +/* + * Upper bound, in kilobytes, on the memory a single backend may use for its + * cached global graph contexts (the whole-graph adjacency copies built by + * manage_GRAPH_global_contexts for VLE and shortest-path traversal). + * + * -1, the default, means unlimited, which is the historical behavior. Any + * other value is a per-backend total across every graph the backend has + * cached, in the spirit of temp_file_limit: a load that would exceed it + * fails with ERRCODE_CONFIGURATION_LIMIT_EXCEEDED instead of letting the + * backend grow until the OOM killer takes it down. + */ +extern int age_max_global_graph_memory; + void define_config_params(void); #endif From 88a7313ed1b470eed38ae2ec7755ceba550cdc0c Mon Sep 17 00:00:00 2001 From: himmel Date: Tue, 8 Sep 2026 02:41:31 +0000 Subject: [PATCH 3/3] Release cached graphs to stay under the global graph memory limit age.max_global_graph_memory could only fail a load. When a session traverses more than one graph and the limit does not fit them all at once, that means every other statement errors, even though the global graph cache is rebuildable by definition: releasing one context costs a later reload, not lost work. Reload is always the better trade than a failed query, so a load that would exceed the limit now releases other cached graphs to make room, oldest first, and only fails if that is not enough. The reason this was not in the original patch is that a context may be in use when it is chosen. age_vle keeps vlelctx->ggctx and a raw GraphIdNode pointer into that context's vertices list for the duration of one SRF execution, and a single statement can traverse two graphs, so a load for the second can be reached while the first is suspended; freeing there would be a use-after-free. Contexts are therefore refcounted. A context that is released while pinned - by eviction or by the invalidation sweep, which had the same hazard already - is unlinked from global_graph_contexts so that no new caller can find it, and freed when its last user unpins it. Only the window of one SRF execution needs a pin. age_vle takes it after building its local context and drops it from a reset callback on the SRF's multi_call_memory_ctx, so every exit path - completion, error, cancellation - releases it without the pin having to be matched by hand. A VLE_local_context cached across statements needs nothing: it re-resolves its ggctx by oid on reuse and discards itself if that returns NULL or an invalidated context. age_shortest_path and age_all_shortest_paths need nothing either, since they materialize their result paths during the first call and never retain a ggctx. Two guards keep eviction from making things worse. A load that does not fit even in an empty cache evicts nothing: it cannot be helped, and discarding contexts other queries would have reused would only add a reload to the failure. And the pre-load estimate no longer evicts at all. Its bytes-per-element constant is deliberately far below the real cost, so a load that appears to fit in the remaining budget routinely does not, and acting on that estimate meant releasing a live cache and then failing anyway. It now answers only the question it can answer reliably - whether the graph could fit in an empty cache - and the in-load check, which measures real allocations, is the only thing that evicts. Evictions are logged. A session alternating between graphs that do not both fit will reload on every statement, and the remedy is a higher limit, which is only visible to an operator who can see it happening. Verified on a 500k-vertex/1.5M-edge and a 500k/500k graph that do not fit together under a 300 MB limit: alternating between them succeeds instead of failing every other query, through both the shortest-path and the VLE entry points, and RssAnon converges to roughly the limit over 20 alternations rather than growing. A graph too large for the limit still fails, in milliseconds, with the other graph's cache intact. 43/43 installcheck. --- regress/expected/age_global_graph.out | 31 +++- regress/sql/age_global_graph.sql | 17 +- src/backend/utils/adt/age_global_graph.c | 193 +++++++++++++++++++++-- src/backend/utils/adt/age_vle.c | 38 +++++ src/include/utils/age_global_graph.h | 8 + 5 files changed, 268 insertions(+), 19 deletions(-) diff --git a/regress/expected/age_global_graph.out b/regress/expected/age_global_graph.out index e3bea26d9..b86f58be5 100644 --- a/regress/expected/age_global_graph.out +++ b/regress/expected/age_global_graph.out @@ -826,7 +826,8 @@ SELECT * FROM cypher('ggm2', $$ MATCH (a:V {n: 'a'})-[:E*1..1]->(x) RETURN x.n $$) AS (n agtype); ERROR: global graph cache for graph "ggm2" would exceed age.max_global_graph_memory --- and the graph that was already cached is still usable +-- a load that cannot fit even in an empty cache must not evict on its way +-- out: the graph already cached is still there and still usable SELECT * FROM cypher('ggm', $$ MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n ORDER BY x.n $$) AS (n agtype); @@ -836,6 +837,34 @@ $$) AS (n agtype); "c" (2 rows) +-- with a limit that one graph fits under but two do not, the second load +-- evicts the first instead of failing, and both queries succeed +SET age.max_global_graph_memory = '12MB'; +SELECT * FROM cypher('ggm2', $$ + MATCH (a:V {n: 'a'})-[:E*1..1]->(x) RETURN x.n +$$) AS (n agtype); + n +----- + "b" +(1 row) + +SELECT * FROM cypher('ggm', $$ + MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n ORDER BY x.n +$$) AS (n agtype); + n +----- + "b" + "c" +(2 rows) + +SELECT * FROM cypher('ggm2', $$ + MATCH (a:V {n: 'a'})-[:E*1..1]->(x) RETURN x.n +$$) AS (n agtype); + n +----- + "b" +(1 row) + RESET age.max_global_graph_memory; \set VERBOSITY default SELECT * FROM drop_graph('ggm', true); diff --git a/regress/sql/age_global_graph.sql b/regress/sql/age_global_graph.sql index 04ddc4a87..dc328b027 100644 --- a/regress/sql/age_global_graph.sql +++ b/regress/sql/age_global_graph.sql @@ -411,10 +411,25 @@ SELECT * FROM cypher('ggm2', $$ MATCH (a:V {n: 'a'})-[:E*1..1]->(x) RETURN x.n $$) AS (n agtype); --- and the graph that was already cached is still usable +-- a load that cannot fit even in an empty cache must not evict on its way +-- out: the graph already cached is still there and still usable SELECT * FROM cypher('ggm', $$ MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n ORDER BY x.n $$) AS (n agtype); + +-- with a limit that one graph fits under but two do not, the second load +-- evicts the first instead of failing, and both queries succeed +SET age.max_global_graph_memory = '12MB'; +SELECT * FROM cypher('ggm2', $$ + MATCH (a:V {n: 'a'})-[:E*1..1]->(x) RETURN x.n +$$) AS (n agtype); +SELECT * FROM cypher('ggm', $$ + MATCH (a:V {n: 'a'})-[:E*1..2]->(x) RETURN x.n ORDER BY x.n +$$) AS (n agtype); +SELECT * FROM cypher('ggm2', $$ + MATCH (a:V {n: 'a'})-[:E*1..1]->(x) RETURN x.n +$$) AS (n agtype); + RESET age.max_global_graph_memory; \set VERBOSITY default diff --git a/src/backend/utils/adt/age_global_graph.c b/src/backend/utils/adt/age_global_graph.c index ca8ecd3c9..4e2921d35 100644 --- a/src/backend/utils/adt/age_global_graph.c +++ b/src/backend/utils/adt/age_global_graph.c @@ -174,6 +174,8 @@ typedef struct GRAPH_global_context MemoryContext vertex_mcxt; /* child context owning the vertex side */ MemoryContext edge_table_mcxt; /* child context owning edge_table */ int64 memcheck_counter; /* insertions since the last memory check */ + int refcount; /* live users holding this pointer */ + bool unlinked; /* off the list; free at the last unpin */ uint64 graph_version; /* version counter for cache invalidation */ TransactionId xmin; /* snapshot fallback: transaction xmin */ TransactionId xmax; /* snapshot fallback: transaction xmax */ @@ -247,6 +249,9 @@ static Size ggctx_memory_limit_bytes(void); static void ggctx_memory_limit_error(const char *graph_name, Size required, Size other_cached, Size limit); static void precheck_graph_memory_limit(Oid graph_oid, char *graph_name); +static void release_GRAPH_global_context(GRAPH_global_context *ggctx); +static bool evict_GRAPH_global_contexts(GRAPH_global_context *except, + Size needed, Size limit); static void enforce_ggctx_memory_limit(GRAPH_global_context *ggctx); static void check_ggctx_memory_limit(GRAPH_global_context *ggctx); static List *get_ag_labels_names(Snapshot snapshot, Oid graph_oid, @@ -769,6 +774,152 @@ static bool insert_vertex_edge(GRAPH_global_context *ggctx, } +/* + * Refcounting for cached contexts. + * + * A context can be released for two reasons: it was invalidated by a write to + * the graph, or it was evicted to make room under + * age.max_global_graph_memory. Either way it may be in use: age_vle keeps + * vlelctx->ggctx and a raw GraphIdNode pointer into the vertices list for the + * duration of one SRF execution, and a single statement can traverse two + * graphs, so a load for the second graph can be reached while the first is + * suspended. Freeing under a running traversal would be a use-after-free, so + * a pinned context is unlinked from the list instead - no new caller can find + * it - and freed when its last user releases it. + * + * Only the window of one SRF execution needs a pin. A VLE_local_context + * cached across statements does not trust its stored ggctx: it looks the + * graph up again by oid and discards itself if that returns NULL or an + * invalidated context. age_shortest_path never retains a ggctx at all; it + * materializes its result paths during the first call. + */ +void pin_GRAPH_global_context(GRAPH_global_context *ggctx) +{ + if (ggctx != NULL) + { + ggctx->refcount++; + } +} + +void unpin_GRAPH_global_context(GRAPH_global_context *ggctx) +{ + if (ggctx == NULL) + { + return; + } + + Assert(ggctx->refcount > 0); + + if (ggctx->refcount > 0) + { + ggctx->refcount--; + } + + /* the last user of an already unlinked context frees it */ + if (ggctx->refcount == 0 && ggctx->unlinked) + { + free_specific_GRAPH_global_context(ggctx); + } +} + +/* + * Drop a context that has already been taken off global_graph_contexts, + * deferring the free while it is still in use. + */ +static void release_GRAPH_global_context(GRAPH_global_context *ggctx) +{ + if (ggctx == NULL) + { + return; + } + + ggctx->next = NULL; + + if (ggctx->refcount > 0) + { + ggctx->unlinked = true; + return; + } + + free_specific_GRAPH_global_context(ggctx); +} + +/* + * Release cached contexts to make room for a load of `needed` bytes, oldest + * first, skipping contexts that are in use and the one being built. Returns + * true when the load now fits. + * + * The cache is rebuildable by definition, so evicting is always preferable to + * failing a query: the cost is a later reload, not lost work. It is logged + * because a session alternating between graphs that do not both fit will + * evict on every statement, and the remedy for that - a higher limit - is + * only visible to an operator who can see it happening. + */ +static bool evict_GRAPH_global_contexts(GRAPH_global_context *except, + Size needed, Size limit) +{ + /* + * If the load does not fit even in an empty cache, evicting cannot help, + * and throwing away contexts that other queries would have reused makes + * the failure worse than it needs to be. Fail with the cache intact. + */ + if (needed > limit) + { + return false; + } + + for (;;) + { + GRAPH_global_context *victim = NULL; + GRAPH_global_context *prev_of_victim = NULL; + GRAPH_global_context *prev = NULL; + GRAPH_global_context *curr = NULL; + Size victim_bytes; + + if (cached_graphs_memory_used(except) + needed <= limit) + { + return true; + } + + /* + * Pick the last eligible context in the list. New contexts are added + * at the head, so this evicts the least recently built first. + */ + for (curr = global_graph_contexts; curr != NULL; curr = curr->next) + { + if (curr != except && curr->refcount == 0) + { + victim = curr; + prev_of_victim = prev; + } + + prev = curr; + } + + if (victim == NULL) + { + return false; + } + + victim_bytes = ggctx_memory_used(victim); + + if (prev_of_victim == NULL) + { + global_graph_contexts = victim->next; + } + else + { + prev_of_victim->next = victim->next; + } + + ereport(LOG, + (errmsg("released cached global graph \"%s\" (%zu kB) to stay within age.max_global_graph_memory", + victim->graph_name, (victim_bytes + 1023) / 1024))); + + release_GRAPH_global_context(victim); + } +} + /* * age.max_global_graph_memory support. * @@ -932,10 +1083,22 @@ static void precheck_graph_memory_limit(Oid graph_oid, char *graph_name) MemoryContextDelete(tmpctx); required = (Size) (elements * GGCTX_PRECHECK_BYTES_PER_ELEMENT); - other_cached = cached_graphs_memory_used(NULL); - if (other_cached + required > limit) + /* + * Compare against the limit alone, not against the limit minus what is + * already cached, and do not evict here. + * + * The estimate is deliberately far below the real cost, so a load that + * looks like it fits in the remaining budget routinely does not. Evicting + * on the strength of it would throw away contexts other queries would + * have reused and then fail anyway. This check therefore answers only the + * question it can answer reliably - whether the graph could fit even in + * an empty cache - and leaves everything else to the in-load check, which + * measures real allocations and can evict on that basis. + */ + if (required > limit) { + other_cached = cached_graphs_memory_used(NULL); ggctx_memory_limit_error(graph_name, required, other_cached, limit); } } @@ -959,12 +1122,14 @@ static void enforce_ggctx_memory_limit(GRAPH_global_context *ggctx) } used = ggctx_memory_used(ggctx); - other_cached = cached_graphs_memory_used(ggctx); - if (other_cached + used > limit) + if (evict_GRAPH_global_contexts(ggctx, used, limit)) { - ggctx_memory_limit_error(ggctx->graph_name, used, other_cached, limit); + return; } + + other_cached = cached_graphs_memory_used(ggctx); + ggctx_memory_limit_error(ggctx->graph_name, used, other_cached, limit); } /* @@ -1302,8 +1467,6 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name, /* if the transaction ids have changed, we have an invalid graph */ if (is_ggctx_invalid(curr_ggctx)) { - bool success = false; - /* * If prev_ggctx is NULL then we are freeing the top of the * contexts. So, we need to point the contexts variable to the @@ -1318,16 +1481,12 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name, prev_ggctx->next = curr_ggctx->next; } - /* free the current graph context */ - success = free_specific_GRAPH_global_context(curr_ggctx); - - /* if it wasn't successfull, there was a missing vertex entry */ - if (!success) - { - - ereport(ERROR, (errcode(ERRCODE_DATA_EXCEPTION), - errmsg("missing vertex or edge entry during free"))); - } + /* + * Drop the context. It is already off the list, so no new caller + * can find it; if a traversal is still walking it the free is + * deferred to that traversal's release. + */ + release_GRAPH_global_context(curr_ggctx); } else { diff --git a/src/backend/utils/adt/age_vle.c b/src/backend/utils/adt/age_vle.c index cb036b154..b898660b8 100644 --- a/src/backend/utils/adt/age_vle.c +++ b/src/backend/utils/adt/age_vle.c @@ -196,6 +196,7 @@ static VLE_local_context *build_local_vle_context(FunctionCallInfo fcinfo, FuncCallContext *funcctx); static void create_VLE_local_state_hashtable(VLE_local_context *vlelctx); static void free_VLE_local_context(VLE_local_context *vlelctx); +static void vle_unpin_ggctx_callback(void *arg); /* VLE graph traversal functions */ static edge_state_entry *get_edge_state_with_hash(VLE_local_context *vlelctx, graphid edge_id, @@ -1991,6 +1992,30 @@ Datum age_vle(PG_FUNCTION_ARGS) */ funcctx->user_fctx = vlelctx; + /* + * Pin the GRAPH global context for the rest of this execution. + * + * vlelctx->ggctx, and the GraphIdNode pointer into that context's + * vertices list, are dereferenced on every subsequent call. Without a + * pin, a load for another graph reached while this SRF is suspended + * could evict or invalidate the context out from under the traversal. + * A VLE_local_context cached across statements does not need this: it + * re-resolves its ggctx by oid on reuse. + */ + if (vlelctx->ggctx != NULL) + { + MemoryContextCallback *cb; + + pin_GRAPH_global_context(vlelctx->ggctx); + + cb = MemoryContextAlloc(funcctx->multi_call_memory_ctx, + sizeof(MemoryContextCallback)); + cb->func = vle_unpin_ggctx_callback; + cb->arg = vlelctx->ggctx; + MemoryContextRegisterResetCallback(funcctx->multi_call_memory_ctx, + cb); + } + /* if we are starting from zero [*0..x] flag it */ if (vlelctx->lidx == 0) { @@ -2140,6 +2165,19 @@ Datum age_vle(PG_FUNCTION_ARGS) } } +/* + * Release the pin taken on the GRAPH global context for the duration of one + * age_vle SRF execution. + * + * Registered as a reset callback on the SRF's multi_call_memory_ctx so that + * it runs on every exit path - normal completion, an error, a cancelled + * query - without the pin having to be matched by hand at each of them. + */ +static void vle_unpin_ggctx_callback(void *arg) +{ + unpin_GRAPH_global_context((GRAPH_global_context *) arg); +} + /* * Exposed helper function to make an agtype AGTV_PATH from a * VLE_path_container. diff --git a/src/include/utils/age_global_graph.h b/src/include/utils/age_global_graph.h index d68530a91..09ad4ca1a 100644 --- a/src/include/utils/age_global_graph.h +++ b/src/include/utils/age_global_graph.h @@ -60,6 +60,14 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name, Oid graph_oid); GRAPH_global_context *find_GRAPH_global_context(Oid graph_oid); bool is_ggctx_invalid(GRAPH_global_context *ggctx); +/* + * Pin a context for as long as a caller holds the pointer, or a pointer into + * anything the context owns. A pinned context that is invalidated or evicted + * is unlinked so no new caller finds it, and freed when the last user + * releases it. + */ +void pin_GRAPH_global_context(GRAPH_global_context *ggctx); +void unpin_GRAPH_global_context(GRAPH_global_context *ggctx); /* GRAPH retrieval functions */ ListGraphId *get_graph_vertices(GRAPH_global_context *ggctx); vertex_entry *get_vertex_entry(GRAPH_global_context *ggctx,