Re: meson and check-tests

2024-06-01 Thread Tom Lane
"Tristan Partin"  writes:
> On Fri May 31, 2024 at 12:02 PM CDT, Ashutosh Bapat wrote:
>> We talked this off-list at the conference. It seems we have to somehow
>> avoid passing pg_regress --schedule argument and instead pass the list of
>> tests. Any idea how to do that?

> I think there are 2 solutions to this.
> 1. Avoid passing --schedule by default, which doesn't sound like a great 
>solution.
> 2. Teach pg_regress to ignore the --schedule option if specific tests 
>are passed instead.
> 3. Add a --no-schedule option to pg_regress which would override the 
>previously added --schedule option.
> I personally prefer 2 or 3.

Just to refresh peoples' memory of what the Makefiles do:
src/test/regress/GNUmakefile has

check: all
$(pg_regress_check) $(REGRESS_OPTS) 
--schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)

check-tests: all | temp-install
$(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS) 
$(EXTRA_TESTS)

(and parallel cases for installcheck etc).  AFAICS, meson.build has
no equivalent to the EXTRA_TESTS add-on, nor does it have behavior
equivalent to check-tests' substitution of $(TESTS) for --schedule.
But I suggest that those behaviors have stood for a long time and
so the appropriate thing to do is duplicate them as best we can,
not invent something different.

regards, tom lane




Re: meson and check-tests

2024-06-01 Thread jian he
On Sun, Jun 2, 2024 at 4:48 AM Tristan Partin  wrote:
>
> On Fri May 31, 2024 at 12:02 PM CDT, Ashutosh Bapat wrote:
> > Hi Tristan,
> > Using make I can run only selected tests under src/test/regress using
> > TESTS=... make check-tests. I didn't find any way to do that with meson.
> > meson test --suite regress runs all the regression tests.
> >
> > We talked this off-list at the conference. It seems we have to somehow
> > avoid passing pg_regress --schedule argument and instead pass the list of
> > tests. Any idea how to do that?
>
> I think there are 2 solutions to this.
>
> 1. Avoid passing --schedule by default, which doesn't sound like a great
>solution.
>
> 2. Teach pg_regress to ignore the --schedule option if specific tests
>are passed instead.
>
> 3. Add a --no-schedule option to pg_regress which would override the
>previously added --schedule option.
>
> I personally prefer 2 or 3.
>
> 2: meson test -C build regress/regress --test-args my_specific_test
> 3: meson test -C build regress/regress --test-args "--no-schedule 
> my_specific_test"
>
> Does anyone have an opinion?
>

if each individual sql file can be tested separately, will
`test: test_setup`?
be aslo executed as a prerequisite?



in
# --
# The first group of parallel tests
# --
test: boolean char name varchar text int2 int4 int8 oid float4 float8
bit numeric txid uuid enum money rangetypes pg_lsn regproc

# --
# The second group of parallel tests
# multirangetypes depends on rangetypes
# multirangetypes shouldn't run concurrently with type_sanity
# --
test: strings md5 numerology point lseg line box path polygon circle
date time timetz timestamp timestamptz interval inet macaddr macaddr8
multirangetypes


If we can test each individual sql file via meson, that would be great.
but based on the above comments, we need to preserve the specified order.
like:
TEST=rangetypes, multirangetypes

will first run rangetypes then multirangetypes.


can we tag/name each src/test/regress/parallel_schedule groups
like:
group1:test: boolean char name varchar text int2 int4 int8 oid float4
float8 bit numeric txid uuid enum money rangetypes pg_lsn regproc
group2:test: strings md5 numerology point lseg line box path polygon
circle date time timetz timestamp timestamptz interval inet macaddr
macaddr8 multirangetypes

Then, we can test each individual group.
TEST=group1, group2.




Extension security improvement: Add support for extensions with an owned schema

2024-06-01 Thread Jelte Fennema-Nio
Writing the sql migration scripts that are run by CREATE EXTENSION and
ALTER EXTENSION UPDATE are security minefields for extension authors.
One big reason for this is that search_path is set to the schema of the
extension while running these scripts, and thus if a user with lower
privileges can create functions or operators in that schema they can do
all kinds of search_path confusion attacks if not every function and
operator that is used in the script is schema qualified. While doing
such schema qualification is possible, it relies on the author to never
make a mistake in any of the sql files. And sadly humans have a tendency
to make mistakes.

This patch adds a new "owned_schema" option to the extension control
file that can be set to true to indicate that this extension wants to
own the schema in which it is installed. What that means is that the
schema should not exist before creating the extension, and will be
created during extension creation. This thus gives the extension author
an easy way to use a safe search_path, while still allowing all objects
to be grouped together in a schema. The implementation also has the
pleasant side effect that the schema will be automatically dropped when
the extension is dropped.

One way in which certain extensions currently hack around the
non-existence of this feature is by using the approach that pg_cron
uses: Setting the schema to pg_catalog and running "CREATE SCHEMA
pg_cron" from within the extension script. While this works, it's
obviously a hack, and a big downside of it is that it doesn't allow
users to choose the schema name used by the extension.

PS. I have never added fields to pg_catalag tables before, so there's
a clear TODO in the pg_upgrade code related to that. If anyone has
some pointers for me to look at to address that one that would be
helpful, if not I'll probably figure it out myself. All other code is
in pretty finished state, although I'm considering if
AlterExtensionNamespace should maybe be split a bit somehow, because
owned_schema skips most of the code in that function.
From cca2fc5a820822a29d9bc3b810a811adb1d081e1 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio 
Date: Fri, 31 May 2024 02:04:31 -0700
Subject: [PATCH v1] Add support for extensions with an owned schema

