Limit eartdistance regression testcase to the public schema

Started by Zsolt Parragi4 months ago3 messages
#1Zsolt Parragi
zsolt.parragi@percona.com
1 attachment(s)

Hello hackers,

We try to run the entire postgres regression test suite with our
extensions loaded into custom schemas, to verify that we don't break
anything. This works quite well, except for one issue: the
earthdistance regression test case displays everything from every
schema, and because of this, we have to record an alternative output
for it. And we have to rerecord this every time we change something in
an extension.

I attached a simple patch to the test case that modifies it to only
list objects from the public schema. Part of the test case is already
limited to the public schema, this patch just aligns the rest to do
the same.

What do you think?

Attachments:

0001-Earthdistance-test-should-only-list-objects-from-the.patchapplication/octet-stream; name=0001-Earthdistance-test-should-only-list-objects-from-the.patchDownload
From 715bcb405a32037e2c006ce0822aa3ed5c0570da Mon Sep 17 00:00:00 2001
From: Zsolt Parragi <zsolt.parragi@percona.com>
Date: Tue, 18 Feb 2025 22:58:49 +0000
Subject: [PATCH] Earthdistance test should only list objects from the public
 schema

It is possible to run the test suite with additional extensions
installed to make sure that everything works with them. If the
additional extension is installed in a custom schema it doesn't change
any of the normal test outputs.

Except for the eartdistance test, which lists all objects from every
schema.

This commit modifies the testcase so that it only selects objects from
the public schema to avoid this. This was already the case for part of
the test, these changes that unify everything in it.
---
 contrib/earthdistance/expected/earthdistance.out | 10 +++++-----
 contrib/earthdistance/sql/earthdistance.sql      | 10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/contrib/earthdistance/expected/earthdistance.out b/contrib/earthdistance/expected/earthdistance.out
index 26a843c3faa..298df2152a3 100644
--- a/contrib/earthdistance/expected/earthdistance.out
+++ b/contrib/earthdistance/expected/earthdistance.out
@@ -962,7 +962,7 @@ SELECT abs(cube_distance(ll_to_earth(-30,-90), '(0)'::cube) / earth() - 1) <
 -- Now we are going to test extension create/drop scenarios.
 --
 -- list what's installed
-\dT
+\dT public.*
                                               List of data types
  Schema | Name  |                                         Description                                         
 --------+-------+---------------------------------------------------------------------------------------------
@@ -979,7 +979,7 @@ drop type cube;  -- fail, extension cube requires it
 ERROR:  cannot drop type cube because extension cube requires it
 HINT:  You can drop extension cube instead.
 -- list what's installed
-\dT
+\dT public.*
                                              List of data types
  Schema | Name |                                         Description                                         
 --------+------+---------------------------------------------------------------------------------------------
@@ -994,19 +994,19 @@ HINT:  Use DROP ... CASCADE to drop the dependent objects too.
 drop table foo;
 drop extension cube;
 -- list what's installed
-\dT
+\dT public.*
      List of data types
  Schema | Name | Description 
 --------+------+-------------
 (0 rows)
 
-\df
+\df public.*
                        List of functions
  Schema | Name | Result data type | Argument data types | Type 
 --------+------+------------------+---------------------+------
 (0 rows)
 
-\do
+\do public.*
                              List of operators
  Schema | Name | Left arg type | Right arg type | Result type | Description 
 --------+------+---------------+----------------+-------------+-------------
diff --git a/contrib/earthdistance/sql/earthdistance.sql b/contrib/earthdistance/sql/earthdistance.sql
index 41455612175..e80a7c53e30 100644
--- a/contrib/earthdistance/sql/earthdistance.sql
+++ b/contrib/earthdistance/sql/earthdistance.sql
@@ -304,7 +304,7 @@ SELECT abs(cube_distance(ll_to_earth(-30,-90), '(0)'::cube) / earth() - 1) <
 --
 
 -- list what's installed
-\dT
+\dT public.*
 
 drop extension cube;  -- fail, earthdistance requires it
 
@@ -313,7 +313,7 @@ drop extension earthdistance;
 drop type cube;  -- fail, extension cube requires it
 
 -- list what's installed
-\dT
+\dT public.*
 
 create table foo (f1 cube, f2 int);
 
@@ -324,9 +324,9 @@ drop table foo;
 drop extension cube;
 
 -- list what's installed
-\dT
-\df
-\do
+\dT public.*
+\df public.*
+\do public.*
 
 create schema c;
 
-- 
2.43.0

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Zsolt Parragi (#1)
Re: Limit eartdistance regression testcase to the public schema

Zsolt Parragi <zsolt.parragi@percona.com> writes:

I attached a simple patch to the test case that modifies it to only
list objects from the public schema. Part of the test case is already
limited to the public schema, this patch just aligns the rest to do
the same.

What do you think?

I think this is breaking the test, because part of the point is
to check that there's not anything installed outside the public
schema (until we tell it to, further down).

Admittedly, this only checks schemas that are in the search_path,
but why do you have other stuff in the search_path? That seems
to cause all kinds of risks of accidentally interfering with
assorted test cases.

regards, tom lane

#3Zsolt Parragi
zsolt.parragi@percona.com
In reply to: Tom Lane (#2)
Re: Limit eartdistance regression testcase to the public schema

why do you have other stuff in the search_path? That seems
to cause all kinds of risks of accidentally interfering with
assorted test cases.

That was a workaround that seemed to work with everything, except the
earthdistance testcase.

* If I create the extension in the public schema, that interferes with
multiple postgres tests
* If I create the extension in a specific schema, that doesn't change
any postgres test outputs, but then I can't run the existing tests of
the extension itself on the same installation, as it expects the
extension in the search path
* If I create the extension in a specific schema, and add it to the
search path, only the earthdistance testcase has failures

I think this is breaking the test, because part of the point is
to check that there's not anything installed outside the public
schema (until we tell it to, further down).

I see. Further down the next checks specifically only test the
`public` and `c` schemas, but `c` doesn't exist at this point, it is
only created later. This seemed like an easy and safe fix based on
this, I assumed it is also okay to limit the earlier test to the same
schemas. Is there a reason why the later checks are more specific,
shouldn't we remove the `public.*` from the later tests then?

There are of course other solutions to my problem, I mainly chose this
approach because it looked logical to unify these checks.

Show quoted text

On Fri, Sep 26, 2025 at 7:16 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Zsolt Parragi <zsolt.parragi@percona.com> writes:

I attached a simple patch to the test case that modifies it to only
list objects from the public schema. Part of the test case is already
limited to the public schema, this patch just aligns the rest to do
the same.

What do you think?

I think this is breaking the test, because part of the point is
to check that there's not anything installed outside the public
schema (until we tell it to, further down).

Admittedly, this only checks schemas that are in the search_path,
but why do you have other stuff in the search_path? That seems
to cause all kinds of risks of accidentally interfering with
assorted test cases.

regards, tom lane