Skip to content

Commit

Permalink
Fix operator family identifiers.
Browse files Browse the repository at this point in the history
The previous code was broken, as an operator family name is only unique for a
given access method.

Thanks to Zacharias Knudsen for the report.  As seen in the bug report, it can
lead to false positive problem as the comparision was depending on the physical
tuple positions in the table.

Tests added to reproduce the problem in the pg_broken_extupgrade extension in
"head" version.  I wasn't able to reproduce it with only 2 operator families
though.  There is also another test to cover an actual diff with operator
families.
  • Loading branch information
rjuju committed Nov 24, 2022
1 parent f9dfc6e commit 0bbacb3
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 1 deletion.
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,14 @@ Some GUC changes leaked the script for version head-1.0--head-1.1:
- upgraded has no value, while installed has
+ {rjuju=UC/rjuju}
- in opfamilies:
installed and upgraded both have 4 OpFamily but some mismatch in them:
1 OpFamily missing in installed:
- public.my_opf_btree USING btree
1 OpFamily missing in upgraded:
- public.my_opf_btree USING gin
- in extra_queries:
installed and upgraded both have 3 Resultset but some mismatch in them:
SELECT 1 / (random() * 2)::int AS may_fail
Expand Down
2 changes: 1 addition & 1 deletion src/extension/pg_opfamily.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::{

DbStruct! {
OpFamily:opfname:OpFamily {
opfname: Text = ("n.nspname || '.' || opf.opfname"),
opfname: Text = ("n.nspname || '.' || opf.opfname || ' USING ' || am.amname"),
opfmethod: Name = ("am.amname"),
opfowner: Name = ("r.rolname"),
}
Expand Down
5 changes: 5 additions & 0 deletions tests/fixtures/pg_broken_extupgrade--head-1.0--head-1.1.sql
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@
SET work_mem = '321kB';
SET LOCAL maintenance_work_mem TO '6666';
SET LOCAL client_min_messages = WARNING;

-- those 3 are just to validate that the am is used in the identifier
ALTER OPERATOR FAMILY my_opf1 USING gist RENAME TO my_opf2;
ALTER OPERATOR FAMILY my_opf1 USING gin RENAME TO my_opf2;
ALTER OPERATOR FAMILY my_opf1 USING btree RENAME TO my_opf2;
5 changes: 5 additions & 0 deletions tests/fixtures/pg_broken_extupgrade--head-1.0.sql
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,8 @@ COMMENT ON AGGREGATE agg_1(int) IS 'smaller';
CREATE POLICY popol0 ON tbl0 AS PERMISSIVE FOR ALL TO public USING (id > 0);
CREATE SCHEMA nsp_1;
CREATE SCHEMA nsp_2;
CREATE OPERATOR FAMILY my_opf_btree USING btree;
-- those 3 are just to validate that the am is used in the identifier
CREATE OPERATOR FAMILY my_opf1 USING btree;
CREATE OPERATOR FAMILY my_opf1 USING gin;
CREATE OPERATOR FAMILY my_opf1 USING gist;
5 changes: 5 additions & 0 deletions tests/fixtures/pg_broken_extupgrade--head-1.1.sql
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,8 @@ CREATE POLICY popol0 ON tbl0 AS RESTRICTIVE FOR SELECT TO current_user USING (id
CREATE SCHEMA nsp_1;
CREATE SCHEMA nsp_2;
REVOKE USAGE ON SCHEMA nsp_2 FROM public;
CREATE OPERATOR FAMILY my_opf_btree USING gin;
-- those 3 are just to validate that the am is used in the identifier
CREATE OPERATOR FAMILY my_opf2 USING btree;
CREATE OPERATOR FAMILY my_opf2 USING gin;
CREATE OPERATOR FAMILY my_opf2 USING gist;

0 comments on commit 0bbacb3

Please sign in to comment.