Avoid a potential unstable test case: xmlmap.sql
Hi:
In the test case of xmlmap.sql, we have the query below
under schema_to_xml.
explain (costs off, verbose)
SELECT oid FROM pg_catalog.pg_class
WHERE relnamespace = 28601
AND relkind IN ('r','m','v')
AND pg_catalog.has_table_privilege (oid, 'SELECT')
ORDER BY relname;
If the query is using SeqScan, the execution order of the quals is:
has_table_privilege(pg_class.oid, 'SELECT'::text) AND
(pg_class.relnamespace = '28601'::oid) AND (pg_class.relkind = ANY
('{r,m,v}'::"char"[]))
based on current cost setting and algorithm. With this plan,
has_table_privilege(pg_class.oid, 'SELECT'::text) may be executed
against all the relations (not just the given namespace), so if a
tuple in pg_class is scanned and before has_table_privilege is called,
the relation is dropped, then we will get error:
ERROR: relation with OID xxx does not exist
To overcome this, if disabling the seqscan, then only index scan on
relnamespace is possible, so relnamespace = '28601'::oid will be filtered
first before calling has_table_privilege. and in this test case, we are
sure
the relation belonging to the current namespace will never be dropped, so
no error is possible. Here is the plan for reference:
Seq Scan:
Sort
Output: oid, relname
Sort Key: pg_class.relname
-> Seq Scan on pg_catalog.pg_class
Output: oid, relname
Filter: (has_table_privilege(pg_class.oid, 'SELECT'::text) AND
(pg_class.relnamespace = '28601'::oid) AND (pg_class.relkind = ANY
('{r,m,v}'::"char"[])))
enable_seqscan to off
QUERY PLAN
------------------------------------------------------------------------------------------------------------------
Index Scan using pg_class_relname_nsp_index on pg_catalog.pg_class
Output: oid, relname
Index Cond: (pg_class.relnamespace = '28601'::oid)
Filter: (has_table_privilege(pg_class.oid, 'SELECT'::text) AND
(pg_class.relkind = ANY ('{r,m,v}'::"char"[])))
Patch is attached.
--
Best Regards
Andy Fan
Attachments:
v1-0001-Avoid-a-potential-unstable-testcase.patchapplication/octet-stream; name=v1-0001-Avoid-a-potential-unstable-testcase.patchDownload
From 30463655f6959686e7344119044226530b50e628 Mon Sep 17 00:00:00 2001
From: Andy Fan <zhihui.fan1213@gmail.com>
Date: Tue, 15 Aug 2023 18:50:00 +0800
Subject: [PATCH v1] Avoid a potential unstable testcase.
---
src/test/regress/expected/xmlmap.out | 8 ++++++++
src/test/regress/sql/xmlmap.sql | 10 +++++++++-
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/src/test/regress/expected/xmlmap.out b/src/test/regress/expected/xmlmap.out
index ccc54606630..8ea1293c3fa 100644
--- a/src/test/regress/expected/xmlmap.out
+++ b/src/test/regress/expected/xmlmap.out
@@ -809,6 +809,12 @@ SELECT cursor_to_xmlschema('xc'::refcursor, true, false, '');
</xsd:schema>
(1 row)
+-- In order to not scan the relation which not belongs to
+-- the given schema, it is better to make sure the filter
+-- on relnamespace is executed as the first qual. Index
+-- scan can grantee that while SeqScan will just break it.
+set enable_seqscan to off;
+set enable_bitmapscan to off;
SELECT schema_to_xml('testxmlschema', false, true, '');
schema_to_xml
-----------------------------------------------------------------------
@@ -1276,6 +1282,8 @@ SELECT schema_to_xml_and_xmlschema('testxmlschema', true, true, 'foo');
(1 row)
+reset enable_seqscan;
+reset enable_bitmapscan;
-- test that domains are transformed like their base types
CREATE DOMAIN testboolxmldomain AS bool;
CREATE DOMAIN testdatexmldomain AS date;
diff --git a/src/test/regress/sql/xmlmap.sql b/src/test/regress/sql/xmlmap.sql
index 16582bf6abd..89dfcf89fb7 100644
--- a/src/test/regress/sql/xmlmap.sql
+++ b/src/test/regress/sql/xmlmap.sql
@@ -41,12 +41,20 @@ MOVE BACKWARD ALL IN xc;
SELECT cursor_to_xml('xc'::refcursor, 5, true, false, '');
SELECT cursor_to_xmlschema('xc'::refcursor, true, false, '');
+-- In order to not scan the relation which not belongs to
+-- the given schema, it is better to make sure the filter
+-- on relnamespace is executed as the first qual. Index
+-- scan can grantee that while SeqScan will just break it.
+
+set enable_seqscan to off;
+set enable_bitmapscan to off;
SELECT schema_to_xml('testxmlschema', false, true, '');
SELECT schema_to_xml('testxmlschema', true, false, '');
SELECT schema_to_xmlschema('testxmlschema', false, true, '');
SELECT schema_to_xmlschema('testxmlschema', true, false, '');
SELECT schema_to_xml_and_xmlschema('testxmlschema', true, true, 'foo');
-
+reset enable_seqscan;
+reset enable_bitmapscan;
-- test that domains are transformed like their base types
--
2.21.0
I overlooked the fact even in the bitmap index scan loose mode, the recheck
is still executed before the qual, so bitmap index scan is OK in this case.
Sort
Output: oid, relname
Sort Key: pg_class.relname
-> Bitmap Heap Scan on pg_catalog.pg_class
Output: oid, relname
Recheck Cond: (pg_class.relnamespace = '28601'::oid)
Filter: (has_table_privilege(pg_class.oid, 'SELECT'::text) AND
(pg_class.relkind = ANY ('{r,m,v}'::"char"[])))
-> Bitmap Index Scan on pg_class_relname_nsp_index
Index Cond: (pg_class.relnamespace = '28601'::oid)
v2 attached.
--
Best Regards
Andy Fan
Attachments:
v2-0001-Avoid-a-potential-unstable-testcase.patchapplication/octet-stream; name=v2-0001-Avoid-a-potential-unstable-testcase.patchDownload
From a0093353d5fad21e044468825a9f321289d75dd0 Mon Sep 17 00:00:00 2001
From: Andy Fan <zhihui.fan1213@gmail.com>
Date: Tue, 15 Aug 2023 18:50:00 +0800
Subject: [PATCH v2] Avoid a potential unstable testcase.
---
src/test/regress/expected/xmlmap.out | 6 ++++++
src/test/regress/sql/xmlmap.sql | 8 +++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/test/regress/expected/xmlmap.out b/src/test/regress/expected/xmlmap.out
index ccc54606630..d186b260e73 100644
--- a/src/test/regress/expected/xmlmap.out
+++ b/src/test/regress/expected/xmlmap.out
@@ -809,6 +809,11 @@ SELECT cursor_to_xmlschema('xc'::refcursor, true, false, '');
</xsd:schema>
(1 row)
+-- In order to not scan the relation which not belongs to
+-- the given schema, it is better to make sure the filter
+-- on relnamespace is executed as the first qual. Index
+-- scan can grantee that while SeqScan will just break it.
+set enable_seqscan to off;
SELECT schema_to_xml('testxmlschema', false, true, '');
schema_to_xml
-----------------------------------------------------------------------
@@ -1276,6 +1281,7 @@ SELECT schema_to_xml_and_xmlschema('testxmlschema', true, true, 'foo');
(1 row)
+reset enable_seqscan;
-- test that domains are transformed like their base types
CREATE DOMAIN testboolxmldomain AS bool;
CREATE DOMAIN testdatexmldomain AS date;
diff --git a/src/test/regress/sql/xmlmap.sql b/src/test/regress/sql/xmlmap.sql
index 16582bf6abd..4aaba5a0b49 100644
--- a/src/test/regress/sql/xmlmap.sql
+++ b/src/test/regress/sql/xmlmap.sql
@@ -41,12 +41,18 @@ MOVE BACKWARD ALL IN xc;
SELECT cursor_to_xml('xc'::refcursor, 5, true, false, '');
SELECT cursor_to_xmlschema('xc'::refcursor, true, false, '');
+-- In order to not scan the relation which not belongs to
+-- the given schema, it is better to make sure the filter
+-- on relnamespace is executed as the first qual. Index
+-- scan can grantee that while SeqScan will just break it.
+
+set enable_seqscan to off;
SELECT schema_to_xml('testxmlschema', false, true, '');
SELECT schema_to_xml('testxmlschema', true, false, '');
SELECT schema_to_xmlschema('testxmlschema', false, true, '');
SELECT schema_to_xmlschema('testxmlschema', true, false, '');
SELECT schema_to_xml_and_xmlschema('testxmlschema', true, true, 'foo');
-
+reset enable_seqscan;
-- test that domains are transformed like their base types
--
2.21.0
Hi Andy,
15.08.2023 14:09, Andy Fan wrote:
Hi:
In the test case of xmlmap.sql, we have the query below under schema_to_xml.
Please look at the bug #18014:
/messages/by-id/18014-28c81cb79d44295d@postgresql.org
There were other aspects of the xmlmap test failure discussed in that thread as well.
Best regards,
Alexander
Please look at the bug #18014:
/messages/by-id/18014-28c81cb79d44295d@postgresql.org
There were other aspects of the xmlmap test failure discussed in that
thread as well.
Thank you Alexander for the information, I will go through there for
discussion.
--
Best Regards
Andy Fan