Re: meson and check-tests
"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
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
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
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
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
"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
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