[HACKERS] (spelling) Ensure header of postgresql.auto.conf is consistent

[Feike Steenbergen]

Attached a patch that ensures the header of postgresql.auto.conf is
consistent, whether created by initdb or recreated when ALTER SYSTEM
is issued.

The tiny difference caused some false-positives on our configuration
management identifying changes, which was enough of an itch for me to
scratch.

regards,

Feike Steenbergen


0001-Make-header-of-postgresql.conf.auto-consistent.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Document pgstattuple privileges without ambiguity

[Feike Steenbergen]

Hi,

When installing pgstattuple on 10, the documentation about its
privileges was unclear to me. (Does the pg_stat_scan_tables role get
EXECUTE privileges by default or not?).

By making the privilege paragraph less verbose and a duplicate of the
paragraph used for pgfreespacemap and pgbuffercache we remove the
ambiguity and make the documentation more uniform.

The replacement paragrahp is much less verbose and loses some detailed
pointers (to GRANT syntax), but in this instance I feel less is more.

Regards,

Feike


pgstattuple_privilege_documentation_v1.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] [BUGS] BUG #14600: Passwords in user mappings leaked by psql \deu+ command

[Feike Steenbergen]

Forwarding message from pgsql-bugs for review


Attached a patch which copies the logic from commit
93a6be63a55a8cd0d73b3fa81eb6a46013a3a974.

In the current implementation we only consider privileges of the foreign
server
in determining whether or not to show the user mapping details. This patch
copies the same logic (and documentation) used in commit
93a6be63a55a8cd0d73b3fa81eb6a46013a3a974 to not always show the user mapping
options.

regards,

Feike


user_mappings_leak.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Support for pg_receivexlog --post-segment command

[Feike Steenbergen]

On 6 January 2017 at 15:42, Magnus Hagander  wrote:

> Is there actual value in providing both %p and %f? It's not like it's
really hard to do, but since the path will be specified on the same
commandline, you could just put it in the command?

As %f can be determined from %p I don't mind that much. However, having a
single static --whatever command may be very useful for configuration
management or backup tools that want to use static commands.

The other reason why I'd offer both is to have some uniformity with
archive_command, possibly allowing some reuse of code.

Re: [HACKERS] Support for pg_receivexlog --post-segment command

[Feike Steenbergen]

On Fri, Jan 6, 2017 at 2:30 PM, David Steele  wrote:
> For my part I still prefer an actual command to be executed so it will
start/restart the archiver if it is not already running or died.  This
reduces the number of processes that I need to ensure are running.
>
> If the consensus is that a signal is better then I'll make that work.  I
will say this raises the bar on what is required to write a good archive
command and we already know it is quite a difficult task.

On 6 January 2017 at 14:37, Magnus Hagander  wrote:
> I like the idea of a command as well, for flexibility. If you want a
signal, you can write a trivial command that sends the signal... Maximum
flexibility, as long as we don't create a lot of caveats for users.

Agreed, I think it is also easier to understand the mechanism (instead of a
signal), and would allow for some reuse of already existing scripts.

If we do use a full command (vs a signal), I propose we do also offer the
%p and %f placeholders for the command.

Re: [HACKERS] Support for pg_receivexlog --post-segment command

[Feike Steenbergen]

On 6 January 2017 at 13:50, Magnus Hagander  wrote:
> I think we're better off clearly documenting that we don't care about it.
And basically let the external command be responsible for that part.

> So for example, your typical backup manager would listen to this signal
or whatever to react quickly. But it would *also* have some sort of
fallback. For example, whenever it's triggered it also checks if there are
any previous segments it missed (this would also cover the startup
sequence).

For me this works fine. I just want to ensure that if there is any work to
be done, my backup manager will do the work quickly. My implementation
might be very simply a process that checks every n seconds or when
signalled.

> Since we never actually remove anything (unlike archive_command which has
the integration with wal segment rotation), I think this can be done
perfectly safe.
>
> Looking at the usecases where you have been doing it, are there any where
this would not work?

This would work for all usecases I've come across.

[HACKERS] Support for pg_receivexlog --post-segment command

[Feike Steenbergen]

Hi all,

When reading through "Support for pg_receivexlog --format=plain|tar"[1], I
came across a notion from Magnus Hagander that has crossed my mind a few
times as well in the past years. As the feature proposed here is not
directly related to that thread, I thought it best to start a new thread to
discuss.

> I have been talking to David about it a couple of times, and he agreed
that it'd be useful to have a post-segment command. We haven't discussed it
in much detail though. I'll add him to direct-cc here to see if he has any
further input :)

