[PATCH] Use role name "system_user" instead of "user" for unsafe_tests
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t47724psql -h localhost -U postgresBuilt from patchset v1 (message #1), September 20, 2026 at 05:19 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t47724_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t47724_1 && git checkout t47724_1Patchset v1 (message #1) is on t47724_1
Hi,
While playing with a new single board computer (VisionFive 2) I
discovered that postgresql:unsafe_tests suite fails like this:
```
--- /home/user/projects/postgresql/src/test/modules/unsafe_tests/expected/rolenames.out
2023-04-11 14:58:57.844550612 +0000
+++ /home/user/projects/postgresql/build/testrun/unsafe_tests/regress/results/rolenames.out
2023-04-11 17:54:22.999024391 +0000
@@ -53,6 +53,7 @@
CREATE ROLE "current_user";
CREATE ROLE "session_user";
CREATE ROLE "user";
+ERROR: role "user" already exists
RESET client_min_messages;
CREATE ROLE current_user; -- error
ERROR: CURRENT_USER cannot be used as a role name here
@@ -1089,4 +1090,5 @@
DROP OWNED BY regress_testrol0, "Public", "current_role",
"current_user", regress_testrol1, regress_testrol2, regress_testrolx
CASCADE;
DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2,
regress_testrolx;
DROP ROLE "Public", "None", "current_role", "current_user",
"session_user", "user";
+ERROR: current user cannot be dropped
DROP ROLE regress_role_haspriv, regress_role_nopriv;
```This happens because the developers of this SBC choose the default
username "user", which I had no reason to change.
Test merely checks that we can distinguish a username "user" from the
USER keyword. Maybe it's worth replacing "user" with "system_user"? It
is also a keyword but is a less likely choice for the OS user name.
--
Best regards,
Aleksander Alekseev
On 2023-04-11 Tu 14:25, Aleksander Alekseev wrote:
Hi,
While playing with a new single board computer (VisionFive 2) I
discovered that postgresql:unsafe_tests suite fails like this:``` --- /home/user/projects/postgresql/src/test/modules/unsafe_tests/expected/rolenames.out 2023-04-11 14:58:57.844550612 +0000 +++ /home/user/projects/postgresql/build/testrun/unsafe_tests/regress/results/rolenames.out 2023-04-11 17:54:22.999024391 +0000 @@ -53,6 +53,7 @@ CREATE ROLE "current_user"; CREATE ROLE "session_user"; CREATE ROLE "user"; +ERROR: role "user" already exists RESET client_min_messages; CREATE ROLE current_user; -- error ERROR: CURRENT_USER cannot be used as a role name here @@ -1089,4 +1090,5 @@ DROP OWNED BY regress_testrol0, "Public", "current_role", "current_user", regress_testrol1, regress_testrol2, regress_testrolx CASCADE; DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2, regress_testrolx; DROP ROLE "Public", "None", "current_role", "current_user", "session_user", "user"; +ERROR: current user cannot be dropped DROP ROLE regress_role_haspriv, regress_role_nopriv; ```This happens because the developers of this SBC choose the default
username "user", which I had no reason to change.Test merely checks that we can distinguish a username "user" from the
USER keyword. Maybe it's worth replacing "user" with "system_user"? It
is also a keyword but is a less likely choice for the OS user name.
I don't think we can protect against all possible user names. Wouldn't
it be better to run the tests under an OS user with a different name,
like "marmaduke"? ("user" is a truly terrible default user name).
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
Hi Andrew,
I don't think we can protect against all possible user names. Wouldn't it be better to run the tests under an OS user with a different name, like "marmaduke"? ("user" is a truly terrible default user name).
100% agree. The point is not to protect against all possible user
names but merely to reduce the likelihood of the problem. For this
particular test there is no difference which keyword to use for the
test. I realize this is a minor problem, however the fix is trivial
too.
--
Best regards,
Aleksander Alekseev
Aleksander Alekseev <aleksander@timescale.com> writes:
I don't think we can protect against all possible user names. Wouldn't it be better to run the tests under an OS user with a different name, like "marmaduke"? ("user" is a truly terrible default user name).
100% agree. The point is not to protect against all possible user
names but merely to reduce the likelihood of the problem.
It only reduces the likelihood if you assume that "system_user"
is less likely than "user" as a choice of OS user name to run
the tests under. That seems like a debatable assumption;
perhaps it's actually *more* likely.
Whether we need to have a test for this at all is perhaps a
more interesting argument.
regards, tom lane
Hi,
Whether we need to have a test for this at all is perhaps a
more interesting argument.
This was my initial thought but since somebody put it there I assumed
this is a very important test.
Any objections if we remove the tests for "user"?
--
Best regards,
Aleksander Alekseev
On Wed, Apr 12, 2023 at 03:30:03PM +0300, Aleksander Alekseev wrote:
Any objections if we remove the tests for "user"?
Based on some rather-recent experience in this area with
COERCE_SQL_SYNTAX, the relationship between the SQL keywords and the
way they can handled internally could be tricky if this area of the
code is touched. So I would choose to keep these tests, FWIW.
--
Michael
Hi,
On Wed, Apr 12, 2023 at 03:30:03PM +0300, Aleksander Alekseev wrote:
Any objections if we remove the tests for "user"?
Based on some rather-recent experience in this area with
COERCE_SQL_SYNTAX, the relationship between the SQL keywords and the
way they can handled internally could be tricky if this area of the
code is touched. So I would choose to keep these tests, FWIW.
Thanks for the feedback. I see this is a controversial topic so in the
interest of saving our time I'm withdrawing the patch.
--
Best regards,
Aleksander Alekseev