Writing the sql migration scripts that are run by CREATE EXTENSION and
ALTER EXTENSION UPDATE are security minefields for extension authors.
One big reason for this is that search_path is set to the schema of the
extension while running these scripts, and thus if a user with lower
privileges can create functions or operators in that schema they can do
all kinds of search_path confusion attacks if not every function and
operator that is used in the script is schema qualified. While doing
such schema qualification is possible, it relies on the author to never
make a mistake in any of the sql files. And sadly humans have a tendency
to make mistakes.

This patch adds a new "owned_schema" option to the extension control
file that can be set to true to indicate that this extension wants to
own the schema in which it is installed. What that means is that the
schema should not exist before creating the extension, and will be
created during extension creation. This thus gives the extension author
an easy way to use a safe search_path, while still allowing all objects
to be grouped together in a schema. The implementation also has the
pleasant side effect that the schema will be automatically dropped when
the extension is dropped.
---
 doc/src/sgml/extend.sgml  |  13 ++
 doc/src/sgml/ref/create_extension.sgml|   2 +-
 src/backend/commands/extension.c  | 139 +-
 src/backend/utils/adt/pg_upgrade_support.c|   1 +
 src/include/catalog/pg_extension.h|   1 +
 src/include/commands/extension.h  |   4 +-
 .../expected/test_extensions.out  |  50 +++
 src/test/modules/test_extensions/meson.build  |   4 +
 .../test_extensions/sql/test_extensions.sql   |  27 
 .../test_ext_owned_schema--1.0.sql|   2 +
 .../test_ext_owned_schema.control |   5 +
 ...test_ext_owned_schema_relocatable--1.0.sql |   2 +
 .../test_ext_owned_schema_relocatable.control |   4 +
 13 files changed, 214 insertions(+), 40 deletions(-)
 create mode 100644 src/test/modules/test_extensions/test_ext_owned_schema--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_owned_schema.control
 create mode 100644 src/test/modules/test_extensions/test_ext_owned_schema_relocatable--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_owned_schema_relocatable.control

diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5ce..36dc692abef 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -809,6 +809,19 @@ RETURNS anycompatible AS ...
   
  
 
+ 
+  owned_schema (boolean)
+  
+   
+An extension is 

Fix grammar oddities in comments

2024-06-01 Thread James Coleman
Hi,

See attached for a small patch fixing some typos and grammatical
errors in a couple of comments.

Side note: It's not clear to me what "Vars of higher levels don't
matter here" means in this context (or how that claim is justified),
but I haven't changed that part of the comment opting to simply
resolve the clear mistakes in the wording here.

Regards,
James Coleman


v1-0001-Fix-comment-grammar-oddities.patch
Description: Binary data


Re: meson and check-tests

2024-06-01 Thread Tristan Partin

On Fri May 31, 2024 at 12:02 PM CDT, Ashutosh Bapat wrote:

Hi Tristan,
Using make I can run only selected tests under src/test/regress using
TESTS=... make check-tests. I didn't find any way to do that with meson.
meson test --suite regress runs all the regression tests.

We talked this off-list at the conference. It seems we have to somehow
avoid passing pg_regress --schedule argument and instead pass the list of
tests. Any idea how to do that?


I think there are 2 solutions to this.

1. Avoid passing --schedule by default, which doesn't sound like a great 
  solution.


2. Teach pg_regress to ignore the --schedule option if specific tests 
  are passed instead.


3. Add a --no-schedule option to pg_regress which would override the 
  previously added --schedule option.


I personally prefer 2 or 3.

2: meson test -C build regress/regress --test-args my_specific_test
3: meson test -C build regress/regress --test-args "--no-schedule 
my_specific_test"

Does anyone have an opinion?

--
Tristan Partin
https://tristan.partin.io




Re: pltcl crashes due to a syntax error

2024-06-01 Thread Tom Lane
"a.kozhemyakin"  writes:
> When executing the following query on master branch:

> CREATE EXTENSION pltcl;
> CREATE or replace PROCEDURE test_proc5(INOUT a text)
>      LANGUAGE pltcl
>      AS $$
>      set aa [concat $1 "+" $1]
>      return [list $aa $aa])
>      $$;

> CALL test_proc5('abc');
> CREATE EXTENSION
> CREATE PROCEDURE
> server closed the connection unexpectedly

Replicated here.  I'll look into it later if nobody beats me
to it.  Thanks for the report!

regards, tom lane




Re: ResourceOwner refactoring

2024-06-01 Thread Alexander Lakhin

Hello Heikki,

I've stumbled upon yet another anomaly introduced with b8bff07da.

Please try the following script:
SELECT 'init' FROM pg_create_logical_replication_slot('slot',
  'test_decoding');
CREATE TABLE tbl(t text);
BEGIN;
TRUNCATE table tbl;

SELECT data FROM pg_logical_slot_get_changes('slot', NULL, NULL,
 'include-xids', '0', 'skip-empty-xacts', '1', 'stream-changes', '1');

On b8bff07da (and the current HEAD), it ends up with:
ERROR:  ResourceOwnerEnlarge called after release started

But on the previous commit, I get no error:
 data
--
(0 rows)

Best regards,
Alexander