I'm coming across a few usecases where this would seem very useful. I'm
looking to push finished segments to some api as soon as possible. Having
the post-segment command would allow me to get there. I've tried the
following approaches, none of them are very satisfying however:

- periodic checking of new files (cron)
- using inotify
- tailing verbose pg_receivexlog output to see when a segment was switched

> It could be that the best idea is to just notify some other process of
what's happening. But making it an external command would give that a lot
of flexibility. Of course, we need to be careful not to put ourselves back
in the position we are in with archive_command, in that it's very difficult
to write a good one.

A signal for  would be good enough for my use case, I don't necessarily
need all the bookkeeping to ensure the post-segment command was finished
successfully. However I can see people expecting similar behaviour for the
post-segment command as for the archive_command. If we would use an
external command, does this also imply that we need some bookkeeping around
which segments are ready and done, similar to what is done on the server in
the archive_status directory?

Thanks,

Feike

[1]
https://www.postgresql.org/message-id/CAB7nPqTEVXjtH+fehcCbP791H71cfLN_p9rrd-h=ymjfshz...@mail.gmail.com

Re: [HACKERS] pg_basebackups and slots

[Feike Steenbergen]

> but -X stream is, then we use a temporary slot always.
This seems even more useful with -X fetch to me, as with fetch we are sure
we
are not immediately receiving the WAL. So, I'd propose to use it whenever -X
is specified, regardless of which method is specified.

> (I still think we should change the default here, but that's a different
topic)
+1

> Does that seem reasonable? Or would people prefer it to default to off?
Yes. Users are specifically requesting wal using -X, so making sure that
actually happens by default would work.

Re: [HACKERS] Parallel indicators not written by pg_get_functiondef

[Feike Steenbergen]

This patch is redundant as of
commit 2ac3be2e763d9b971352819f285dd51519e0aeb9

(Ashutosh Sharma diagnosed and patched the same problem)

On 15 April 2016 at 14:13, Michael Paquier <michael.paqu...@gmail.com>
wrote:

> On Fri, Apr 15, 2016 at 8:48 PM, Feike Steenbergen
> <feikesteenber...@gmail.com> wrote:
> > pg_get_functiondef(oid) does not return the PARALLEL indicators.
> >
> > Attached a small patch to have pg_get_functiondef actually add these
> > indicators, using commit 7aea8e4f2 (pg_dump.c) as a guide.
> >
> > The logic is the same as with the volatility: we only append non-default
> > indicators.
>
> +1 for a good catch. Your patch is correct.
> --
> Michael
>

[HACKERS] Parallel indicators not written by pg_get_functiondef

[Feike Steenbergen]

pg_get_functiondef(oid) does not return the PARALLEL indicators.

Attached a small patch to have pg_get_functiondef actually add these
indicators, using commit 7aea8e4f2 (pg_dump.c) as a guide.

The logic is the same as with the volatility: we only append non-default
indicators.

Feike Steenbergen


add_parallel_indicators_to_get_functiondef.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] SuperUser check in pg_stat_statements

[Feike Steenbergen]

You can create a Security Definer Funtion which can then be executed by
then non-superuser monitoring role:

(Assuming you have a role monitoring and pg_stat_statements is installed in
schema public)

-- connected as a superuser
CREATE FUNCTION pg_stat_statements()
RETURNS SETOF pg_stat_statements
LANGUAGE SQL
SET search_path='public'
SECURITY DEFINER
AS
$BODY$
SELECT *
  FROM pg_stat_statements;
$BODY$;

REVOKE ALL ON FUNCTION pg_stat_statements() FROM public;
GRANT EXECUTE ON FUNCTION pg_stat_statements() TO monitoring;

-- connected as monitoring
SELECT * FROM pg_stat_statements();

Re: [HACKERS] Add regression tests for autocommit-off mode for psql and fix some omissions

[Feike Steenbergen]

Apologies for the previous message, I didn't send the full version.


On 6 October 2014 16:01, Tom Lane t...@sss.pgh.pa.us wrote:
 What class of bug would that prevent exactly?

ERROR: [...] cannot run inside a transaction block

when:
- running psql in AUTOCOMMIT off
- not having started a transaction yet

Currently some statements (ALTER TYPE name ADD VALUE, DROP INDEX CONCURRENTLY)
can only be run in psql when enabling autocommit
(which I consider a bug - either in the code, or in the documentation),
whilst many others (VACUUM, CREATE DATABASE) can be run in AUTOCOMMIT
off because
they will not implicitly create a transaction in psql.

 It seems to me like
 something that would normally get forgotten when we add any new
 such statement.

