Copilot commented on code in PR #2577:
URL: https://github.com/apache/age/pull/2577#discussion_r4084647866
##########
age--1.8.0--y.y.y.sql:
##########
@@ -65,3 +65,38 @@ CREATE FUNCTION ag_catalog.load_edges_from_file(graph_name
name,
RETURNS void
LANGUAGE c
AS 'MODULE_PATHNAME';
+
+DO $$
+DECLARE
+ btree_family oid;
+BEGIN
+ SELECT opcfamily INTO STRICT btree_family
+ FROM pg_catalog.pg_opclass
+ WHERE opcnamespace = 'ag_catalog'::regnamespace
+ AND opcname = 'agtype_ops_btree'
+ AND opcmethod = (SELECT oid FROM pg_catalog.pg_am WHERE amname =
'btree');
+
+ UPDATE pg_catalog.pg_amop
+ SET amopstrategy = amopstrategy + 10
+ WHERE amopfamily = btree_family
+ AND amoplefttype = 'ag_catalog.agtype'::regtype
+ AND amoprighttype = 'ag_catalog.agtype'::regtype
+ AND amopstrategy IN (4, 5);
+
+ UPDATE pg_catalog.pg_amop
+ SET amopstrategy = CASE amopopr
+ WHEN 'ag_catalog.>=(ag_catalog.agtype,ag_catalog.agtype)'::regoperator
THEN 4
+ WHEN 'ag_catalog.>(ag_catalog.agtype,ag_catalog.agtype)'::regoperator
THEN 5
+ END
+ WHERE amopfamily = btree_family
+ AND amoplefttype = 'ag_catalog.agtype'::regtype
+ AND amoprighttype = 'ag_catalog.agtype'::regtype
+ AND amopstrategy IN (14, 15);
Review Comment:
This CASE expression has no ELSE branch, so any unexpected `amopopr` value
matching the WHERE clause would set `amopstrategy` to NULL, which can fail
(and/or corrupt the intended mapping) since `pg_amop.amopstrategy` is not
nullable. To make this robust, either (a) add an ELSE that preserves the
current value, or (b) tighten the WHERE clause to only target the two intended
operators (e.g., add an `amopopr IN (...)` predicate) and optionally raise if
the expected rows aren’t present.
##########
age--1.8.0--y.y.y.sql:
##########
@@ -65,3 +65,38 @@ CREATE FUNCTION ag_catalog.load_edges_from_file(graph_name
name,
RETURNS void
LANGUAGE c
AS 'MODULE_PATHNAME';
+
+DO $$
+DECLARE
+ btree_family oid;
+BEGIN
+ SELECT opcfamily INTO STRICT btree_family
+ FROM pg_catalog.pg_opclass
+ WHERE opcnamespace = 'ag_catalog'::regnamespace
+ AND opcname = 'agtype_ops_btree'
+ AND opcmethod = (SELECT oid FROM pg_catalog.pg_am WHERE amname =
'btree');
+
+ UPDATE pg_catalog.pg_amop
+ SET amopstrategy = amopstrategy + 10
+ WHERE amopfamily = btree_family
+ AND amoplefttype = 'ag_catalog.agtype'::regtype
+ AND amoprighttype = 'ag_catalog.agtype'::regtype
+ AND amopstrategy IN (4, 5);
+
+ UPDATE pg_catalog.pg_amop
+ SET amopstrategy = CASE amopopr
+ WHEN 'ag_catalog.>=(ag_catalog.agtype,ag_catalog.agtype)'::regoperator
THEN 4
+ WHEN 'ag_catalog.>(ag_catalog.agtype,ag_catalog.agtype)'::regoperator
THEN 5
+ END
+ WHERE amopfamily = btree_family
+ AND amoplefttype = 'ag_catalog.agtype'::regtype
+ AND amoprighttype = 'ag_catalog.agtype'::regtype
+ AND amopstrategy IN (14, 15);
+
+ UPDATE pg_catalog.pg_opclass
+ SET opcfamily = btree_family
+ WHERE opcnamespace = 'ag_catalog'::regnamespace
+ AND opcname = 'agtype_ops_btree'
+ AND opcfamily = btree_family;
Review Comment:
This UPDATE is intentionally a no-op assignment (it sets `opcfamily` to its
current value) to force syscache invalidation, but that purpose isn’t obvious
from the SQL and the `AND opcfamily = btree_family` predicate is redundant
given how `btree_family` is selected. Add an explicit comment explaining that
this statement exists solely to invalidate cached ordering operators (so future
maintainers don’t remove it as dead code), and consider simplifying the WHERE
clause to reduce confusion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
