Re: Allowing additional commas between columns, and at the end of the SELECT clause
Hi, As a developer, I love this feature. But as a developer of an universal TDOP SQL parser[1], this can be a pain. Please request it to the standard. Regards, Étienne [1]: https://gitlab.com/dalibo/transqlate
Re: Security lessons from liblzma - libsystemd
Hi, > There are many more interesting and scary libraries in the dependency > tree of "postgres", so just picking off one right now doesn't really > accomplish anything. The next release of libsystemd will drop all > the compression libraries as hard dependencies, so the issue in that > sense is gone anyway. Also, fun fact: liblzma is also a dependency > via libxml2. Having an audit of all libraries linked to postgres and their level of trust should help to point the next weak point. I'm pretty sure we have several of these tiny libraries maintained by a lone out of time hacker linked somewhere. What is the next xz ? Regards, Étienne -- DALIBO
REASSIGN pg_auth_members
Hi, I encounter a new behaviour in Postgres 16: DROP OWNED BY drops memberships when dropping grantor of memberhsip. A call on REASSIGN OWNED BY does not reassign membership from grantor to target owner. Attached is a script my-reassign.sql which reproduce the behaviour. Just run it with psql -f to reproduce. If you get *MEMBERSHIP LOST!!* message, then the DROP OWNED BY "unexpectedly" dropped the membership. This script run smoothly on Postgres 15. Membership is survives drop of grantor. What do you think of this ? How to change grantor of memberships before dropping the grantor role ? Should we fix REASSIGN to change grantor in pg_auth_members ? Regards, Étienne BERSAC Dalibo my-reassign.sql Description: application/sql
Re: REVOKE FROM warning on grantor
Hi, > ldap2pg really ought to issue REVOKE x FROM y GRANTED BY z. Thanks for this. I missed this notation and it is exactly what I need. You could consider this subject as closed. Thanks for your time and explanation. Regards, Étienne
Re: REVOKE FROM warning on grantor
Hi, > https://dev.mysql.com/doc/refman/8.0/en/revoke.html documents an "IF > EXISTS" option whose documentation reads, in relevant part, > "Otherwise, REVOKE executes normally; if the user does not exist, the > statement raises an error." > > https://community.snowflake.com/s/article/Access-Control-Error-Message-When-Revoking-a-Non-existent-Role-Grant-From-a-Role-or-User > is kind of interesting. It says that such commands used to fail with > an error but that's been changed; now they don't. It's not about inexistant user. It's not about inexistant membership. It's about membership you are not allowed to revoke. ldap2pg goals is to revoke spurious privileges. If ldap2pg find a spurious membership, it revokes it. Postgres 16 does not revoke some membership revoked before, and does not fail. The usual case is: a superuser grants writers role to alice. In directory, alice is degraded to readers. ldap2pg is not superuser but has CREATEROLE. ldap2pg applies the changes. In Postgres 15, revocation is completed. In Postgres 16, alice still has writers privileges and ldap2pg is not aware of this without clunky checks. Do you see a security concern here ? Regards, Étienne
Re: REVOKE FROM warning on grantor
Hi Tom, Thanks for your anwser. > It does not say that that set must be nonempty. Admittedly it's not > very clear from this one point. However, if you look around in the > standard it seems clear that they expect no-op revokes to be no-ops > not errors. Postgres actually identifies memberhips to revoke. The list is not empty. Event if revoker has USAGE privilege on parent role, the membership is protected by a new check on grantor of membership. This is a new semantic for me. I guess this may obfuscate other people too. I would compare denied revoking of role with revoking privilege on denied table: > REVOKE SELECT ON TABLE toto FROM PUBLIC ; ERROR: permission denied for table toto > Even taking the position that this is an unspecified point that we > could implement how we like, I don't think there's a sufficient > argument for changing behavior that's stood for a couple of decades. In Postgres 15, revoking a membership granted by another role is accepted. I suspect this is related to the new CREATEROLE behaviour implemented by Robert Haas (which is great job anyway). Attached is a script to reproduce. Here is the output on Postgres 15: SET DROP ROLE DROP ROLE DROP ROLE CREATE ROLE CREATE ROLE CREATE ROLE GRANT ROLE SET REVOKE ROLE DO Here is the output of the same script on Postgres 16: SET DROP ROLE DROP ROLE DROP ROLE CREATE ROLE CREATE ROLE CREATE ROLE GRANT ROLE SET psql:ldap2pg/my-revoke.sql:12: WARNING: role "r" has not been granted membership in role "g" by role "m" REVOKE ROLE psql:ldap2pg/my-revoke.sql:18: ERROR: REVOKE failed CONTEXTE : PL/pgSQL function inline_code_block line 4 at RAISE Can you confirm this ? Regards, Étienne my-revoke.sql Description: application/sql
Re: REVOKE FROM warning on grantor
Hi David, > That should have read: the granted permission does not exist Thanks, its clear. However, I'm hitting the warning when removing a role from a group. But the membership remains after the warning. In this case, I expect an error. I'll try to patch the behaviour to ensure an error if the REVOKE is ineffective. Regards, Étienne
Re: REVOKE FROM warning on grantor
Hi David, Thanks for your answer. > The choice of warning is made because after the command ends the > grantmin question does not exist. The revoke was a no-op and the > final state is as the user intended. Sorry, can you explain me what's the grantmin question is ? Regards, Étienne
REVOKE FROM warning on grantor
Hi, Since ldap2pg 6, I'm working on running by default as non-super role with CREATEDB. Robert Haas made this a viable solution as of Postgres 16. I got a case where ldap2pg tries to remove a role from a group. But ldap2pg user is not the grantor of this membership. This triggers a warning: $ REVOKE owners FROM alice; WARNING: role "alice" has not been granted membership in role "owners" by role "ldap2pg" I'll add a condition on grantor when listing manageable membership to simply avoid this. However, I'd prefer if Postgres fails properly. Because the GRANT is actually not revoked. This prevent ldap2pg to report an issue in handling privileges on such roles. What do you think of make this warning an error ?
Re: RFC: Logging plan of the running query
> Hi,
>
> If we use client log message, that message is shown on the target
> process whose pid is specified by the parameter of
> pg_log_query_plan():
>
> (pid:1000)=# select pg_sleep(60);
> (pid:1001)=# select pg_log_query_plan(1000);
> (pid:1000)=# LOG: query plan running on backend with PID 1000 is:
> Query Text: select pg_sleep(1000);
> Result (cost=0.00..0.01 rows=1 width=4)
> Output: pg_sleep('1000'::double precision)
>
> I think this is not an expected behavior and we set elevel to
> LOG_SERVER_ONLY.
Makes sens. Thanks.
Re: run pgindent on a regular basis / scripted manner
Hello, > Yes, there's a lot to look out for, and you're a damn sight better at > it > than I am. But we should try to automate the things that can be > automated, even if that leaves many tasks that can't be. I have three > things in my pre-commit hook: a check for catalog updates, a check > for > new typedefs, and an indent check. Could you share your configuration ? Could we provide more helper and integration to help produce consistent code ? For logfmt extension, I configured clang-formatd so that Emacs format the buffer on save. Any editor running clangd will use this. This is ease my mind about formatting. I need to investigate how to use pgindent instead or at lease ensure clang-format produce same output as pgindent. https://gitlab.com/dalibo/logfmt/-/blob/0d808b368e649b23ac06ce2657354b67be398b21/.clang-format Automate nitpicking in CI is good, but checking locally before sending the patch will save way more round-trip. Regards, Étienne
Re: RFC: Logging plan of the running query
Hi Torikoshia, > If so, we once tried to implement such function for getting memory > contexts. > However, this attempt didn't succeed because it could lead dead lock > situation[1]. Thanks for the pointer. Why not use client log message to allow client to get plan without locking backend memory context and without access to server log ? I missed the rationnal for not sending the plan to client. Regards, Étienne
Re: RFC: Logging plan of the running query
> Hi, > > Yeah, and when we have a situation where we want to run > pg_log_query_plan(), we can run it in any environment as long as it > is bundled with the core. Is it possible to get the plan of a backend programmatically without access to the logs? Something like pg_get_query_plan(pid, format) which output would be the same as EXPLAIN. Regards, Étienne BERSAC Dalibo
Re: logfmt and application_context
Hi, Le mercredi 27 septembre 2023 à 10:14 +0200, Daniel Gustafsson a écrit : > Being a common format in ingestion tools makes it interesting though, but I > wonder if those tools aren't alreday supporting CSV such that adding logfmt > won't move the compatibility markers much? Compared to CSV, logfmt has explicit named fields. This helps tools to apply generic rules like : ok this is pid, this is timestamp, etc. without any configuration. Loki and Grafana indexes a subset of known fields. This is harder to achieve with a bunch a semi-colon separated values. Compared to JSON, logfmt is terser and easier for human eyes and fingers. This is why I think logfmt for PostgreSQL could be a good option. Regards, Étienne
Re: logfmt and application_context
Hi Daniel, Thanks for the feedback. Le mardi 05 septembre 2023 à 11:35 +0200, Daniel Gustafsson a écrit : > > On 30 Aug 2023, at 14:36, Étienne BERSAC wrote: > > > ..what do you think of having logfmt output along json and CSV ? > > Less ideal is > that there is no official formal definition of what logfmt is [...] If we add > support for it, can we reasonably expect that what we emit is what consumers > of > it assume it will look like? I didn't know logfmt had variation. Do you have a case of incompatibility ? Anyway, I think that logfmt will be better handled inside Postgres rather than in an extension due to limitation in syslogger extendability. I could send a patch if more people are interested in this. What do you think about application_context as a way to render e.g. a web request UUID to all backend log messages ? Regards, Étienne
logfmt and application_context
Hi everyone, I just release a logfmt log collector for PostgreSQL : https://pgxn.org/dist/logfmt/1.0.0/ . This works quite well but I have a few issues I would like to share with hackers. First, what do you think of having logfmt output along json and CSV ? PostgreSQL internal syslogger has builtin support for the different LOG_DESTINATION_*. Thus logfmt does not send log collector headers using write_syslogger_file or write_pipe_chunks but plain log line with write_console. Do you have some hint about this ? The consequences ? How much is it a good bet to write a custom log collector in a shared preload library ? Second issue, logfmt provides a guc called `logfmt.application_context`. The purpose of application_context is the same as `application_name` but for a more varying value like request UUID, task ID, etc. What do you think of this ? Would it be cool to have this GUC in PostgreSQL and available in log_line_prefix ? Anyway, it's my first attempt at writing C code for PostgreSQL, with the help of Guillaume LELARGE and Jehan-Guillaume de RORTHAIS and it's a pleasure ! PostgreSQL C code is very readable. Thanks everyone for this ! Regards, Étienne BERSAC Developer at Dalibo