I think that is probably true; it has already been forgotten to be added
to psql for a few commands.
Perhaps I am the only one using autocommit-off mode and we shouldn't put effort
into fixing this?

For me the reason to add some tests was to make sure that the current behaviour
will not change in future versions; the function command_no_begin might be added
to, modified, or rewritten.



On 7 October 2014 01:41, Jim Nasby jim.na...@bluetreble.com wrote:
 The options I see...

 1) If there's a definitive way to tell from backend source code what
 commands disallow transactions then we can just use that information to
 generate the list of commands psql shouldn't do that with.

 2) Always run the regression test with auto-commit turned off.

 3) Run the regression in both modes (presumably only on the build farm due
 to how long it would take).


1) I don't know about a definitive way. I used grep to find all
   statements calling PreventTransactionChain.

2) - I expect most people use autocommit-on; so only running it in
 autocommit-off would not test the majority of users.
   - autocommit-off also obliges you to explicitly rollback transactions after
errors occur; this would probably mean a rewrite of some tests?

kind regards,

Feike Steenbergen


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Add regression tests for autocommit-off mode for psql and fix some omissions

[Feike Steenbergen]

On 7 October 2014 09:55, Marko Tiikkaja ma...@joh.to wrote:
 It's not clear to me that this is fixing a problem, to be honest.  If you're
 running autocommit=off, you have an expectation that you can roll back
 commands at will.  It's fine if I can't roll back a VACUUM, for example,
 since I would practically never want to do that.  But  ALTER TYPE .. ADD
 VALUE ..;  is an entirely different beast.  That one's permanent; there's no
 DROP equivalent.  If the command is just executed, and I can't roll it back,
 wouldn't that be a serious violation of the principle of least astonishment?

I think you have a valid and good point; however the autocommit-off mode can
currently already execute statements which cannnot be rolled back.
Perhaps it is a good idea to not allow any of these statements in autocommit-off
mode to prevent astonishement from users, but that would be a discussion of
itself.

My reason for proposing this is to have all these commands treated
consistently.
The expectation of being able to roll back commands at will cannot be fulfilled
currently, many statemens that are allowed with autocommit-off fall into the
category different beast.

Currently the following statemens call PreventTransactionChain and do not
generate errors in autocommit-off mode:
- REINDEX DATABASE
- CREATE INDEX CONCURRENTLY
- ALTER SYSTEM
- CREATE DATABASE
- DROP DATABASE
- CREATE TABLESPACE
- DROP TABLESPACE
- CLUSTER
- VACUUM

The following statements call PreventTransactionChain and do generate errors
in autocommit-off mode:
- DROP INDEX CONCURRENTLY
- ALTER DATABASE ... SET TABLESPACE
- ALTER TYPE ... ADD

I don't see why these last three should be treated seperately from the
first list; we should
either allow all, or none of these statements IMHO.

kind regards,

Feike Steenbergen

On 7 October 2014 09:55, Marko Tiikkaja ma...@joh.to wrote:
 On 10/7/14, 9:11 AM, Feike Steenbergen wrote:

 Perhaps I am the only one using autocommit-off mode


 You most definitely aren't.

 and we shouldn't put effort
 into fixing this?


 It's not clear to me that this is fixing a problem, to be honest.  If you're
 running autocommit=off, you have an expectation that you can roll back
 commands at will.  It's fine if I can't roll back a VACUUM, for example,
 since I would practically never want to do that.  But  ALTER TYPE .. ADD
 VALUE ..;  is an entirely different beast.  That one's permanent; there's no
 DROP equivalent.  If the command is just executed, and I can't roll it back,
 wouldn't that be a serious violation of the principle of least astonishment?
 DROP INDEX CONCURRENTLY has a bit of the same problem.  You can CREATE INDEX
 CONCURRENTLY, but it might take days in some cases.

 I think that just running the command is a bad idea, and if we want to fix
 something here we should focus on just providing a better error message.


 .marko


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Add regression tests for autocommit-off mode for psql and fix some omissions

[Feike Steenbergen]

On 6 October 2014 14:09, Michael Paquier michael.paqu...@gmail.com wrote:
 That's a good catch and it should be a separate patch. This could even be
 considered for a back-patch down to 9.2. Thoughts?

If I isolate DROP INDEX concurrently, this patch would do the trick.


20141006_drop_index_concurrently.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Add regression tests for autocommit-off mode for psql and fix some omissions

[Feike Steenbergen]

It would test that when setting AUTOCOMMIT to off that you will not run into:

ERROR: [...] cannot run inside a transaction block

when issuing one of these PreventTransactionChain commands. In
src/bin/psql/common.c


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers