Re: [HACKERS] remove wal_level archive

2015-11-02 Thread Craig Ringer
On 2 November 2015 at 14:19, Michael Paquier  wrote:

> pg_upgradecluster has some logic to switch a parameter value (see
> strrepl)

That's part of pg_wrapper, not core, though.

I'd quite like to see pg_wrapper become part of the PGDG RPMs, but
right now AFAIK it's a Debian-derived-only thing.

> and pg_upgrade does not handle parameter name switches by
> itself

Exactly.

> so the price to pay would be more maintenance annoyance for
> existing upgrade scripts, which happens at more or less each major
> release (checkpoint_segments removed in 9.5, unix_socket_directory
> renamed in 9.3, etc.).

Fair point. I'm not a great fan of how those changes affect users, but
I've also seen what too much backward compatibility can do to a
project. (Ref: Java, the Win32 APIs).

If users miss it then it won't explode anything in a way that's
dangerous or harmful, so I won't complain overly. I just wanted to
raise it as a possible concern.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Tom Lane
Andrew Dunstan  writes:
> I don't know if this was discussed at the time ALTER SYSTEM was 
> implemented, but I have just discovered that if postgresql.auto.conf is 
> a symlink to a file elsewhere, ALTER SYSTEM will happily break that link 
> and write its own local copy. That strikes me as rather unfriendly. Why 
> not just truncate the file rather than unlink it as it's being 
> rewritten? Even if we don't want to do that a warning in the docs might 
> help users avoid the "mistake" I just made.

Frankly, that behavior strikes me as a good idea.  There is no situation,
IMV, where it's sane to try to put a symlink there.

regards, tom lane


-- 
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Andrew Dunstan



On 11/02/2015 09:36 AM, Tom Lane wrote:

Andrew Dunstan  writes:

I don't know if this was discussed at the time ALTER SYSTEM was
implemented, but I have just discovered that if postgresql.auto.conf is
a symlink to a file elsewhere, ALTER SYSTEM will happily break that link
and write its own local copy. That strikes me as rather unfriendly. Why
not just truncate the file rather than unlink it as it's being
rewritten? Even if we don't want to do that a warning in the docs might
help users avoid the "mistake" I just made.

Frankly, that behavior strikes me as a good idea.  There is no situation,
IMV, where it's sane to try to put a symlink there.





OK. I was experimenting with some backup procedures, and I have since 
abandoned the approach I was following. However, it does seem like it's 
worth documenting the behaviour.


cheers

andrew


--
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] fortnight interval support

2015-11-02 Thread Andrew Dunstan



On 11/02/2015 09:20 AM, Merlin Moncure wrote:

On Sun, Nov 1, 2015 at 2:20 PM, David Fetter  wrote:

On Sun, Nov 01, 2015 at 01:06:39PM -0500, Tom Lane wrote:

David Fetter  writes:

On Tue, Oct 27, 2015 at 07:30:13PM +, Nathan Wagner wrote:

On Wed, Oct 28, 2015 at 08:17:25AM +1300, Gavin Flower wrote:
What, it's a "fortnight", not a "quinzaine".

You have no idea how hard it was to resist updating the patch...

Well, if you won't do it, I will.

Please tell me this is a joke.

Yes, it's a joke.


(FWIW, I don't have a problem with "fortnight", but I draw the line
at units that are not used in English.)

It's used in English, but not in this context.

https://en.wikipedia.org/wiki/Quinzaine

As to localization, I think we need to consider carefully whether
PostgreSQL is to be a US-only RDBMS with a few concessions to usage
elsewhere, or one usable by all the world's peoples

That baggage comes with the SQL Standard.  It's fun to think about a
functional equivalent of SQL that doesn't attempt to have statements
be grammatically correct English sentences but due to various human
pressures that isn't how things worked out.  Since I'm daydreaming,
let's have this hypothetical language implement relational division.

By the way, I fell for the joke.  Note, we do implement 'allballs' so
I'll take a pass.



I respectfully submit that there is a case for supporting lovers of Jane 
Austen (such as me!) by recognizing "se'nnight", with or without 
apostrophe,  as a synonym for "week".


cheers

andrew



--
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] pglogical_output - a general purpose logical decoding output plugin

2015-11-02 Thread Shulgin, Oleksandr
On Mon, Nov 2, 2015 at 1:17 PM, Craig Ringer  wrote:

> Hi all
>
> I'd like to submit pglogical_output for inclusion in the 9.6 series as
> a contrib.


Yay, that looks pretty advanced! :-)

Still digesting...

--
Alex


Re: [HACKERS] ALTER SYSTEM vs symlink

2015-11-02 Thread Andres Freund
On 2015-11-02 09:43:02 -0500, Stephen Frost wrote:
> What this request strikes me as asking for is the same as what I asked
> for when this feature was originally going in- there should be a way to
> disable it.

You can just revoke permissions on the file if necessary. Results in the
expected
ERROR:  XX000: could not open file "../postgresql.auto.conf": Permission denied


-- 
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Stephen Frost
* Andrew Dunstan (and...@dunslane.net) wrote:
> On 11/02/2015 09:36 AM, Tom Lane wrote:
> >Andrew Dunstan  writes:
> >>I don't know if this was discussed at the time ALTER SYSTEM was
> >>implemented, but I have just discovered that if postgresql.auto.conf is
> >>a symlink to a file elsewhere, ALTER SYSTEM will happily break that link
> >>and write its own local copy. That strikes me as rather unfriendly. Why
> >>not just truncate the file rather than unlink it as it's being
> >>rewritten? Even if we don't want to do that a warning in the docs might
> >>help users avoid the "mistake" I just made.
> >Frankly, that behavior strikes me as a good idea.  There is no situation,
> >IMV, where it's sane to try to put a symlink there.
> 
> OK. I was experimenting with some backup procedures, and I have
> since abandoned the approach I was following. However, it does seem
> like it's worth documenting the behaviour.

No problem with that, certainly.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] fortnight interval support

2015-11-02 Thread Merlin Moncure
On Sun, Nov 1, 2015 at 2:20 PM, David Fetter  wrote:
> On Sun, Nov 01, 2015 at 01:06:39PM -0500, Tom Lane wrote:
>> David Fetter  writes:
>> > On Tue, Oct 27, 2015 at 07:30:13PM +, Nathan Wagner wrote:
>> >> On Wed, Oct 28, 2015 at 08:17:25AM +1300, Gavin Flower wrote:
>> >> What, it's a "fortnight", not a "quinzaine".
>> >>
>> >> You have no idea how hard it was to resist updating the patch...
>>
>> > Well, if you won't do it, I will.
>>
>> Please tell me this is a joke.
>
> Yes, it's a joke.
>
>> (FWIW, I don't have a problem with "fortnight", but I draw the line
>> at units that are not used in English.)
>
> It's used in English, but not in this context.
>
> https://en.wikipedia.org/wiki/Quinzaine
>
> As to localization, I think we need to consider carefully whether
> PostgreSQL is to be a US-only RDBMS with a few concessions to usage
> elsewhere, or one usable by all the world's peoples

That baggage comes with the SQL Standard.  It's fun to think about a
functional equivalent of SQL that doesn't attempt to have statements
be grammatically correct English sentences but due to various human
pressures that isn't how things worked out.  Since I'm daydreaming,
let's have this hypothetical language implement relational division.

By the way, I fell for the joke.  Note, we do implement 'allballs' so
I'll take a pass.

merlin


-- 
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] pglogical_output - a general purpose logical decoding output plugin

2015-11-02 Thread Craig Ringer
On 2 November 2015 at 20:17, Craig Ringer  wrote:
> Hi all
>
> I'd like to submit pglogical_output for inclusion in the 9.6 series as
> a contrib.

Here's the protocol documentation discussed in the README. It's
asciidoc at the moment, so it can be formatted into something with
readable tables.

If everyone thinks it's reasonable to document the pglogical protocol
as part of the SGML docs then it can be converted. Since the walsender
protocol is documented in the public SGML docs it probably should be,
it's just a matter of deciding what goes where.

Thanks to Darko Milojković for the asciidoc conversion. All errors are
likely to be my edits.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
= Pg_logical protocol

This isn’t a whole new top-level TCP protocol - it uses the libpq and walsender 
protocols, rather than replacing them. It’s is a protocol within a protocol 
within a protocol.

This protocol is an inner layer in a stack:

 * tcp or unix sockets
 ** libpq protocol
 *** walsender COPY BOTH mode
  pg_logical output plugin => consumer protocol

This protocol has evolved out of the BDR protocol. For why changes were needed, 
see the document ++_Development plan_++ and ++_Rationale 
for protocol changes_++.

== ToC


== Notes for understanding the protocol documentation


When reading the protocol docs, note that:

 * The walsender IDENTIFY SYSTEM message exposes upstream’s:
 ** sysid
 ** tli (Timeline Identifier)
 ** xlogpos (LSN, or Log Sequence Number)
 ** logptr
 ** dbname (not db oid)
 * We can accept an arbitrary list of params to START LOGICAL REPLICATION. 
After that, the only information we can send from downstream to upstream is 
replay progress feedback messages.
== 
== 
== Protocol messages

The individual protocol messages are discussed in the following sub sections. 
Protocol flow and logic comes in the next major section.

Absolutely all top-level protocol messages begin with a message type byte. 
While represented in code as a character, this is a signed byte with no 
associated encoding.

Since the PostgreSQL libpq COPY protocol supplies a message length there’s no 
need for top-level protocol messages to embed a length in their header.
=== 
=== BEGIN message

A stream of rows starts with a BEGIN message. Rows may only be sent after a 
BEGIN and before a COMMIT.

|===
|*Message*|*Type/Size*|*Notes*

|Message type|signed char|Literal ‘**B**’ (0x42)
|flags|uint8| * 0-3: Reserved, client _must_ ERROR if set and not recognised.
|lsn|uint64|“final_lsn” in decoding context - currently it means lsn of commit
|commit time|uint64|“commit_time” in decoding context
|remote XID|uint32|“xid” in decoding context
|===

=== 
=== 
=== Forwarded transaction origin message

Sent if and only if the immediately prior message was a _BEGIN_ message with 
the FORWARDED_TRANSACTION flag set, the message after the BEGIN _must_ be a 
_forwarded transaction origin _message indicating what upstream host the 
transaction came from.

It is a protocol error to send/receive a forwarded transaction origin message 
at any other time.

The origin identifier is of arbitrary and application-defined format. 
Applications _should_ prefix their origin identifier with a fixed application 
name part, like _bdr__, _myapp__, etc. It is application-defined what an 
application does with forwarded transactions from other applications.

The origin identifier is typically closely related to replication slot names 
and replication origins’ names in an application system.

For more detail see _Changeset Forwarding_ in the README.


|===
|*Message*|*Type/Size*|*Notes*

|Message type|signed char|Literal ‘**O**’ (0x4f)
|flags|uint8| * 0-3: Reserved, application _must_ ERROR if set and not 
recognised
|origin_lsn|uint64|Log sequence number (LSN, XLogRecPtr) of the transaction’s 
commit record on its origin node (as opposed to the forwarding node’s commit 
LSN, which is ‘lsn’ in the BEGIN message)
|origin_identifier_length|uint8|Length in bytes of origin_identifier
|origin_identifier|signed char[origin_identifier_length]|An origin identifier 
of arbitrary, upstream-application-defined structure. _Should_ be text in the 
same encoding as the upstream database. NULL-terminated. _Should_ be 7-bit 
ASCII.
|===

== 
=== COMMIT message
A stream of rows ends with a COMMIT message.

There is no ROLLBACK message because aborted transactions are not sent by the 
upstream.


|===
|*Message*|*Type/Size*|*Notes*

|Message type|signed char|Literal ‘**C**’ (0x43)
|Flags|uint8| * 0-3: Reserved, client _must_ ERROR if set and not recognised
|Commit LSN|uint64|commit_lsn in decoding commit decode callback. This is the 
same value as in the BEGIN message, and marks the end of the transaction.
|End LSN|uint64|end_lsn in decoding transaction context
|Commit time|uint64|commit_time in decoding transaction context
|===


=== INSERT, UPDATE or 

Re: [HACKERS] ALTER SYSTEM vs symlink

2015-11-02 Thread Stephen Frost
* Andres Freund (and...@anarazel.de) wrote:
> On 2015-11-02 09:43:02 -0500, Stephen Frost wrote:
> > What this request strikes me as asking for is the same as what I asked
> > for when this feature was originally going in- there should be a way to
> > disable it.
> 
> You can just revoke permissions on the file if necessary. Results in the
> expected
> ERROR:  XX000: could not open file "../postgresql.auto.conf": Permission 
> denied

Yes, I know, but that's a really grotty way of offering a way to disable
ALTER SYSTEM.  It's also not exactly intuitive to someone reading the
release notes or working on upgrading their existing postgresql.conf.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] ALTER SYSTEM vs symlink

2015-11-02 Thread Tom Lane
Stephen Frost  writes:
> * Andres Freund (and...@anarazel.de) wrote:
>> You can just revoke permissions on the file if necessary. Results in the
>> expected
>> ERROR:  XX000: could not open file "../postgresql.auto.conf": Permission 
>> denied

> Yes, I know, but that's a really grotty way of offering a way to disable
> ALTER SYSTEM.  It's also not exactly intuitive to someone reading the
> release notes or working on upgrading their existing postgresql.conf.

While I won't stand in the way if someone is dead set on providing a
disable switch for ALTER SYSTEM, I fail to see the point of one.  It's
a superuser-only feature to begin with, and if you are handing out
superuser on production-critical installations to people you don't trust
completely, you need to have your head examined.

As a directly comparable example, I note that you yourself were in favor
of getting rid of rolcatupdate, which was the only mechanism we ever had
that could prevent a superuser from destroying the catalogs entirely
with a mistyped update --- consider "DELETE FROM pg_proc", for example,
which unlike ALTER SYSTEM there is simply no way to recover from.

How is it that we don't need rolcatupdate but we do need a way to shut
off ALTER SYSTEM?  Doesn't compute, IMO.

regards, tom lane


-- 
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] Patent warning about the Greenplum source code

2015-11-02 Thread Simon Riggs
On 1 November 2015 at 07:47, Bruce Momjian  wrote:

> On Sun, Nov  1, 2015 at 01:27:13AM -0500, Bruce Momjian wrote:
> > On Fri, Oct 30, 2015 at 04:47:35AM -0400, Bruce Momjian wrote:
> > > Therefore, I caution people from viewing the Greenplum source code as
> > > you might see patented ideas that could be later implemented in
> > > Postgres, opening Postgres up to increased patent violation problems.
> I
> > > am also concerned about existing community members who work for
> > > Pivotal/Greenplum and therefore are required to view the patented
> source
> > > code.  The license issue might eventually be improved by
> > > Pivotal/Greenplum, but, for now, I think caution is necessary.
> > >
> > > Of course, never mention known-patented ideas in any community forum,
> > > including this email list.
> >
> > I just found out that Citus Data has patent applications pending, so
> > viewing Citus Data source code has the same problems as Greenplum.
>
> Actually, it might only be their closed source software that contains
> patents, i.e. not pg_shard.  I will check and report back when I can
> unless someone else reports here first.


While you are doing that, please also check EnterpriseDB. My information is
that there are patents filed there, so we must check that just as much as
any other company or person. If you didn't know before, you do now.

I am disappointed that your approach to this appears unbalanced and
partisan. Worse, Greenplum have been quite vocal about their intentions, so
any feedback you have could easily have been given many months ago, not on
the day of their announcement. I think you should have declared this
situation in a very different way to the way you have approached this. 5
minutes thought on whether other companies might also have been affected
would have been sensible, plus the whole thing could have been discussed
completely offlist. If you do discuss things on-list then you should at
least state for the record that you are an EnterpriseDB employee when
discussing your concerns, since that is likely to have a material affect on
how this situation is viewed by anyone worried by your post.

For the record, I have no commercial relationship of any kind with
Greenplum, so I am an informed observer only.

Please say no more until you have a full set of information; I suggest you
discuss that privately with each person/company first, to give them time to
explain.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] Patent warning about the Greenplum source code

2015-11-02 Thread José Luis Tallón

On 11/02/2015 02:41 PM, Simon Riggs wrote:
On 1 November 2015 at 07:47, Bruce Momjian > wrote:


On Sun, Nov  1, 2015 at 01:27:13AM -0500, Bruce Momjian wrote:
> On Fri, Oct 30, 2015 at 04:47:35AM -0400, Bruce Momjian wrote:
> > Therefore, I caution people from viewing the Greenplum source
code as
> > you might see patented ideas that could be later implemented in
> > Postgres, opening Postgres up to increased patent violation
problems.  I
> > am also concerned about existing community members who work for
> > Pivotal/Greenplum and therefore are required to view the
patented source
> > code.  The license issue might eventually be improved by
> > Pivotal/Greenplum, but, for now, I think caution is necessary.
> >
> > Of course, never mention known-patented ideas in any community
forum,
> > including this email list.
>
> I just found out that Citus Data has patent applications pending, so
> viewing Citus Data source code has the same problems as Greenplum.

Actually, it might only be their closed source software that contains
patents, i.e. not pg_shard.  I will check and report back when I can
unless someone else reports here first.


While you are doing that, please also check EnterpriseDB. My 
information is that there are patents filed there, so we must check 
that just as much as any other company or person. If you didn't know 
before, you do now.


I am disappointed that your approach to this appears unbalanced and 
partisan. Worse, Greenplum have been quite vocal about their 
intentions, so any feedback you have could easily have been given many 
months ago, not on the day of their announcement. I think you should 
have declared this situation in a very different way to the way you 
have approached this. 5 minutes thought on whether other companies 
might also have been affected would have been sensible, plus the whole 
thing could have been discussed completely offlist. If you do discuss 
things on-list then you should at least state for the record that you 
are an EnterpriseDB employee when discussing your concerns, since that 
is likely to have a material affect on how this situation is viewed by 
anyone worried by your post.


FWIW, Bruce has --for as long as I can remember-- always sent e-mail to 
the list including a signature similar to the following:

-
Bruce Momjian  http://momjian.us
EnterpriseDB http://enterprisedb.com
-

So I guess his affiliation with EnterpriseDB is pretty clear even 
to outsiders just perusing the archives.
Others' interests are IMHO not nearly as clear from their e-mails' 
contents, though.


(not that I have any particular voice/opinion on this matter anyway. I 
am precluded from taking a look at any such release for the time being 
for other reasons...)



I do thank you for all the time you devote to Postgres. All 
community members' contributions are very much appreciated.



/ J.L.



Re: [HACKERS] ALTER SYSTEM vs symlink

2015-11-02 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Andrew Dunstan  writes:
> > I don't know if this was discussed at the time ALTER SYSTEM was 
> > implemented, but I have just discovered that if postgresql.auto.conf is 
> > a symlink to a file elsewhere, ALTER SYSTEM will happily break that link 
> > and write its own local copy. That strikes me as rather unfriendly. Why 
> > not just truncate the file rather than unlink it as it's being 
> > rewritten? Even if we don't want to do that a warning in the docs might 
> > help users avoid the "mistake" I just made.
> 
> Frankly, that behavior strikes me as a good idea.  There is no situation,
> IMV, where it's sane to try to put a symlink there.

I tend to agree with this.  Where a symlink makes sense for us are cases
where the configuration information is intentionally stored in the
correct directory structure (eg: /etc on Debian/Ubuntu and systems which
follow the FHS), and intended to be user-modifyable.  That's not the
case with ALTER SYSTEM.

What this request strikes me as asking for is the same as what I asked
for when this feature was originally going in- there should be a way to
disable it.  Allowing changes through ALTER SYSTEM which can result in
the system not being able to start is dangerous and confusing to many.

Thanks!

Stephen


signature.asc
Description: Digital signature


[HACKERS] ALTER SYSTEM vs symlink

2015-11-02 Thread Andrew Dunstan
I don't know if this was discussed at the time ALTER SYSTEM was 
implemented, but I have just discovered that if postgresql.auto.conf is 
a symlink to a file elsewhere, ALTER SYSTEM will happily break that link 
and write its own local copy. That strikes me as rather unfriendly. Why 
not just truncate the file rather than unlink it as it's being 
rewritten? Even if we don't want to do that a warning in the docs might 
help users avoid the "mistake" I just made.


cheers

andrew


--
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Tom Lane
Robert Haas  writes:
> On Mon, Nov 2, 2015 at 10:14 AM, Tom Lane  wrote:
>> While I won't stand in the way if someone is dead set on providing a
>> disable switch for ALTER SYSTEM, I fail to see the point of one.

> I have not seen much evidence that the problem with ALTER SYSTEM is
> more than hypothetical.

Yeah, that's an independent line of argument that I also agree with.
Part of the reason why I was happy to throw rolcatupdate overboard was
that it had sat there in the code for twenty years without anyone ever
getting interested enough to turn it into a real feature.  And that
was because we hardly ever heard any reports of anyone actually doing
"DELETE FROM pg_proc" or whatever.  Just as Unix has never really grown
any protections against root doing "rm -rf /", I'm skeptical that we
need superuser training wheels of this ilk.

> I would be willing to wager that a lot more people will hose their
> systems by avoiding ALTER SYSTEM than will do so by using it.

Well, mumble --- the subtext I thought I was hearing from Stephen was
that he'd not give his DBAs write access on postgresql.conf either.
But yes, pushing people away from ALTER SYSTEM and towards manual editing
of postgresql.conf would be a foolish way of "improving safety".

regards, tom lane


-- 
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] pglogical_output - a general purpose logical decoding output plugin

2015-11-02 Thread Craig Ringer
On 2 November 2015 at 20:17, Craig Ringer  wrote:
> Hi all
>
> I'd like to submit pglogical_output for inclusion in the 9.6 series as
> a contrib.

A few points are likely to come up in anything but the most cursory
examination of the patch.

The README alludes to protocol docs that aren't in the tree. A
followup will add them shortly, they just need a few tweaks.

There are pg_regress tests, but they're limited. The output plugin
uses the binary output mode, and pg_regress doesn't play well with
that at all. Timestamps, XIDs, LSNs, etc are embedded in the output.
Also, pglogical its self emits LSNs and timestamps in commit messages.
Some things, like the startup message, are likely to contain variable
data in future too. So we can't easily do a "dumb" comparison of
expected output.

That's why the bulk of the tests in test/ are in Python, using
psycopg2. Python and psycopg2 were used partly because of the
excellent work done by Oleksandr Shulgin at Zalando
(https://github.com/zalando/psycopg2/tree/feature/replication-protocol,
https://github.com/psycopg/psycopg2/pull/322) which means we can
connect to the walsender and consume the replication protocol, rather
than relying only on the SQL interfaces. Both are supported, and only
the SQL interface is used by default. It also means the tests can have
logic to validate the protocol stream, examining it message by message
to ensure it's exactly what's expected. Rather than a diff where two
lines of binary gibberish don't match, you get a specific error. Of
course, I'm aware that the buildfarm animals aren't required to have
Python, let alone a patched psycopg2, so we can't rely on these as
smoketests. That's why the pg_regress tests are there too.

There another extension inside it, in
contrib/pglogical_output/examples/hooks . I'm not sure if this should
be separated out into a separate contrib/ since it's very tightly
coupled to pglogical_output. Its purpose is to expose the hooks from
pglogical_output to SQL, so that they can be implemented by plpgsql or
whatever, instead of having to be C functions. It's not integrated
into pglogical_output proper because I see this as mainly a test and
prototyping facility. It's necessary to have this in order for the
unit tests to cover filtering and hooks, but most practical users will
never want or need it. So I'd rather not integrate it into
pglogical_output proper.

pglogical_output has headers, and it installs them into Pg's include/
tree at install time. This is not something that prior contribs have
done, so there's no policy for it as yet. The reason for doing so is
that the output plugin exposes a hooks API so that it can be reused by
different clients with different needs, rather than being tightly
coupled to just one downstream user. For example, it makes no
assumptions about things like what replication origin names mean -
keeping with the design of replication origins, which just provide
mechanism without policy. That means that the client needs to tell the
output plugin how to filter transactions if it wants to do selective
replication on a node-by-node basis. Similarly, there's no built-in
support for selective replication on a per-table basis, just a hook
you can implement. So clients can provide their own policy for how to
decide what tables to replicate. When we're calling hooks for each and
every row we really want a C function pointer so we can avoid the need
to go through the fmgr each time, and so we can pass a `struct
Relation` and bypass the need for catalog lookups. That sort of thing.

Table metadata is sent for each row. It really needs to be sent once
for each consecutive series of rows for the same table. Some care is
required to make sure it's invalidated and re-sent when the table
structure changes mid-series. So that's a pending change. It's
important for efficiency, but pretty isolated and doesn't make the
plugin less useful otherwise, so I thought it could wait.

Sending the whole old tuple is not yet supported, per the fixme in
pglogical_write_update . It should really be a TODO, since to support
this we really need a way to keep track of replica identity for a
table, but also WAL-log the whole old tuple. (ab)using REPLICA
IDENTITY FULL to log the old tuple means we lose information about
what the real identity key is. So this is more of a wanted future
feature, and I'll change it to a TODO.

I'd like to delay some ERROR messages until after the startup
parameters are sent. That way the client can see more info about the
server's configuration, version, capabilities, etc, and possibly
reconnect with acceptable settings. Because a logical decoding plugin
isn't allowed to generate input during its startup callback, though,
this could mean indefinitely delaying an error until the upstream does
some work that results in a decoding callback. So for now errors on
protocol mismatches, etc, are sent immediately.

Text encoding names are compared byte-wise. They should be 

Re: [HACKERS] ALTER SYSTEM vs symlink

2015-11-02 Thread Joe Conway
On 11/02/2015 09:24 AM, Stephen Frost wrote:
> I certainly look forward to having more fine grained control, to the
> point where I'd like to be able to run a system reasonably without an
> active superuser login.  Having superusers logging into production
> running databases is extremely dangerous.  What I have seen happening,
> in multiple organizations, is a move to proxy everything going to the
> database through some other system which attempts to vet and verify that
> the action is acceptable (this also happens to offer up much better
> auditing than what we have today).

I've seen this *repeatedly* in the past few years as well.

> I feel confident that we can provide a better solution than those proxy-based 
> approaches.

+1

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] ALTER SYSTEM vs symlink

2015-11-02 Thread Robert Haas
On Mon, Nov 2, 2015 at 11:39 AM, Stephen Frost  wrote:
> This is all very environment specific.  Changes to postgresql.conf, in
> many environments, go through a serious of tests before being deployed
> by a CM system.  How do we accomplish the same kind of tests before
> deploying a change with ALTER SYSTEM?  We provide no mechanism to do
> that today.

We provide no mechanism to put the changes to put postgresql.conf
changes through a series of tests before being deployed by a CM
system, either.  But you can do that if you want.

Two different methods of restricting ALTER SYSTEM have already been
discussed on this thread: one using file permissions, and the other
using ProcessUtility_hook.  I personally think that's good enough.
It's true that you could have a separate GUC for it, but then somebody
could lock themselves out by turning the GUC on using ALTER SYSTEM, so
now you've made things easier for one group of users while creating a
new pitfall for another group of users.  I'm not sure we really come
out ahead, there.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] remove wal_level archive

2015-11-02 Thread Robert Haas
On Mon, Nov 2, 2015 at 12:21 AM, Craig Ringer  wrote:
> We need to keep both, IMO, with 'archive' as an obsolete synonym for
> hot_standby.
>
> Otherwise pg_upgrade will get grumpy, and so will users who migrate
> their configurations.

Removing options entirely arguably brings some worthwhile
simplification from a user perspective, but it's really unclear to me
that mapping the same set of options onto fewer underlying behaviors
buys us much.  If we don't care enough about getting rid of archive to
force people to change postgresql.conf, I doubt whether this is buying
us enough to be worthwhile.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] Patent warning about the Greenplum source code

2015-11-02 Thread Josh Berkus
On 11/01/2015 06:37 PM, Bruce Momjian wrote:
> Let me add that this is more than hypothetical.  While we don't think
> any of these companies would sue the community for patent infringement,
> they could sue users, and the company could be bought by a sinister
> company that could enforce those patents.  For example, few had problems
> with Sun's control over Java, 

You only say this because you're not part of the Java world.  LOTS of
people had issues with Sun's control over Java; some of them even went
to court.

> but when Oracle bought Sun, more people
> were concerned.  Someone could buy the company _just_ to sue for patent
> infringement --- happens all the time.

Not as often as you'd think, and it hasn't happened in the database
world yet, for some good reasons.  This is all besides the point,
though; PostgreSQL has been accepting contributions from patent-holding
companies for over a decade, and that doesn't seem likely to stop any
time soon.  Greenplum is not in any way special, especially since we
already accepted contributions from Greenplum Inc. back in 2005-2006.

Overall, this thread seems designed to kick up a lot of fuss with no
potential useful outcome.  How about we terminate it now?

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Robert Haas
On Mon, Nov 2, 2015 at 10:14 AM, Tom Lane  wrote:
> Stephen Frost  writes:
>> * Andres Freund (and...@anarazel.de) wrote:
>>> You can just revoke permissions on the file if necessary. Results in the
>>> expected
>>> ERROR:  XX000: could not open file "../postgresql.auto.conf": Permission 
>>> denied
>
>> Yes, I know, but that's a really grotty way of offering a way to disable
>> ALTER SYSTEM.  It's also not exactly intuitive to someone reading the
>> release notes or working on upgrading their existing postgresql.conf.
>
> While I won't stand in the way if someone is dead set on providing a
> disable switch for ALTER SYSTEM, I fail to see the point of one.  It's
> a superuser-only feature to begin with, and if you are handing out
> superuser on production-critical installations to people you don't trust
> completely, you need to have your head examined.
>
> As a directly comparable example, I note that you yourself were in favor
> of getting rid of rolcatupdate, which was the only mechanism we ever had
> that could prevent a superuser from destroying the catalogs entirely
> with a mistyped update --- consider "DELETE FROM pg_proc", for example,
> which unlike ALTER SYSTEM there is simply no way to recover from.
>
> How is it that we don't need rolcatupdate but we do need a way to shut
> off ALTER SYSTEM?  Doesn't compute, IMO.

Yeah, I quite agree.  I think ALTER SYSTEM is a heck of a lot safer
than some things we are currently allowing superusers to do.  I'd
really like there to be some kind of GUC like allow_me_to_hose_myself.
If that's not set, direct catalog modifications should fail, even as
superuser.  I don't think rolcatupdate has ever prevented that, and I
think a GUC would be better than a user-level setting, because you
might easily want to turn it off on a per-session basis.

I have not seen much evidence that the problem with ALTER SYSTEM is
more than hypothetical.  I agree that you COULD configure it so that
the database server doesn't start, but the same is true of manually
editing postgresql.conf - but now that we're mostly out from under
operating system limits on shared memory, there aren't actually that
many ways to do that.  And, unlike vi $PGDATA/postgresql.conf, ALTER
SYSTEM actually has a pretty healthy amount of sanity checking built
in:

rhaas=# alter system set wal_level = arcive;
ERROR:  invalid value for parameter "wal_level": "arcive"
HINT:  Available values: minimal, archive, hot_standby, logical.

I would be willing to wager that a lot more people will hose their
systems by avoiding ALTER SYSTEM than will do so by using it.  Has
anyone ever run across a case where this actually happened?  What GUC
did the user set to cause the problem, and to what value?  I was able
to make it happen on my MacBook by setting max_connections to 100,000,
but that's unlikely to come up much in the real world, and 50,000
worked.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] WIP: Rework access method interface

2015-11-02 Thread Alvaro Herrera
Tom Lane wrote:

> Probably the least messy way to fix this is to drop that #include and
> instead use dummy declarations like "struct PlannerInfo;" and "struct
> IndexPath;" here.  We could additionally dumb the amcostestimate
> declaration down from using "Cost" and "Selectivity" to just saying
> "double".

I'm not a fan of this approach.  I'd rather split the executor headers
in two, a leanone with the typedefs only and another with the actual
struct definitions.  That way we have one very lean executor header that
can be included everywhere (in headers and .c files that only pass the
structs around), and a fat one that is only included by the executor .c
files (and the few extra .c files that need access to the struct
definitions).

This would be similar in spirit to the htup.h / htup_details.h split.

I think (almost?) all the headers that define nodes suffer from this
disease and could be cured in the same way.

> (Note: I realize that there's a precedent in fdwapi.h of including planner
> headers into what's otherwise mostly an executor thing.  That one's not
> quite as destructive to header modularity because it's not used in as many
> places as amapi.h will be.  But maybe we should rethink that as well.)

Yes, rethink++.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
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] remove wal_level archive

2015-11-02 Thread David Steele

On 11/2/15 12:21 AM, Craig Ringer wrote:

On 1 September 2015 at 10:39, Peter Eisentraut  wrote:

So we've had several rounds of discussions about simplifying replication
configuration in general and the wal_level setting in particular. [0][1]

[snip]

Bike-shedding:  In this patch, I removed "archive" and kept
"hot_standby", because that's what the previous discussions suggested.
Historically and semantically, it would be more correct the other way
around.  On the other hand, keeping "hot_standby" would probably require
fewer configuration files to be changed.  Or we could keep both, but
that would be confusing (for users and in the code).


We need to keep both, IMO, with 'archive' as an obsolete synonym for
hot_standby.



I would prefer to rename 'hot_standby to 'archive' and make 
'hot_standby' a deprecated synonym for the new 'archive' setting.  This 
prevents breakage in current configurations and avoids propagating a 
misleading setting.


I see a fair number of installations with backup/archiving but no hot 
standby (or any standby at all).  There is often confusion when I 
suggest setting 'wal_level' to 'hot_standby' to be better prepared for 
the future.  Admittedly these setups are becoming less common but they 
are certainly out there.


--
-David
da...@pgmasters.net


--
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> > * Andres Freund (and...@anarazel.de) wrote:
> >> You can just revoke permissions on the file if necessary. Results in the
> >> expected
> >> ERROR:  XX000: could not open file "../postgresql.auto.conf": Permission 
> >> denied
> 
> > Yes, I know, but that's a really grotty way of offering a way to disable
> > ALTER SYSTEM.  It's also not exactly intuitive to someone reading the
> > release notes or working on upgrading their existing postgresql.conf.
> 
> While I won't stand in the way if someone is dead set on providing a
> disable switch for ALTER SYSTEM, I fail to see the point of one.  It's
> a superuser-only feature to begin with, and if you are handing out
> superuser on production-critical installations to people you don't trust
> completely, you need to have your head examined.

There's a few different pieces to this.

The first is that there is no superuser without a running database and
there's a difference between the perfect admin and reality.  A DBA may
be trusted to be a superuser and to connect to the database to perform
superuser operations without fully understanding the shared_buffers
parameter or the requirements it places on the system.  Perhaps that
implies that they shouldn't be a superuser, but it's far different to
say "ALTER SYSTEM SET shared_buffers = X;" and "DELETE FROM pg_proc;" to
even minimally experienced DBAs, though both can lead to a PG instance
which can't be started.  As I argued on the ALTER SYSTEM thread, there
may be an organizational role distinction between the roles which are
responsible for starting the database and making sure that it's running
and the set of individuals who have superuser rights.  This is quite
common in environments where there is a central management (CM) system
where everything in /etc is really under the OPs perview, as is making
sure that things are running.  We made things more difficult for them
when we added ALTER SYSTEM because it's no longer the case that the GUCs
which can only be set through postgresql.conf can, well, only be set
through postgresql.conf.  Even a superuser can't control what
postgresql.conf is used to start or to make changes directly to the
postgresql.conf if it's owned by root or another user.

The second is that through shared_preload_libraries and the various
hooks which are available, it's possible to load libraries which do
reduce the capabilities of the superuser in PG.  If the superuser can
still use ALTER SYSTEM and remove those libraries from
shared_preload_libraries then that approach to limiting what actions can
be performed doesn't, ultimately, help.  Such a library could hook into
standard_ProcessUtility to deal with that, but a way to configure
postgresql.conf would be much cleaner.  A deeper hook in the
ALTER SYSTEM might be another approach, or one in the GUC system to
allow external libraries to control what various GUCs can be set to
(either via ALTER SYSTEM or through regular SET calls).

Lastly, I've long been a strong advocate of reducing the set of actions
only a superuser can perform by allowing non-superusers access to
more, by extending our privilege system to be more fine-grained.  Adding
more superuser-only operations goes against the grain a bit.  I'm not
suggesting that it would make sense for ALTER SYSTEM to be non-superuser
or that we shouldn't add superuser-only options, just that I'd like us
to offer more control during initial configuration of the system, to the
point where it's very clear what actions are allowed during ongoing
operations.

> As a directly comparable example, I note that you yourself were in favor
> of getting rid of rolcatupdate, which was the only mechanism we ever had
> that could prevent a superuser from destroying the catalogs entirely
> with a mistyped update --- consider "DELETE FROM pg_proc", for example,
> which unlike ALTER SYSTEM there is simply no way to recover from.

I was an advocate of getting rid of rolcatupdate because it wasn't
fully implemented and that didn't appear likely to change.  Further, I
can see cases where that could end up being more a problem than a help-
where the catalogs have been corrupted in some part and need to be
fixed.

> How is it that we don't need rolcatupdate but we do need a way to shut
> off ALTER SYSTEM?  Doesn't compute, IMO.

I'd like the ability to control all of the above, ultimately.  I don't
believe that we should be allowing the superuser to always modify the
catalog directly- and things like the sepgsql module can actually
address that and limit when the superuser is allowed to with better
granularity then what rolcatupdate provided (or was ever likely to
provide, being a single boolean role attribute).

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] exposing pg_controldata and pg_config as functions

2015-11-02 Thread Joe Conway
On 10/30/2015 06:53 AM, Erik Rijkers wrote:
>> [2015082503-pgconfig_controldata.diff]
> 
> I tried to build this, and the patch applies cleanly but then a ld error
> emerges:

I'm sure the patch would need to be rebased, but the last thread died
with significant complaints and questions about what was the correct
approach. I therefore never even bothered submitting my latest patch
version. I'll try to pick this back up in the next week and see if I can
come up with something we can get a consensus on...

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions

2015-11-02 Thread Pavel Stehule
>>
> >> That sounds to be a solution for this problem or otherwise for such a
> case
> >> can't we completely abort the active transaction and set a flag like
> >> PrevCommandFailed/PrevTransFailed and on receiving next message if
> >> such a flag is set, then throw an appropriate error.
> >
> > This is only partial solution - when some application is broken, then
> there
> > will be orphaned sessions. It is less wrong, than orphaned connections,
> but
> > it can enforce some issues too. The solution of this problem should to
> work
> > well with session pool sw like pgbouncer and similar.
>

I wrote a nonsense - should be "It is less wrong, than orphaned transaction"


>
> Sure.  Unfortunately it's not always practical to do so when you have
> 100's of applications running against 100's of databases, all written
> by teams of variable quality, some of whom have been ejected for
> overseas devlopment or vice versa.  This is the world I live in.
>

I would to say so the breaking transaction is not enough - it needs some
protocol enhancing. There is a advantage of terminate_session, because if
keep_alive packets are used, then client can to know so session is broken
in few seconds.

>
> The point stands that neither pg_cancel_backend or statement_timeout
> (especially) provide *any* kind of safety guarantees because they only
> work if execution is in the database.  All the locks they hold and
> other long running issues pertaining to long running transactions
> (say, advancing xmin) are silent killers with no automatic way of
> detecting or destroying.  I understand the challenges here -- not
> griping in any way -- the workaround is to cron up an executioner.
> Just pointing out we have an issue.
>

It is 100% true. But the users can do strange things. If we solve idle
transactions and not idle session, then they are able to increase
max_connections to thousands with happy smile in face.

I have not strong idea about how to solve it well - maybe introduce
transaction_idle_timeout and session_idle_timeout?

Regards

Pavel


>
> merlin
>


Re: [HACKERS] Patch: Implement failover on libpq connect level.

2015-11-02 Thread Peter Eisentraut
On 10/30/15 9:26 AM, Robert Haas wrote:
> That's true, but doesn't allowing every parameter to be multiply
> specified greatly increase the implementation complexity for a pretty
> marginal benefit?

Well, the way I would have approached is that after all the parsing you
end up with a list of PQconninfoOption arrays and you you loop over that
list and call connectDBStart() until you have a connection that
satisfies you.

I see that the patch doesn't do that and it looks quite messy as a result.



-- 
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] WIP: Rework access method interface

2015-11-02 Thread Alvaro Herrera
Tom Lane wrote:

> I follow your reasoning, but I don't particularly want to make this
> patch wait on a large and invasive refactoring of existing headers.

Sure.

> As a down payment on this problem, maybe we could invent a new planner
> header that provides just enough info to support amapi.h and fdwapi.h;
> it looks like this would be "typedef struct PlannerInfo PlannerInfo;",
> likewise for RelOptInfo, ForeignPath, and IndexPath, and real declarations
> of Cost and Selectivity.

Works for me, under the assumption that, down the road and without any
rush, we can shuffle some more stuff around to make this whole area a
bit cleaner.

> Not sure what to name the new header.

Yeah, this is always a problem for such patches :-(  I have no great
ideas ATM.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Joe Conway
On 11/02/2015 08:33 AM, Stephen Frost wrote:
> A deeper hook in the ALTER SYSTEM might be another approach, or one
> in the GUC system to allow external libraries to control what various
> GUCs can be set to (either via ALTER SYSTEM or through regular SET
> calls).


I have run into multiple situations where having finer grained control
over the setting of GUCs would have been very useful, and spoken to many
others in the community with the same wish.


> I'd like the ability to control all of the above, ultimately.  I
> don't believe that we should be allowing the superuser to always
> modify the catalog directly

+10

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] WIP: Rework access method interface

2015-11-02 Thread Tom Lane
Alvaro Herrera  writes:
> Tom Lane wrote:
>> As a down payment on this problem, maybe we could invent a new planner
>> header that provides just enough info to support amapi.h and fdwapi.h;
>> it looks like this would be "typedef struct PlannerInfo PlannerInfo;",
>> likewise for RelOptInfo, ForeignPath, and IndexPath, and real declarations
>> of Cost and Selectivity.

> Works for me, under the assumption that, down the road and without any
> rush, we can shuffle some more stuff around to make this whole area a
> bit cleaner.

Well, since we're at the start of a devel cycle, we'd have the rest of 9.6
for somebody to whack it around to their heart's content.  Once we ship
9.6 it would get a little harder to redefine the new header(s).

>> Not sure what to name the new header.

> Yeah, this is always a problem for such patches :-(  I have no great
> ideas ATM.

I'll draft something, but in the meantime, file name ideas solicited.

regards, tom lane


-- 
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Josh Berkus
On 11/02/2015 06:36 AM, Tom Lane wrote:
> Andrew Dunstan  writes:
>> I don't know if this was discussed at the time ALTER SYSTEM was 
>> implemented, but I have just discovered that if postgresql.auto.conf is 
>> a symlink to a file elsewhere, ALTER SYSTEM will happily break that link 
>> and write its own local copy. That strikes me as rather unfriendly. Why 
>> not just truncate the file rather than unlink it as it's being 
>> rewritten? Even if we don't want to do that a warning in the docs might 
>> help users avoid the "mistake" I just made.
> 
> Frankly, that behavior strikes me as a good idea.  There is no situation,
> IMV, where it's sane to try to put a symlink there.

So, just a doc patch then?

Since we have both include files and config_dir, I really don't
understand why anyone symlinks conf files anymore.  Anyone care to
enlighten me?

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
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] exposing pg_controldata and pg_config as functions

2015-11-02 Thread Alvaro Herrera
Erik Rijkers wrote:

> utils/fmgrtab.o:(.rodata+0x19f78): undefined reference to `_null_'
> utils/fmgrtab.o:(.rodata+0x1a078): undefined reference to `_null_'

The fix for this is to add a parallel-safe flag to new pg_proc.h lines.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
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] fortnight interval support

2015-11-02 Thread Gavin Flower

On 02/11/15 07:06, Tom Lane wrote:

David Fetter  writes:

On Tue, Oct 27, 2015 at 07:30:13PM +, Nathan Wagner wrote:

On Wed, Oct 28, 2015 at 08:17:25AM +1300, Gavin Flower wrote:


I had actually written:

> You trying to get PostgreSQL banned in France???
>
> When I was learning French many years ago, I was told that the French
> consider their fortnight to be 15 days!!!

The following 2 lines were from Nathan, not me!

What, it's a "fortnight", not a "quinzaine".

You have no idea how hard it was to resist updating the patch...

Well, if you won't do it, I will.

Please tell me this is a joke.

(FWIW, I don't have a problem with "fortnight", but I draw the line
at units that are not used in English.)

regards, tom lane






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


[HACKERS]

2015-11-02 Thread Peter Geoghegan
On Mon, Nov 2, 2015 at 10:36 AM, Josh Berkus  wrote:
> Not as often as you'd think, and it hasn't happened in the database
> world yet, for some good reasons.  This is all besides the point,
> though; PostgreSQL has been accepting contributions from patent-holding
> companies for over a decade, and that doesn't seem likely to stop any
> time soon.  Greenplum is not in any way special, especially since we
> already accepted contributions from Greenplum Inc. back in 2005-2006.

The way that patents are yielded as weapons tends to result in
technology companies seeking them out as a deterrent, if nothing else.
I am not in the least bit surprised that an EMC spin-out has a patent
portfolio.

-- 
Peter Geoghegan


-- 
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] Patent warning about the Greenplum source code

2015-11-02 Thread Bruce Momjian
On Mon, Nov  2, 2015 at 10:36:48AM -0800, Josh Berkus wrote:
> > but when Oracle bought Sun, more people
> > were concerned.  Someone could buy the company _just_ to sue for patent
> > infringement --- happens all the time.
> 
> Not as often as you'd think, and it hasn't happened in the database
> world yet, for some good reasons.  This is all besides the point,
> though; PostgreSQL has been accepting contributions from patent-holding
> companies for over a decade, and that doesn't seem likely to stop any
> time soon.  Greenplum is not in any way special, especially since we
> already accepted contributions from Greenplum Inc. back in 2005-2006.
> 
> Overall, this thread seems designed to kick up a lot of fuss with no
> potential useful outcome.  How about we terminate it now?

I am posting this at the request of Josh Berkus, who wanted
clarification on some issues.  FYI, I have been speaking in this thread
as a community member, and not as a member of core, and made some
mistakes in my handling of this --- my apologies.

First, I have always seen the best intentions from every patent holder I
have worked with in relation to Postgres, including the many Pivotal
employees I have talked to.  I understand the value of those patents to
their companies and their companies' valuation, and releasing any kind
of rights to held patents is never an easy decision.  I know all
companies involved are trying to deal with this complex issue in the
best possible way.

Second, if Pivotal had to chose one license to release their code under,
Apache 2.0 was absolutely the best one, because of the patent clause.  I
will continue to work with them or anyone else as requested to see if
there is an even better approach.

The crux of my concern is that a patent in close-source software is
barely visible --- it might be mentioned in marketing material or
documentation, but that is unlikely.  When something is released as open
source, by definition, the patented idea is visible in that code.  In a
strange twist of fate, open source actually allows more chances for
seeing patented ideas than closed source.  I should have made this clear
in my initial post, and there is nothing Greenplum-specific about any of
this.  My lack of clarity on this caused much confusion --- again my
apologies.

Steven might be right that if you don't know the patent is there, you
are not any more culpable than if you discovered the technique on your
own.  However, the odds of you getting a patented idea from looking at
the source is probably higher than the odds of you coming up with the
idea on your own.

Anyway, I just wanted people to be aware of these risks.  I doubt we can
really do anymore than warn folks as there is just no good boundary on
how to avoid problems, as Andres and Simon pointed out.  Josh is right
that patent problems have been a rarity, and I have every hope that this
will continue.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


-- 
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] [ADMIN] Replication slots and isolation levels

2015-11-02 Thread Vladimir Borodin

> 2 нояб. 2015 г., в 23:37, Robert Haas  написал(а):
> 
> On Fri, Oct 30, 2015 at 9:49 AM, Vladimir Borodin  wrote:
>> I’ve tried two ways - bare SELECT in autocommit mode and BEGIN; SELECT;
>> ROLLBACK. I first described the problem in thread on pgsql-admin@ [0], there
>> is copy-paste from psql there, but during conversation initial description
>> was lost.
>> 
>> [0]
>> http://www.postgresql.org/message-id/7f74c5ea-6741-44fc-b6c6-e96f18d76...@simply.name
> 
> Hmm.  That behavior seems unexpected to me, but I might be missing something.

Me too. That’s why I started the thread. One small detail that might have a 
value is that the big table being queried is partitioned into 64 inhereted 
tables. Now I’m trying to write a simple script to reproduce the problem, but 
that is not so easy because AFAIK VACUUM on master should happen while single 
query on standby is running and it should vacuum those rows that have not been 
accessed by the query on standby yet.

> 
> -- 
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company


--
May the force be with you…
https://simply.name



Re: [HACKERS] WIP: Rework access method interface

2015-11-02 Thread Petr Jelinek

On 2015-11-02 23:28, Tom Lane wrote:

Alvaro Herrera  writes:

Tom Lane wrote:

Regardless of that, I'm a bit skeptical that any of these structs ought
to be defined as part of the amapi.h interface.  They seem to be making
premature judgments as to what an opclass verifier will care about.  In
any case, tying the opclass verifier API to DDL-command implementation
details doesn't seem like a good plan.



That's a different argument, with which I don't necessarily disagree.
I think it's not unlikely that a verifier will want to examine the
contents of the struct, but I don't think that means we need to expose
the struct in amapi.h.


I think we'd be considerably better off to *not* re-use OpFamilyMember
in the verifier API.  It's a struct that was only ever designed for
internal use in CREATE/ALTER OPERATOR CLASS.

I'm kind of inclined to just let the verifiers read the catalogs for
themselves.  AFAICS, a loop around the results of SearchSysCacheList
is not going to be significantly more code than what this patch does,
and it avoids presuming that we know what the verifiers will wish to
look at.


I like this idea. I didn't like from beginning that verifier is tied to 
opclass but didn't have better solution, but this seems reasonable.


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


--
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] NOTIFY in Background Worker

2015-11-02 Thread Haribabu Kommi
On Sat, Aug 29, 2015 at 12:55 PM, Thomas Munro
 wrote:
> On Sat, Aug 29, 2015 at 9:03 AM, Thomas Munro
>  wrote:
>>
>> On Fri, Aug 28, 2015 at 10:30 PM, jacques klein
>>  wrote:
>>>
>>> Hello,
>>>
>>> I added a "NOFITY chan" to the SQL arg of an SPI_execute(), (I did it
>>> also with just the NOTIFY statement),
>>> but the listeners (other workers) don't get the notification until a
>>> "NOTIFY chan" is done for example with pgadmin,
>>>
>>> They don't get lost, just not emited after the "not forgotten" call of
>>> CommitTransactionCommand().
>>>
>>> Is this normal ( i.e. not supported (yet) ), a bug, or did I overlook
>>> some doc. (or source code) ?.
>>>
>>> For now, I will try to "emit" the NOTIFY via libpq.
>>
>>
>> That's because ProcessCompletedNotifies isn't being called.  For regular
>> backends it is called inside the top level loop PostgresMain.  I think you
>> need to include "commands/async.h" and add a call to
>> ProcessCompletedNotifies() after your background worker commits to make this
>> work.
>
>
> For the record, Jacques confirmed off-list that this worked, and I also did
> a couple of tests.
>
> Is this expected?  If so, should it be documented -- perhaps with something
> like the attached?  Alternatively there may be some way to make
> CommitTransactionCommand do it, though the comments in
> ProcessCompletedNotifies explain why that was rejected, at least as far as
> AtCommit_Notify goes.
>
> This made me wonder what happens if a background worker calls LISTEN.
> NotifyMyFrontEnd simply logs the notifications, since there is no remote
> libpq to sent a message to.  Perhaps a way of delivering to background
> workers could be developed, though of course there are plenty of other kinds
> of IPC available already.

With this commit - bde39eed0cafb82bc94c40e95d96b5cf47b6f719, it is not possible
to execute Notify commands inside a parallel worker. Can't we change
it as disable
both listen and notify commands inside a background worker?

Regards,
Hari Babu
Fujitsu Australia


-- 
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] [ADMIN] Replication slots and isolation levels

2015-11-02 Thread Robert Haas
On Fri, Oct 30, 2015 at 9:49 AM, Vladimir Borodin  wrote:
> I’ve tried two ways - bare SELECT in autocommit mode and BEGIN; SELECT;
> ROLLBACK. I first described the problem in thread on pgsql-admin@ [0], there
> is copy-paste from psql there, but during conversation initial description
> was lost.
>
> [0]
> http://www.postgresql.org/message-id/7f74c5ea-6741-44fc-b6c6-e96f18d76...@simply.name

Hmm.  That behavior seems unexpected to me, but I might be missing something.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] pglogical_output - a general purpose logical decoding output plugin

2015-11-02 Thread Jim Nasby

On 11/2/15 8:36 AM, Craig Ringer wrote:

Here's the protocol documentation discussed in the README. It's
asciidoc at the moment, so it can be formatted into something with
readable tables.


Is this by chance up on github? It'd be easier to read the final output 
there than the raw asciidoctor. ;)


Great work on this!
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
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] Patch to install config/missing

2015-11-02 Thread Robert Haas
On Fri, Oct 30, 2015 at 2:42 PM, Jim Nasby  wrote:
> Currently, config/missing isn't being installed. This can lead to confusing
> error messages, such as if Perl isn't found and something needs it [1].
> Attached patch adds it to install and uninstall recipes.

I find it somewhat hard to believe this is the right thing to do.  But
if this isn't right, then I don't know what is right, either.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions

2015-11-02 Thread Jim Nasby

On 11/2/15 11:15 AM, Pavel Stehule wrote:

I have not strong idea about how to solve it well - maybe introduce
transaction_idle_timeout and session_idle_timeout?


Yes, please. This is a very common problem. I would love a better way to 
detect (or prevent) clients from being brain-dead about how they're 
using transactions, but short of that this is the next best thing.


Actually, one other thing that would help is to have the ability to turn 
this into an ERROR:


begin;
WARNING:  there is already a transaction in progress
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
> On Mon, Nov 2, 2015 at 11:39 AM, Stephen Frost  wrote:
> > This is all very environment specific.  Changes to postgresql.conf, in
> > many environments, go through a serious of tests before being deployed
> > by a CM system.  How do we accomplish the same kind of tests before
> > deploying a change with ALTER SYSTEM?  We provide no mechanism to do
> > that today.
> 
> We provide no mechanism to put the changes to put postgresql.conf
> changes through a series of tests before being deployed by a CM
> system, either.  But you can do that if you want.

I'm trying to understand what you're getting at above and how it is
actually an argument against my point, and I'm not able to do so.

I wasn't suggesting that we need to provide a way for users to vet the
changes to postgresql.conf but was rather saying that having ALTER
SYSTEM means that if a user already has such a system, it can end up
being defeated, as it were, by a user using ALTER SYSTEM.

> Two different methods of restricting ALTER SYSTEM have already been
> discussed on this thread: one using file permissions, and the other
> using ProcessUtility_hook.  I personally think that's good enough.

The issue which I have with these suggestions is that one requires users
to install an as-yet-unwritten module and the other is to hack with
permissions in the data directory.  As we've all seen, people playing in
$PGDATA is generally a bad idea.

> It's true that you could have a separate GUC for it, but then somebody
> could lock themselves out by turning the GUC on using ALTER SYSTEM, so
> now you've made things easier for one group of users while creating a
> new pitfall for another group of users.  I'm not sure we really come
> out ahead, there.

We wouldn't have to make it a GUC.  If we decided to anyway, we could
certainly disable the ability to modify it through ALTER SYSTEM, or
ignore it if we find it in postgresql.auto.conf as it surely wouldn't
make any sense to have it there.

The original idea here was to add an include line for .auto.conf.  That
would certainly have made me happy.  Unfortunately, by not doing that,
we've painted ourselves into a corner that's rather ugly to get out of,
since we can't now say that such an include line is required for
ALTER SYSTEM to work.

It wasn't my intent to get into such a discussion regarding ALTER SYSTEM
at this time- there are more important activities at hand, and the ship
has largely sailed on what I had been asking for originally.  Perhaps we
can discuss it again in the future but I'm certainly happy to drop the
it for now.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Dangling Client Backend Process

2015-11-02 Thread Rajeev rastogi
On 30 October 2015 20:33, Andres Freund Wrote:
>On 2015-10-30 10:57:45 -0400, Tom Lane wrote:
>> Andres Freund  writes:
>> > adding a parseInput(conn) into the loop yields the expected
>> > FATAL:  57P01: terminating connection due to unexpected postmaster
>> > exit Is there really any reason not to do that?
>>
>> Might work, but it probably needs some study:
>
>Yea, definitely. I was just at pgconf.eu's keynote catching up on a talk.
>No fully thought through proposal's to be expected ;)
>
>> (a) is it safe
>
>I don't immediately see why not.
>
>> (b) is this the right place / are there other places
>
>I think it's ok for the send failure case, in a quick lookthrough I
>didn't find anything else for writes - I'm not entirely sure all read
>cases are handled tho, but it seems less likely to be mishandles.


I also did some analysis as Andres suggested and observed that since the error 
message is already sent by backend to frontend, 
so error message is available on connection channel just that is was not 
received/processed by frontend.
Also pqHandleSendFailure was checking the available data if any but it was 
ignored to process.

So as Andres suggested above, if we add parseInput to receiver and parse the 
available message on connection channel, we could display appropriate error. 

IMHO above proposed place to add parseInput seems to be right, as already this 
function takes of handling " NOTICE message that the backend might have sent 
just before it died "

Attached is the patch with this change.

Comments?

Thanks and Regards,
Kumar Rajeev Rastogi


dangle-v4.patch
Description: dangle-v4.patch

-- 
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] onlyvalue aggregate (was: First Aggregate Funtion?)

2015-11-02 Thread Dean Rasheed
On 28 October 2015 at 16:50, Marko Tiikkaja  wrote:
> Hi,
>
> Here's a patch for the aggregate function outlined by Corey Huinker in
> CADkLM=foA_oC_Ri23F9PbfLnfwXFbC3Lt8bBzRu3=cb77g9...@mail.gmail.com .

+1. I've wanted something like this a few times. Of the names
suggested so far, I think I prefer "single_value", and yes I think it
should work with NULLs.

Regards,
Dean


-- 
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] WIP: Rework access method interface

2015-11-02 Thread Tom Lane
Alexander Korotkov  writes:
> Tom, could you take a look at them?

I started to look at this today.  (Apologies for the delay, but I came
back from San Francisco with a nasty head cold, and wasn't really up to
doing anything complicated last week.)

The thing that jumps out at me right away is that the proposed new amapi.h
header has this:

+ #include "nodes/relation.h"

to support declaring amcostestimate_function.  This will result in
importing the planner's declarations into pretty much every part of the
executor, because not only is amapi.h #included by a lot of places, but
the proposed patch actually has execnodes.h including it, as well as some
other popular headers.  We might as well shove everything into postgres.h
and forget about header modularity altogether.

Probably the least messy way to fix this is to drop that #include and
instead use dummy declarations like "struct PlannerInfo;" and "struct
IndexPath;" here.  We could additionally dumb the amcostestimate
declaration down from using "Cost" and "Selectivity" to just saying
"double".

A different line of attack would be to try to divide amapi.h into
"executor" and "planner" parts, but I'm not sure I see how to make that
work.  Somewhere you gotta declare the struct of function pointers.

(Note: I realize that there's a precedent in fdwapi.h of including planner
headers into what's otherwise mostly an executor thing.  That one's not
quite as destructive to header modularity because it's not used in as many
places as amapi.h will be.  But maybe we should rethink that as well.)

Thoughts, better ideas?

regards, tom lane


-- 
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Stephen Frost
Robert, Tom,

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Robert Haas  writes:
> > I would be willing to wager that a lot more people will hose their
> > systems by avoiding ALTER SYSTEM than will do so by using it.
> 
> Well, mumble --- the subtext I thought I was hearing from Stephen was
> that he'd not give his DBAs write access on postgresql.conf either.
> But yes, pushing people away from ALTER SYSTEM and towards manual editing
> of postgresql.conf would be a foolish way of "improving safety".

This is all very environment specific.  Changes to postgresql.conf, in
many environments, go through a serious of tests before being deployed
by a CM system.  How do we accomplish the same kind of tests before
deploying a change with ALTER SYSTEM?  We provide no mechanism to do
that today.

What the whole ALTER SYSTEM discussion lacks is an appreciation of the
good CM practices which exist in many environments.  If I set up my CM
correctly, then I deploy new changes to the system via puppet or chef
only after those changes have been applied to the pre-production
environments which have identical system configurations.  Today, a
helpful DBA may make changes in production that make later changes by
the CM to postgresql.conf completely ineffective, leading to problems
and possibly even failures.

Suggesting that we get rid of superuser accounts or minimize them
further than already done is ineffective because we simply don't have
the fine grained controls which are needed to allow that.  I'm hopeful
that we'll get there and will continue to work towards it.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] ALTER SYSTEM vs symlink

2015-11-02 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> > * Tom Lane (t...@sss.pgh.pa.us) wrote:
> >> Well, mumble --- the subtext I thought I was hearing from Stephen was
> >> that he'd not give his DBAs write access on postgresql.conf either.
> >> But yes, pushing people away from ALTER SYSTEM and towards manual editing
> >> of postgresql.conf would be a foolish way of "improving safety".
> 
> > This is all very environment specific.  Changes to postgresql.conf, in
> > many environments, go through a serious of tests before being deployed
> > by a CM system.  How do we accomplish the same kind of tests before
> > deploying a change with ALTER SYSTEM?  We provide no mechanism to do
> > that today.
> 
> Sure, so if you have such a process, you tell your DBAs not to use ALTER
> SYSTEM.  End of problem --- or if it isn't end of problem, you have HR
> issues that the database cannot fix for you.

It's not that simple though, as you point out- ALTER SYSTEM for *some*
operations is just fine, but it isn't for other changes.  That's one of
the difficulties with it.

Further, there's a level of cooperation required between the
postgresql.conf and the system, at a level which, in my mind anyway,
falls under the purview of the system adminsitration team instead of the
DBA team.  In larger organizations, those can be quite distinct sets.
If the DBA team is changing values via ALTER SYSTEM then, again, you can
run into problems when the sysadmin team or the CM system makes a
change.

Consider cases such as listen_addresses or, more generally, startup-time
options.  I brought all of this up on that thread and pointed out that
there are a bunch of postgresql.conf configuration options where system
level changes have to be made at the same time, which is why they make
sense to be put under a CM system which controls both the
postgresql.conf and the system configuration (what the IP address of the
system is, for example).

> The core point here is that if you're handing people superuser and
> expecting that they can't possibly circumvent any training-wheel-type
> restrictions you then put on that, you're wrong.  In the end you'd
> better trust that your DBAs know the process they're supposed to follow
> and follow it.

I don't view this as being about circumvention or training-wheel-type
restrictions.

> It may be that, at some point in the future, we'll have this sliced and
> diced fine enough that it's safe to allow some part of ALTER SYSTEM
> functionality to be accessible to people you don't want to give full
> superuser to.  But there's no such thing as "partial superuser", and
> personally I believe that it would be a tremendous waste of time to
> try to build such a feature.

I certainly look forward to having more fine grained control, to the
point where I'd like to be able to run a system reasonably without an
active superuser login.  Having superusers logging into production
running databases is extremely dangerous.  What I have seen happening,
in multiple organizations, is a move to proxy everything going to the
database through some other system which attempts to vet and verify that
the action is acceptable (this also happens to offer up much better
auditing than what we have today).  I feel confident that we can provide
a better solution than those proxy-based approaches.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] ALTER SYSTEM vs symlink

2015-11-02 Thread Tom Lane
Stephen Frost  writes:
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> How is it that we don't need rolcatupdate but we do need a way to shut
>> off ALTER SYSTEM?  Doesn't compute, IMO.

> I'd like the ability to control all of the above, ultimately.  I don't
> believe that we should be allowing the superuser to always modify the
> catalog directly- and things like the sepgsql module can actually
> address that and limit when the superuser is allowed to with better
> granularity then what rolcatupdate provided (or was ever likely to
> provide, being a single boolean role attribute).

Mumble.  I have no objection to sepgsql deciding to disallow ALTER SYSTEM
--- after all, the entire point of that module is to enforce arbitrary
annoying restrictions ;-).  But I am not convinced that we need any other
way to turn it off.  As Robert points out, it's far *less* dangerous than
most other superuser-only features.

Also, disallowing ALTER SYSTEM altogether strikes me as an extremely
brute-force solution to any of the specific issues you mention.  If you're
worried about locking down shared_preload_libraries, for example, it would
be far better to lock down just that one variable.

regards, tom lane


-- 
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Tom Lane
Stephen Frost  writes:
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> Well, mumble --- the subtext I thought I was hearing from Stephen was
>> that he'd not give his DBAs write access on postgresql.conf either.
>> But yes, pushing people away from ALTER SYSTEM and towards manual editing
>> of postgresql.conf would be a foolish way of "improving safety".

> This is all very environment specific.  Changes to postgresql.conf, in
> many environments, go through a serious of tests before being deployed
> by a CM system.  How do we accomplish the same kind of tests before
> deploying a change with ALTER SYSTEM?  We provide no mechanism to do
> that today.

Sure, so if you have such a process, you tell your DBAs not to use ALTER
SYSTEM.  End of problem --- or if it isn't end of problem, you have HR
issues that the database cannot fix for you.

The core point here is that if you're handing people superuser and
expecting that they can't possibly circumvent any training-wheel-type
restrictions you then put on that, you're wrong.  In the end you'd
better trust that your DBAs know the process they're supposed to follow
and follow it.

It may be that, at some point in the future, we'll have this sliced and
diced fine enough that it's safe to allow some part of ALTER SYSTEM
functionality to be accessible to people you don't want to give full
superuser to.  But there's no such thing as "partial superuser", and
personally I believe that it would be a tremendous waste of time to
try to build such a feature.

regards, tom lane


-- 
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] proposal: PL/Pythonu - function ereport

2015-11-02 Thread Catalin Iacob
Hello,

Here's a detailed review:

1. in PLy_spi_error__init__ you need to check kw for NULL before doing
PyDict_Size(kw) otherwise for plpy.SPIError() you get Bad internal
call because PyDict_Size expects a real dictionary not NULL

2. a test with just plpy.SPIError() is still missing, this would have caught 1.

3. the tests have "possibility to set all accessable fields in custom
exception" above a test that doesn't set all fields, it's confusing
(and accessible is spelled wrong)

4. in the docs, "The constructor of SPIError exception (class)
supports keyword parameters." sounds better as "An SPIError instance
can be constructed using keyword parameters."

5. there is conceptual code duplication between PLy_spi_exception_set
in plpy_spi.c, since that code also constructs an SPIError but from C
and with more info available (edata->internalquery and
edata->internalpos). But making a tuple and assigning it to spidata is
in both. Not sure how this can be improved.

6. __init__ can use METH_VARARGS | METH_KEYWORDS in both Python2 and
3, no need for the #if

7. "could not create dictionary for SPI exception" would be more
precise as "could not create dictionary for SPIError" right?

8. in PLy_add_methods_to_dictionary I would avoid import since it
makes one think of importing modules; maybe "could not create function
wrapper for \"%s\"",  "could not create method wrapper for \"%s\""

9. also in PLy_add_methods_to_dictionary "could public method \"%s\"
in dictionary" is not proper English and is missing not, maybe "could
not add method \"%s\" to class dictionary"?

10. in PLy_add_methods_to_dictionary if PyCFunction_New succeeds but
PyMethod_New fails, func will leak

11. it would be nice to have a test for the invalid SQLSTATE code part

12. similar to 10, in PLy_spi_error__init__ taking the "invalid
SQLSTATE" branch leaks exc_args, in general early returns via PLy_elog
will leave the intermediate Python objects leaking


Will mark the patch as Waiting for author.


-- 
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] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions

2015-11-02 Thread Merlin Moncure
On Mon, Nov 2, 2015 at 1:28 AM, Pavel Stehule  wrote:
>
>
> 2015-11-02 5:23 GMT+01:00 Amit Kapila :
>>
>> On Sun, Nov 1, 2015 at 11:34 PM, Tom Lane  wrote:
>> >
>> > Magnus Hagander  writes:
>> > > On Sat, Oct 31, 2015 at 5:50 AM, Amit Kapila 
>> > > wrote:
>> > >> Why pg_cancel_backend(pid) is not sufficient for the above use case?
>> > >> Basically you want to rollback current transaction, I think that can
>> > >> be
>> > >> achieved by pg_cancel_backend.
>> >
>> > > Not when the session is idle in transaction, only when it's actually
>> > > doing
>> > > something.
>> >
>>
>> Okay, thats right and the reason is that while reading message from
>> client,
>> if an error occurs, it can loose track of previous and next messages and
>> that
>> could lead to an unrecoverable state.
>>
>> >
>> > I think in principle it could be done by transitioning the backend into
>> > a new xact.c state, wherein we know that the active transaction has been
>> > canceled (at least to the extent of releasing externally visible
>> > resources
>> > such as locks and snapshots), but this fact hasn't been reported to the
>> > connected client.  Then the next command submitted by the client would
>> > get
>> > a "transaction cancelled" error and we'd go into the normal transaction-
>> > failed state.
>> >
>>
>> That sounds to be a solution for this problem or otherwise for such a case
>> can't we completely abort the active transaction and set a flag like
>> PrevCommandFailed/PrevTransFailed and on receiving next message if
>> such a flag is set, then throw an appropriate error.
>
> This is only partial solution - when some application is broken, then there
> will be orphaned sessions. It is less wrong, than orphaned connections, but
> it can enforce some issues too. The solution of this problem should to work
> well with session pool sw like pgbouncer and similar.

Sure.  Unfortunately it's not always practical to do so when you have
100's of applications running against 100's of databases, all written
by teams of variable quality, some of whom have been ejected for
overseas devlopment or vice versa.  This is the world I live in.

The point stands that neither pg_cancel_backend or statement_timeout
(especially) provide *any* kind of safety guarantees because they only
work if execution is in the database.  All the locks they hold and
other long running issues pertaining to long running transactions
(say, advancing xmin) are silent killers with no automatic way of
detecting or destroying.  I understand the challenges here -- not
griping in any way -- the workaround is to cron up an executioner.
Just pointing out we have an issue.

merlin


-- 
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] WIP: Rework access method interface

2015-11-02 Thread Tom Lane
Alvaro Herrera  writes:
> Tom Lane wrote:
>> Probably the least messy way to fix this is to drop that #include and
>> instead use dummy declarations like "struct PlannerInfo;" and "struct
>> IndexPath;" here.  We could additionally dumb the amcostestimate
>> declaration down from using "Cost" and "Selectivity" to just saying
>> "double".

> I'm not a fan of this approach.  I'd rather split the executor headers
> in two, a leanone with the typedefs only and another with the actual
> struct definitions.  That way we have one very lean executor header that
> can be included everywhere (in headers and .c files that only pass the
> structs around), and a fat one that is only included by the executor .c
> files (and the few extra .c files that need access to the struct
> definitions).

> This would be similar in spirit to the htup.h / htup_details.h split.
> I think (almost?) all the headers that define nodes suffer from this
> disease and could be cured in the same way.

I follow your reasoning, but I don't particularly want to make this
patch wait on a large and invasive refactoring of existing headers.

As a down payment on this problem, maybe we could invent a new planner
header that provides just enough info to support amapi.h and fdwapi.h;
it looks like this would be "typedef struct PlannerInfo PlannerInfo;",
likewise for RelOptInfo, ForeignPath, and IndexPath, and real declarations
of Cost and Selectivity.  Not sure what to name the new header.

Comments?

regards, tom lane


-- 
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] Patch to install config/missing

2015-11-02 Thread Alvaro Herrera
Robert Haas wrote:
> On Fri, Oct 30, 2015 at 2:42 PM, Jim Nasby  wrote:
> > Currently, config/missing isn't being installed. This can lead to confusing
> > error messages, such as if Perl isn't found and something needs it [1].
> > Attached patch adds it to install and uninstall recipes.
> 
> I find it somewhat hard to believe this is the right thing to do.  But
> if this isn't right, then I don't know what is right, either.

I think 'missing' is as part of the development package as install-sh is
-- and we're installing that one, so why not 'missing'?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
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] WIP: Rework access method interface

2015-11-02 Thread Tom Lane
Alvaro Herrera  writes:
> Tom Lane wrote:
>> ... btw, what is the point of catalog/opfam_internal.h?  I see you added
>> it in b488c580aef4e05f, but it seems quite useless to have split it out
>> as a separate header, since only commands/opclasscmds.c uses it.

> Oh, that slipped in there, didn't it.  The JSON DDL-deparse bits need
> it -- last posted by Alex Shulgin here:
> https://www.postgresql.org/message-id/CACACo5Q_UXYwF117LBhjZ3xaMPyrgqnqE%3DmXvRhEfjJ51aCfwQ%40mail.gmail.com

I still don't see any reference to OpFamilyMember in there?

> I suppose it shouldn't have been committed, and be part of the extension
> instead.

Previously, OpFamilyMember was just a transient internal data structure
inside commands/opclasscmds.c.  Unless we intended that some chunks of
the code in there be exposed for use by extensions, it's not terribly
clear to me why extensions would need to access this struct.  Perhaps
we ought to just revert struct OpFamilyMember back into opclasscmds.c.

Regardless of that, I'm a bit skeptical that any of these structs ought
to be defined as part of the amapi.h interface.  They seem to be making
premature judgments as to what an opclass verifier will care about.  In
any case, tying the opclass verifier API to DDL-command implementation
details doesn't seem like a good plan.

regards, tom lane


-- 
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] WIP: Rework access method interface

2015-11-02 Thread Tom Lane
... btw, what is the point of catalog/opfam_internal.h?  I see you added
it in b488c580aef4e05f, but it seems quite useless to have split it out
as a separate header, since only commands/opclasscmds.c uses it.

My attention got drawn to it because the current patch proposes to
#include it in amapi.h, which is as thorough a subversion of the concept
of "internal header" as I can readily think of.  If we're going to do
that with it we'd definitely need to rename it.  But I'm not following
why struct OpFamilyMember needs to be exposed at all.

regards, tom lane


-- 
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] Patch to install config/missing

2015-11-02 Thread Tom Lane
Robert Haas  writes:
> On Fri, Oct 30, 2015 at 2:42 PM, Jim Nasby  wrote:
>> Currently, config/missing isn't being installed. This can lead to confusing
>> error messages, such as if Perl isn't found and something needs it [1].
>> Attached patch adds it to install and uninstall recipes.

> I find it somewhat hard to believe this is the right thing to do.  But
> if this isn't right, then I don't know what is right, either.

Installing our "missing" script seems like a seriously bad idea.  For one
thing, as the comments in it note, it's similar but not identical to such
a script that exists in many GNU packages; we could easily create problems
for other packages that rely on other variants of it.

I wonder how much we need that script at all though.  If, say, configure
doesn't find bison, what's so wrong with just defining BISON=bison and
letting the usual shell "bison: command not found" error leak through?
I'm not seeing that we get a very large increment of user-friendliness
from interposing the "missing" script.  In at least one way it's a net
negative: if you go and install bison after getting the error, you will
have to re-run configure to recover, whereas playing dumb would frequently
have left us with the right configuration output already.

regards, tom lane


-- 
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] ParallelContexts can get confused about which worker is which

2015-11-02 Thread Robert Haas
On Sun, Nov 1, 2015 at 1:11 AM, Amit Kapila  wrote:
> If we are going to add a new parameter to BackgroundWorker structure,
> then the same needs to be updated in docs [1] as well.

Right, good point.

> I think adding
> a new parameter to this structure might require some updations in
> client applications. It seems worth to add a note for the same in commit
> message, so that same could be reflected in Release Notes.

Client background worker modules need to be recompiled so that they
know about the new structure size, but they shouldn't need any code
changes.

> Also, I don't know why BGW_EXTRALEN needs to be 128 and not 64?
> I guess you kept it so that in future if we need to pass more information,
> then the same could be used which seems reasonable considering that
> this is an exposed structure and we don't want to change it again.

Well, I only need 4 bytes for this purpose, but I figured other users
of the background worker facility might like to have a little more.  I
don't think 64 vs. 128 matters very much; I'm happy to hear other
opinions on this topic.  What I think mostly matters is that we don't
make it so much that the overhead of copying it would add
substantially to background worker startup times.  I assume that
either 64 or 128 is safe in that regard.

> /* Mutex protects remaining fields. */
> slock_t mutex;
>
>
> /* Maximum XactLastRecEnd of any worker. */
> XLogRecPtr last_xlog_end;
>
> } FixedParallelState;
>
> Now as above mutex will be used to just protect last_xlog_end, do you
> think it is better to modify the comment above mutex declaration?

Meh.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] WIP: Rework access method interface

2015-11-02 Thread Alvaro Herrera
Tom Lane wrote:
> ... btw, what is the point of catalog/opfam_internal.h?  I see you added
> it in b488c580aef4e05f, but it seems quite useless to have split it out
> as a separate header, since only commands/opclasscmds.c uses it.
> 
> My attention got drawn to it because the current patch proposes to
> #include it in amapi.h, which is as thorough a subversion of the concept
> of "internal header" as I can readily think of.  If we're going to do
> that with it we'd definitely need to rename it.  But I'm not following
> why struct OpFamilyMember needs to be exposed at all.

Oh, that slipped in there, didn't it.  The JSON DDL-deparse bits need
it -- last posted by Alex Shulgin here:
https://www.postgresql.org/message-id/CACACo5Q_UXYwF117LBhjZ3xaMPyrgqnqE%3DmXvRhEfjJ51aCfwQ%40mail.gmail.com

I suppose it shouldn't have been committed, and be part of the extension
instead.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Robert Haas
On Mon, Nov 2, 2015 at 3:41 PM, Stephen Frost  wrote:
>> Two different methods of restricting ALTER SYSTEM have already been
>> discussed on this thread: one using file permissions, and the other
>> using ProcessUtility_hook.  I personally think that's good enough.
>
> The issue which I have with these suggestions is that one requires users
> to install an as-yet-unwritten module and the other is to hack with
> permissions in the data directory.  As we've all seen, people playing in
> $PGDATA is generally a bad idea.

Well, fair enough.  I think somebody could write that module in about
an hour, though.  All you have to do is latch onto ProcessUtility_hook
and throw an error if you've got yourself an AlterSystemStmt.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] WIP: Rework access method interface

2015-11-02 Thread Alvaro Herrera
Tom Lane wrote:
> Alvaro Herrera  writes:
> > Tom Lane wrote:
> >> ... btw, what is the point of catalog/opfam_internal.h?  I see you added
> >> it in b488c580aef4e05f, but it seems quite useless to have split it out
> >> as a separate header, since only commands/opclasscmds.c uses it.
> 
> > Oh, that slipped in there, didn't it.  The JSON DDL-deparse bits need
> > it -- last posted by Alex Shulgin here:
> > https://www.postgresql.org/message-id/CACACo5Q_UXYwF117LBhjZ3xaMPyrgqnqE%3DmXvRhEfjJ51aCfwQ%40mail.gmail.com
> 
> I still don't see any reference to OpFamilyMember in there?

Sorry, that posting doesn't have the part that generates the JSON.  It's
ddl_deparse.c in the .tar.gz earlier in that thread:
http://www.postgresql.org/message-id/cacaco5qquav+n4gi+ya1jf_a+qenr6sjup8cydpsrxka+fh...@mail.gmail.com

> > I suppose it shouldn't have been committed, and be part of the extension
> > instead.
> 
> Previously, OpFamilyMember was just a transient internal data structure
> inside commands/opclasscmds.c.  Unless we intended that some chunks of
> the code in there be exposed for use by extensions, it's not terribly
> clear to me why extensions would need to access this struct.  Perhaps
> we ought to just revert struct OpFamilyMember back into opclasscmds.c.

The whole point of splitting the struct declaration to a new header was
to get a DDL deparser to examine the list of objects being created, so
that it could construct the deparsed command.  If the struct is opaque
to the outside world, there's no way to do that (as I recall, you can't
construct the full command starting from the parsed version only -- you
need access to the OIDs of the ops/procs actually added to the opclass.)

> Regardless of that, I'm a bit skeptical that any of these structs ought
> to be defined as part of the amapi.h interface.  They seem to be making
> premature judgments as to what an opclass verifier will care about.  In
> any case, tying the opclass verifier API to DDL-command implementation
> details doesn't seem like a good plan.

That's a different argument, with which I don't necessarily disagree.
I think it's not unlikely that a verifier will want to examine the
contents of the struct, but I don't think that means we need to expose
the struct in amapi.h.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
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] eXtensible Transaction Manager API

2015-11-02 Thread Simon Riggs
On 31 October 2015 at 10:22, konstantin knizhnik 
wrote:

> Hi,
>
> PostgresPro cluster team wants to announce proposal for eXtensible
> Transaction Manager API and reference implementation of distributed
> transaction manager (pg_dtm).
> pg_dtm is just a standard PostgreSQL extension which should be installed
> in normal way.
>
> Source of pg_dtm and PostgreSQL patches are available here:
> https://github.com/postgrespro/pg_dtm
> WiKi page: https://wiki.postgresql.org/wiki/DTM


Very interesting work.


> xtm.patch patches PostgreSQL core by replacing direct calls of 7 TM
> functions with indirect calls and


At first I was concerned about recovery, but that looks to be covered.

PostgreSQL assumes that top-level xid commit is atomic, along with all of
its subtransactions. So the API having access to only Get/Set at the xid
level would not work. We would need TransactionIdSetTreeStatus() rather
than just SetStatus. GetStatus is not atomic.


> adding 3 addition events to transaction commit callbacks:
>

Those look uncontentious, so we should add those anyway in a sub-patch.


> We have also pgDTM implementation which is using timestamps (system time)
> as CSNs.
> It is also based on the same XTM transaction API.
> We will publish it later once we clarify problems with recovery and
> performance with this approach.
>

That is most interesting part, so it needs to be published.

At present, I can't tell whether you need subtrans and multixact calls in
the API as well. I would imagine we probably do, though we might imagine an
implementation that didn't support those concepts.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] onlyvalue aggregate (was: First Aggregate Funtion?)

2015-11-02 Thread Simon Riggs
On 2 November 2015 at 09:32, Dean Rasheed  wrote:

> On 28 October 2015 at 16:50, Marko Tiikkaja  wrote:
> > Hi,
> >
> > Here's a patch for the aggregate function outlined by Corey Huinker in
> > CADkLM=foA_oC_Ri23F9PbfLnfwXFbC3Lt8bBzRu3=cb77g9...@mail.gmail.com .
>
> +1. I've wanted something like this a few times. Of the names
> suggested so far, I think I prefer "single_value", and yes I think it
> should work with NULLs.
>

+1

I think we should avoid using ONLY or VALUE within the names since those
already have other meanings in Postgres.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] WIP: Rework access method interface

2015-11-02 Thread Alvaro Herrera
Tom Lane wrote:
> Alvaro Herrera  writes:
> > Tom Lane wrote:
> >>> ... btw, what is the point of catalog/opfam_internal.h?
> 
> > The whole point of splitting the struct declaration to a new header was
> > to get a DDL deparser to examine the list of objects being created, so
> > that it could construct the deparsed command.  If the struct is opaque
> > to the outside world, there's no way to do that (as I recall, you can't
> > construct the full command starting from the parsed version only -- you
> > need access to the OIDs of the ops/procs actually added to the opclass.)
> 
> Hm.  OK, looking closer, I see that we've effectively exposed these
> structs as being what is passed to EventTriggerCollectCreateOpClass, so
> even if there's not currently any committed code that can read that info,
> we clearly need the structs to be accessible to interested event triggers.
> I'm not sold that that was a great design, but it's what's there.

All the deparsers are expected to be able to understand internal
representations of commands, though, such as OIDs and so on.  If you
have a better idea (I don't), we can still rework the API we present to
deparser extensions.

> > That's a different argument, with which I don't necessarily disagree.
> > I think it's not unlikely that a verifier will want to examine the
> > contents of the struct, but I don't think that means we need to expose
> > the struct in amapi.h.
> 
> I think we'd be considerably better off to *not* re-use OpFamilyMember
> in the verifier API.  It's a struct that was only ever designed for
> internal use in CREATE/ALTER OPERATOR CLASS.
>
> I'm kind of inclined to just let the verifiers read the catalogs for
> themselves.  AFAICS, a loop around the results of SearchSysCacheList
> is not going to be significantly more code than what this patch does,
> and it avoids presuming that we know what the verifiers will wish to
> look at.

Hmm, so this amounts to saying the verifier can only run after the
catalog rows are written.  Is that okay?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
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] Parallel Seq Scan

2015-11-02 Thread Robert Haas
On Fri, Oct 30, 2015 at 11:12 PM, Noah Misch  wrote:
> On Wed, Oct 28, 2015 at 01:04:12AM +0100, Robert Haas wrote:
>> Well, OK.  That's not strictly a correctness issue, but here's an
>> updated patch along the lines you suggested.
>
>> Finally, have setup_param_list set a new ParamListInfo field,
>> paramMask, to the parameters actually used in the expression, so that
>> we don't try to fetch those that are not needed when serializing a
>> parameter list.  This isn't necessary for performance, but it makes
>
> s/performance/correctness/
>
>> the performance of the parallel executor code comparable to what we
>> do for cases involving cursors.
>
> With that, the patch is ready.

Thanks, committed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] WIP: Rework access method interface

2015-11-02 Thread Tom Lane
Alvaro Herrera  writes:
> Tom Lane wrote:
>> I'm kind of inclined to just let the verifiers read the catalogs for
>> themselves.  AFAICS, a loop around the results of SearchSysCacheList
>> is not going to be significantly more code than what this patch does,
>> and it avoids presuming that we know what the verifiers will wish to
>> look at.

> Hmm, so this amounts to saying the verifier can only run after the
> catalog rows are written.  Is that okay?

Why not?  Surely we are not interested in performance-optimizing for
the case of a detectably incorrect CREATE OP CLASS command.

I don't actually care that much about running the verifiers during
CREATE/etc OP CLASS at all.  It's at least as important to be able
to run them across the built-in opclasses, considering all the chances
for human error in constructing those things.  Even in the ALTER OP CLASS
case, the verifier would likely need to look at existing catalog rows as
well as the new ones.  So basically, I see zero value in exposing CREATE/
ALTER OP CLASS's internal working representation to the verifiers.

regards, tom lane


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


[HACKERS] Weighted Stats

2015-11-02 Thread David Fetter
Folks,

I'd like to add weighted statistics to PostgreSQL.  While the included
weighted_avg() is trivial to calculate using existing machinery, the
included weighted_stddev_*() functions are not.

I've only done the float8 versions, but if we decide to move forward,
I'd be delighted to add the rest of the numeric types and maybe others
as make sense.

What say?

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4d482ec..2174594 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -12443,6 +12443,29 @@ NULL baz(3 rows)
  
   

+weighted_average
+   
+   
+weighted_avg
+   
+   weighted_avg(value 
expression, weight 
expression)
+  
+  
+   smallint, int,
+   bigint, real, double
+   precision, numeric, or interval
+  
+  
+   numeric for any integer-type argument,
+   double precision for a floating-point argument,
+   otherwise the same as the argument data type
+  
+  the average (arithmetic mean) of all input values, weighted by 
the input weights
+ 
+
+ 
+  
+   
 bit_and

bit_and(expression)
@@ -13086,6 +13109,29 @@ SELECT xmlagg(x) FROM (SELECT x FROM test ORDER BY y 
DESC) AS tab;
  
   

+weighted standard deviation
+population
+   
+   
+weighted_stddev_pop
+   
+   weighted_stddev_pop(value 
expression, weight 
expression)
+  
+  
+   smallint, int,
+   bigint, real, double
+   precision, or numeric
+  
+  
+   double precision for floating-point arguments,
+   otherwise numeric
+  
+  weighted population standard deviation of the input values
+ 
+
+ 
+  
+   
 standard deviation
 sample

@@ -13109,6 +13155,29 @@ SELECT xmlagg(x) FROM (SELECT x FROM test ORDER BY y 
DESC) AS tab;
  
   

+weighted standard deviation
+sample
+   
+   
+weighted_stddev_samp
+   
+   weighted_stddev_samp(value 
expression, weight 
expression)
+  
+  
+   smallint, int,
+   bigint, real, double
+   precision, or numeric
+  
+  
+   double precision for floating-point arguments,
+   otherwise numeric
+  
+  weighted sample standard deviation of the input values
+ 
+
+ 
+  
+   
 variance

variance(expression)
diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c
index 4e927d8..533ce0a 100644
--- a/src/backend/utils/adt/float.c
+++ b/src/backend/utils/adt/float.c
@@ -1774,6 +1774,7 @@ setseed(PG_FUNCTION_ARGS)
  * float8_accum- accumulate for AVG(), variance 
aggregates, etc.
  * float4_accum- same, but input data is float4
  * float8_avg  - produce final result for 
float AVG()
+ * float8_weighted_avg - produce final result for float 
WEIGHTED_AVG()
  * float8_var_samp - produce final result for float 
VAR_SAMP()
  * float8_var_pop  - produce final result for float 
VAR_POP()
  * float8_stddev_samp  - produce final result for float 
STDDEV_SAMP()
@@ -1929,6 +1930,28 @@ float8_avg(PG_FUNCTION_ARGS)
 }
 
 Datum
+float8_weighted_avg(PG_FUNCTION_ARGS)
+{
+   ArrayType  *transarray = PG_GETARG_ARRAYTYPE_P(0);
+   float8 *transvalues;
+   float8  N,
+   sumWX,
+   sumW;
+
+   transvalues = check_float8_array(transarray, "float8_weighted_avg", 6);
+   N = transvalues[0];
+   sumW = transvalues[1];
+   sumWX = transvalues[5];
+
+   if (N < 1.0)
+   PG_RETURN_NULL();
+
+   CHECKFLOATVAL(N, isinf(1.0/sumW) || isinf(sumWX), true);
+
+   PG_RETURN_FLOAT8(sumWX/sumW);
+}
+
+Datum
 float8_var_pop(PG_FUNCTION_ARGS)
 {
ArrayType  *transarray = PG_GETARG_ARRAYTYPE_P(0);
@@ -2467,6 +2490,119 @@ float8_regr_intercept(PG_FUNCTION_ARGS)
PG_RETURN_FLOAT8(numeratorXXY / numeratorX);
 }
 
+/*
+ * ===
+ * WEIGHTED AGGREGATES
+ * ===
+ *
+ * The transition datatype for these aggregates is a 4-element array
+ * of float8, holding the values N, sum(W), sum(W*X), and sum(W*X*X)
+ * in that order.
+ *
+ * First, an accumulator function for those we can't pirate from the
+ * other accumulators.  This accumulator function takes out some of
+ * the rounding error inherent in the general one.
+ * https://en.wikipedia.org/wiki/Standard_deviation#Rapid_calculation_methods
+ *
+ * 

Re: [HACKERS] Freeze avoidance of very large table.

2015-11-02 Thread Robert Haas
On Sat, Oct 31, 2015 at 1:32 AM, Amit Kapila  wrote:
> On Thu, Oct 8, 2015 at 11:05 PM, Simon Riggs  wrote:
>>
>> On 1 October 2015 at 23:30, Josh Berkus  wrote:
>>>
>>> On 10/01/2015 07:43 AM, Robert Haas wrote:
>>> > On Thu, Oct 1, 2015 at 9:44 AM, Fujii Masao 
>>> > wrote:
>>> >> I wonder how much it's worth renaming only the file extension while
>>> >> there are many places where "visibility map" and "vm" are used,
>>> >> for example, log messages, function names, variables, etc.
>>> >
>>> > I'd be inclined to keep calling it the visibility map (vm) even if it
>>> > also contains freeze information.
>>> >
>
> What is your main worry about changing the name of this map, is it
> about more code churn or is it about that we might introduce new issues
> or is it about that people are already accustomed to call this map as
> visibility map?

My concern is mostly that I think calling it the "visibility and
freeze map" is excessively long and wordy.

One observation that someone made previously is that there is a
difference between "all-visible" and "index-only scan OK".  An
all-visible page that has a HOT update is no longer all-visible (it
needs vacuuming) but an index-only scan would still be OK (because
only the non-indexed values in the tuple have changed, and every scan
scan can see either the old or the new tuple but not both.  At
present, the index-only scan will consult the heap page anyway,
because all we know is that the page is not all-visible.  But maybe in
the future somebody will decide to add a bit for that.  Then we'd have
the "visibility, usable for index-only scans, and freeze map", but I
think "_vufiosfm" will not be a good choice for a file suffix.

So similarly here.  The file suffix doesn't need to enumerate all the
bits that are present for each page.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] WIP: Rework access method interface

2015-11-02 Thread Tom Lane
Alvaro Herrera  writes:
> Tom Lane wrote:
>>> ... btw, what is the point of catalog/opfam_internal.h?

> The whole point of splitting the struct declaration to a new header was
> to get a DDL deparser to examine the list of objects being created, so
> that it could construct the deparsed command.  If the struct is opaque
> to the outside world, there's no way to do that (as I recall, you can't
> construct the full command starting from the parsed version only -- you
> need access to the OIDs of the ops/procs actually added to the opclass.)

Hm.  OK, looking closer, I see that we've effectively exposed these
structs as being what is passed to EventTriggerCollectCreateOpClass, so
even if there's not currently any committed code that can read that info,
we clearly need the structs to be accessible to interested event triggers.
I'm not sold that that was a great design, but it's what's there.

>> Regardless of that, I'm a bit skeptical that any of these structs ought
>> to be defined as part of the amapi.h interface.  They seem to be making
>> premature judgments as to what an opclass verifier will care about.  In
>> any case, tying the opclass verifier API to DDL-command implementation
>> details doesn't seem like a good plan.

> That's a different argument, with which I don't necessarily disagree.
> I think it's not unlikely that a verifier will want to examine the
> contents of the struct, but I don't think that means we need to expose
> the struct in amapi.h.

I think we'd be considerably better off to *not* re-use OpFamilyMember
in the verifier API.  It's a struct that was only ever designed for
internal use in CREATE/ALTER OPERATOR CLASS.

I'm kind of inclined to just let the verifiers read the catalogs for
themselves.  AFAICS, a loop around the results of SearchSysCacheList
is not going to be significantly more code than what this patch does,
and it avoids presuming that we know what the verifiers will wish to
look at.

regards, tom lane


-- 
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] Patch to install config/missing

2015-11-02 Thread David E. Wheeler
On Nov 2, 2015, at 1:07 PM, Tom Lane  wrote:

> I wonder how much we need that script at all though.  If, say, configure
> doesn't find bison, what's so wrong with just defining BISON=bison and
> letting the usual shell "bison: command not found" error leak through?

+1 This would certainly make it easier for downstream use cases, as well. Was 
not relishing having to parse the PERL variable to find out if Perl was missing.

David



smime.p7s
Description: S/MIME cryptographic signature


Re: [HACKERS] extend pgbench expressions with functions

2015-11-02 Thread Robert Haas
On Fri, Oct 30, 2015 at 1:01 PM, Fabien COELHO  wrote:
> Here is a v12 which implements the suggestions below.

Sorry it's taken me so long to get around to looking at this.

Some thoughts on an initial read-through:

1. I think there should really be two patches here, the first adding
functions, and the second adding doubles.  Those seem like separate
changes.  And offhand, the double stuff looks a lot less useful that
the function call syntax.

2. ddebug and idebug seem like a lousy idea to me.  If we really need
to be able to print stuff from pgbench, which I kind of doubt, then
maybe we should have a string data type and a print() function, or
maybe a string data type and a toplevel \echo command.

3. I'm perplexed by why you've replaced evaluateExpr() with evalInt()
and evalDouble().  That doesn't seem similar to what I've seen in
other expression-evaluation engines.  Perhaps I could find out by
reading the comments, but actually not, because this entire patch
seems to add only one comment:

+   /* reset column count for this scan */
+   yycol = 0;

While I'm not a fan of excessive commenting, I think a little more
explanation here would be good.

4. The table of functions in pgbench.sgml seems to leave something to
be desired.  We added a pretty detailed write-up on the Gaussian and
exponential options to \setrandom, but exporand() has only this
description:

+  
+   exporand(i,
j, t)
+   integer
+   exponentially distributed random integer in the bounds,
see below
+   exporand(1, 10, 3.0)
+   int between 1 and 10
+  

That's not very helpful.  Without looking at the example, there's no
way to guess what i and j mean, and even with looking at the example,
there's no way to guess what t means.  If, as I'm guessing, exporand()
and guassrand() behave like \setrandom with the exponential and/or
Gaussian options, then the documentation for one of those things
should contain all of the detailed information and the documentation
for the other should refer to it.  More than likely, exporand() and
gaussrand() should get the detailed explanation, and \setrandom should
be document as a deprecated alternative to \set ...
{gauss,expo,}rand(...)

5. I liked Heikki's proposed function names random_gaussian(min, max,
threshold) and random_exponential(min, max, threshold) better than the
ones you've picked here.  I think random() would be OK instead of his
suggestion of random_uniform(), though.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] ALTER ... OWNER TO ... vs. ALTER DEFAULT PRIVILEGES

2015-11-02 Thread Robert Haas
On Sat, Oct 31, 2015 at 10:42 AM, David Fetter  wrote:
> On Sat, Oct 31, 2015 at 12:16:31AM +0100, Robert Haas wrote:
>> On Thu, Oct 29, 2015 at 10:31 PM, David Fetter  wrote:
>> > Had this been part of the original ALTER DEFAULT PRIVILEGES patch,
>> > those privileges would simply have been applied.  Since it wasn't, I'm
>> > ass-u-me'ing that changing the default behavior to that is going to
>> > cause (possibly legitimate) anxiety.
>>
>> The word "applied" is not very clear here.  You want to revoke all
>> existing privileges and then regrant whatever the default privileges
>> would have been given the new owner?  That might be a reasonable thing
>> to have a command for, but doing it automatically on an owner change
>> does not sound like a good idea.  That could be very surprising
>> behavior.
>
> OK, so I think there are operationally useful use cases for
> mix'n'match of the following:
>
> - Clear all existing DEFAULT PRIVILEGES
> - Preserve DEFAULT PRIVILEGES from the previous owner
> - Apply DEFAULT PRIVILEGES for the new owner

I don't believe the privilege grant records in any way whether it came
about because of DEFAULT PRIVILEGES or for some other reason.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] onlyvalue aggregate (was: First Aggregate Funtion?)

2015-11-02 Thread Dean Rasheed
On 2 November 2015 at 10:59, Marko Tiikkaja  wrote:
> On 11/2/15 9:32 AM, Dean Rasheed wrote:
>>
>> On 28 October 2015 at 16:50, Marko Tiikkaja  wrote:
>>>
>>> Here's a patch for the aggregate function outlined by Corey Huinker in
>>> CADkLM=foA_oC_Ri23F9PbfLnfwXFbC3Lt8bBzRu3=cb77g9...@mail.gmail.com .
>>
>>
>> +1. I've wanted something like this a few times. Of the names
>> suggested so far, I think I prefer "single_value", and yes I think it
>> should work with NULLs.
>
>
> This was actually a last-minute design change I made before submitting the
> patch.  The two times I've previously written this aggregate both accepted
> NULLs and only enforced that there must not be more than one non-NULL value,
> but that's only because I was thinking about the "poor man's FILTER" case,
> which is obsolete since version 9.4.  The reason I changed in this version
> is that accepting NULLs can also hide bugs, and it's (usually) easy to
> filter them out with FILTER.
>
> Did you have some specific use case in mind where accepting NULLs would be
> beneficial?
>

I'm not sure what you mean when you say accepting NULLs can hide bugs.
I think that if the input values to the aggregate were
1,1,1,NULL,1,1,1 then it should raise an error. ITSM that that is more
likely to reveal problems with your underlying data or the query. If
you want to ignore NULLs, you can always add a FILTER(WHERE val IS NOT
NULL) clause.

Regards,
Dean


-- 
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] onlyvalue aggregate (was: First Aggregate Funtion?)

2015-11-02 Thread Marko Tiikkaja

On 11/2/15 12:40 PM, Dean Rasheed wrote:

I'm not sure what you mean when you say accepting NULLs can hide bugs.
I think that if the input values to the aggregate were
1,1,1,NULL,1,1,1 then it should raise an error. ITSM that that is more
likely to reveal problems with your underlying data or the query. If
you want to ignore NULLs, you can always add a FILTER(WHERE val IS NOT
NULL) clause.


Ah, I see.  So you're arguing that the aggregate should accept NULLs as 
input, but consider them distinct from any non-NULL values.  I thought 
you meant accepting NULLs and *not* considering them distinct, which 
could easily hide problems.


In that case, I don't oppose to changing the behavior.  I'll make the 
necessary changes.



.m


--
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] WIP: Access method extendability

2015-11-02 Thread Alexander Korotkov
Hi!

Thank you for review!

On Mon, Sep 7, 2015 at 6:41 PM, Teodor Sigaev  wrote:

> Some notices:
>
> 1) create-am.3.patch.gz
>   As I understand, you didn't add schema name to access method. Why?
> Suppose, if we implement SQL-like interface for AM screation/dropping then
> we should provide a schema qualification for them
>

Per subsequent discussion it's unclear why we need to make access methods
schema qualified.


> 2) create-am.3.patch.gz get_object_address_am()
> +   switch (list_length(objname))
> ...
> +   case 2:
> +   catalogname = strVal(linitial(objname));
> +   amname = strVal(lthird(objname));
> ^^ seems, it should be
> lsecond()
>

Fixed.


> 3) create-am.3.patch.gz
>  Suppose, RM_GENERIC_ID is part of generic-xlog.3.patch.gz
>

Fixed.

4) Makefile(s)
> As I can see, object files are lexicographically ordered
>

Fixed.

5) gindesc.c -> genericdesc.c in file header
>

Fixed.


> 6) generic-xlog.3.patch.gz
> I don't like an idea to split START_CRIT_SECTION and END_CRIT_SECTION to
> GenericXLogStart and GenericXLogFinish. User's code could throw a error or
> allocate memory between them and error will become a panic.


Fixed. Now, critical section is only inside GenericXLogFinish.
GenericXLogRegister returns pointer to the page copies which could be
modified. GenericXLogFinish swaps the data between page copy and page
itself. That allow us to avoid critical section in used code.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


generic-xlog.4.patch.gz
Description: GNU Zip compressed data


create-am.4.patch.gz
Description: GNU Zip compressed data


bloom-contrib.4.patch.gz
Description: GNU Zip compressed 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] onlyvalue aggregate (was: First Aggregate Funtion?)

2015-11-02 Thread Dean Rasheed
On 2 November 2015 at 09:10, Simon Riggs  wrote:
> I think we should avoid using ONLY or VALUE within the names since those
> already have other meanings in Postgres.
>

We already have window functions called first_value, last_value and
nth_value, so I think use of "value" in the name is OK (and
consistent). Actually thinking some more, I think the name
"unique_value" is better because it's more suggestive of the fact that
uniqueness is being enforced.

Regards,
Dean


-- 
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] onlyvalue aggregate (was: First Aggregate Funtion?)

2015-11-02 Thread Marko Tiikkaja

On 11/2/15 9:32 AM, Dean Rasheed wrote:

On 28 October 2015 at 16:50, Marko Tiikkaja  wrote:

Here's a patch for the aggregate function outlined by Corey Huinker in
CADkLM=foA_oC_Ri23F9PbfLnfwXFbC3Lt8bBzRu3=cb77g9...@mail.gmail.com .


+1. I've wanted something like this a few times. Of the names
suggested so far, I think I prefer "single_value", and yes I think it
should work with NULLs.


This was actually a last-minute design change I made before submitting 
the patch.  The two times I've previously written this aggregate both 
accepted NULLs and only enforced that there must not be more than one 
non-NULL value, but that's only because I was thinking about the "poor 
man's FILTER" case, which is obsolete since version 9.4.  The reason I 
changed in this version is that accepting NULLs can also hide bugs, and 
it's (usually) easy to filter them out with FILTER.


Did you have some specific use case in mind where accepting NULLs would 
be beneficial?



.m


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


[HACKERS] Re: Why not to use 'pg_ctl start -D ../data' to register posgtresql windows service

2015-11-02 Thread YuanyuanLiu
Yup, I think I totally understand now,  the registered postgres service is
different from  "pg_ctl start".
   To start the postgres windows service needs privileged prompt (to execute
net or sc command) or manually(start or stop in windows service list).After
the service is started, it can be recognized by the SCM and will persist
even if the prompt that started it is closed.
   While,  using "pg_ctl start " needn't privileged prompt(normally prompt
also ok), but after a service started, it cannot recognized by the SCM and
will stop immediatlly when the prompt that started it is closed.
   I think registering postgres windows service is aim to persist the
service when the prompt that started it need to be closed.
   
   I really learned a lot from you, and thank you!
   Regards!

Liu Yuanyuan

 



--
View this message in context: 
http://postgresql.nabble.com/Why-not-to-use-pg-ctl-start-D-data-to-register-posgtresql-windows-service-tp5872282p5872445.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


-- 
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] eXtensible Transaction Manager API

2015-11-02 Thread Konstantin Knizhnik



On 02.11.2015 12:01, Simon Riggs wrote:


At first I was concerned about recovery, but that looks to be covered.

Yes, we have not considered all possible scenarios of working with 
PostgreSQL.
We have tested work with different isolation levels: repeatable read and 
read committed,

but  we have not tested "select for update" and explicit locking.


PostgreSQL assumes that top-level xid commit is atomic, along with all 
of its subtransactions. So the API having access to only Get/Set at 
the xid level would not work. We would 
need TransactionIdSetTreeStatus() rather than just SetStatus. 
GetStatus is not atomic.


XTM API has function SetTransactionStatus but it actually encapsulate 
TransactionIdSetTreeStatus.



adding 3 addition events to transaction commit callbacks:


Those look uncontentious, so we should add those anyway in a sub-patch.

We have also pgDTM implementation which is using timestamps
(system time) as CSNs.
It is also based on the same XTM transaction API.
We will publish it later once we clarify problems with recovery
and performance with this approach.


That is most interesting part, so it needs to be published.


Will be available during this week.


At present, I can't tell whether you need subtrans and multixact calls 
in the API as well. I would imagine we probably do, though we might 
imagine an implementation that didn't support those concepts.


Postgres-XL doesn't support subtransactions. Neither did we at this moment.
Support of subtransactions will be our next step.



Re: [HACKERS] Personal note: changing employers

2015-11-02 Thread Stephen Frost
David,

* David Fetter (da...@fetter.org) wrote:
> It's great that you're doing what you want to do.  I take it that
> Crunchy's model involves more FOSS and less special sauce...

The short answer is yes, Crunchy is committed to FOSS.

Further discussion isn't really appropriate for this list, but feel
free to ping me off-list if you'd like to chat about it.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Re: Why not to use 'pg_ctl start -D ../data' to register posgtresql windows service

2015-11-02 Thread Michael Paquier
On Mon, Nov 2, 2015 at 9:30 PM, Michael Paquier
 wrote:
> On Mon, Nov 2, 2015 at 4:27 PM, YuanyuanLiu  wrote:
>> When we registered postgresql windows service named "postgres-9.4",
>> we can start the postgresql server by manually  starting the windows service
>> , runing command "net stop postgres-9.4" (need user with administrator
>> privilege), or runing command "pg_ctl start -D ../path/to/data".

I think you mean "net start" to start the server here and not "net stop".

> Yep. Those are two examples to stop a registered service. You could
> use "sc stop postgres-9.4" as well. Note also that when a service
> registered is stopped with pg_ctl, SCM will recognize that the service
> has been stopped.

Er... I interpreted "start" by "stop" reading your last sentence.
Please ignore this paragraph.
-- 
Michael


-- 
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] pglogical_output - a general purpose logical decoding output plugin

2015-11-02 Thread Andres Freund
Hi,

On 2015-11-02 20:17:21 +0800, Craig Ringer wrote:
> I'd like to submit pglogical_output for inclusion in the 9.6 series as
> a contrib.

Cool!

> See the README.md and DESIGN.md in the attached patch for details on
> the plugin. I will follow up with a summary in a separate mail, along
> with a few points I'd value input on or want to focus discussion on.

Sounds to me like at least a portion of this should be in sgml, either
in a separate contrib page or in the logical decoding section.

A quick readthrough didn't have a separate description of the
"sub-protocol" in which changes and such are encoded - I think we're
going to need that.

> I anticipate that I'll be following up with a few tweaks, but at this
> point the plugin is feature-complete, documented and substantially
> ready for inclusion as a contrib.

There's a bunch of changes that are hinted at in the files in various
places. Could you extract the ones you think need to be fixed before
integration see in some central place (or just an email)?

Regards,

Andres


-- 
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] eXtensible Transaction Manager API

2015-11-02 Thread Konstantin Knizhnik

On 02.11.2015 06:17, Craig Ringer wrote:

On 31 October 2015 at 17:22, konstantin knizhnik
 wrote:


Waiting for your feedback

For anyone wondering about performance impact, there are some graphs
on page 23 of the PDF presentation. I didn't see anything else, and
the graphs don't seem to cover comparison of Pg with the XTM
transaction manager hooks and no DTM enabled vs Pg without the hooks,
i.e. the hook overhead its self.

Have you done much work on that? Personally I wouldn't expect to see
any meaningful overhead, but I'd really like to have numbers behind
that.


Overhead of indirect call is negligible - see for example
https://gist.github.com/rianhunter/0be8dc116b120ad5fdd4

But we have certainly performed comparison of PostgreSQL with/without 
XTM patch.

Pgbench results are almost the same - within the measurement error:

With XTM:
 transaction type: TPC-B (sort of)
scaling factor: 70
query mode: simple
number of clients: 144
number of threads: 24
duration: 600 s
number of transactions actually processed: 12275179
latency average: 7.037 ms
latency stddev: 46.787 ms
tps = 20456.945469 (including connections establishing)
tps = 20457.164023 (excluding connections establishing)

Without XTM:
transaction type: TPC-B (sort of)
scaling factor: 70
query mode: simple
number of clients: 144
number of threads: 24
duration: 600 s
number of transactions actually processed: 12086367
latency average: 7.156 ms
latency stddev: 48.431 ms
tps = 20114.491785 (including connections establishing)
tps = 20116.074391 (excluding connections establishing)



--
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] Re: Why not to use 'pg_ctl start -D ../data' to register posgtresql windows service

2015-11-02 Thread Michael Paquier
On Mon, Nov 2, 2015 at 4:27 PM, YuanyuanLiu  wrote:
> When we registered postgresql windows service named "postgres-9.4",
> we can start the postgresql server by manually  starting the windows service
> , runing command "net stop postgres-9.4" (need user with administrator
> privilege), or runing command "pg_ctl start -D ../path/to/data".

Yep. Those are two examples to stop a registered service. You could
use "sc stop postgres-9.4" as well. Note also that when a service
registered is stopped with pg_ctl, SCM will recognize that the service
has been stopped.

> The started postgresql service must be stopped in the same session.
> Is this right ?

When a service has been registered to the Windows SCM, a service
started with sc or net will persist even if the prompt that started it
is closed. Now, if you start a service with "pg_ctl start -d data",
even if the service is registered it will *not* by recognized by SCM
as started. And this one will stop immediately once the prompt that
started it is closed.
-- 
Michael


-- 
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] September 2015 Commitfest

2015-11-02 Thread Michael Paquier
On Sun, Nov 1, 2015 at 7:36 PM, Michael Paquier
 wrote:
> On Sun, Nov 1, 2015 at 1:53 AM, Marko Tiikkaja  wrote:
>> On 10/31/15 12:42 AM, Michael Paquier wrote:
>>>
>>> So, seeing nothing happening I have done the above, opened 2015-11 CF
>>> and closed the current one.
>>
>>
>> Are we doing these in an Australian time zone now?  It was quite unpleasant
>> to find that the 2015-11 is "in progress" already and two of my patches will
>> not be in there.  AFAIR the submission deadline used to be around UTC
>> midnight of the first day of the month the CF nominally begins on.
>
> Er, well. Sorry for that... I did all this stuff on Friday evening
> before leaving back for Japan with not much time on the table. I have
> switched the CF back to an open status for now. And I'll switch it
> back to in-progress in 24 hours. If there are patches you would like
> attached to the CF app don't hesitate to ping me.

And now CF begins officially. The axe has fallen as promised 26 hours after.
-- 
Michael


-- 
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] pglogical_output - a general purpose logical decoding output plugin

2015-11-02 Thread Craig Ringer
On 2 November 2015 at 20:35, Andres Freund  wrote:
> On 2015-11-02 20:17:21 +0800, Craig Ringer wrote:
>
>> See the README.md and DESIGN.md in the attached patch for details on
>> the plugin. I will follow up with a summary in a separate mail, along
>> with a few points I'd value input on or want to focus discussion on.
>
> Sounds to me like at least a portion of this should be in sgml, either
> in a separate contrib page or in the logical decoding section.

Yes, I think so. Before rewriting to SGML I wanted to solicit opinions
on what should be hidden away in a for-developers README file and what
parts deserve exposure in the public docs.

> A quick readthrough didn't have a separate description of the
> "sub-protocol" in which changes and such are encoded - I think we're
> going to need that.

It didn't quite make the first cut as I have to make a couple of edits
to reflect late changes. I should be able to follow up with that later
today.

The protocol design documentation actually predates the plugin its
self, though it saw a few changes where it became clear something
wouldn't work as envisioned. It's been quite a pleasure starting with
a detailed design, then implementing it.

> There's a bunch of changes that are hinted at in the files in various
> places. Could you extract the ones you think need to be fixed before
> integration see in some central place (or just an email)?

Yep, writing that up at the moment. I didn't want to make the initial
post too verbose.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


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


[HACKERS] Building from git source on ubuntu with gssapi

2015-11-02 Thread Jeff Janes
I can't ./configure --with-gssapi from git on ubuntu 14.04.3 because:

configure: error: gssapi.h header file is required for GSSAPI

If I download the distribution-specific 9.3 source with apt, I find
their secret sauce to make it work:

./debian/rules:LDFLAGS+= -Wl,--as-needed -L/usr/lib/mit-krb5
-L/usr/lib/$(DEB_HOST_MULTIARCH)/mit-krb5

./debian/rules:CFLAGS+= -fPIC -pie -I/usr/include/mit-krb5


Usually the packagers' secret sauce is there to change the
installation locations and defaults and such, not to allow it to
configure and compile at all.  It makes it a bit hard to test new code
if you can't compile it without a bunch of messing around.

Is there something we can and should do to make this compile directly
out of git?

Cheers,

Jeff


-- 
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] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions

2015-11-02 Thread Amit Kapila
On Mon, Nov 2, 2015 at 10:45 PM, Pavel Stehule 
wrote:
>
>
> It is 100% true. But the users can do strange things. If we solve idle
> transactions and not idle session, then they are able to increase
> max_connections to thousands with happy smile in face.
>
> I have not strong idea about how to solve it well - maybe introduce
> transaction_idle_timeout and session_idle_timeout?
>
>
What exactly do we want to define session_idle_timeout?  Some
possibilities:
a. Reset the session related variables like transaction, prepared
statements, etc. and retain it for connection pool kind of stuff
b. Exit from the session

If we want something on lines of option (a), then I think it is better
to have just a single time out (session_idle_timeout/idle_timeout)



With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] [BUGS] BUG #12989: pg_size_pretty with negative values

2015-11-02 Thread Robert Haas
On Sat, Oct 31, 2015 at 2:25 PM, Julien Rouhaud
 wrote:
> I just reviewed your patch, everything looks fine for me. Maybe some
> minor cosmetic changes could be made to avoid declaring too many vars,
> but I think a committer would have a better idea on this, so I mark
> this patch as ready for committer.

I don't think we should define Sign(x) as a macro in c.h. c.h is
really only supposed to contain stuff that's pretty generic and
universal, and the fact that we haven't needed it up until now
suggests that Sign(x) isn't.  I'd suggest instead defining something
like:

#define half_rounded(x)   (((x) + (x) < 0 ? 0 : 1) / 2)

Maybe rename numeric_divide_by_two to numeric_half_rounded.
Generally, let's try to make the numeric and int64 code paths look as
similar as possible.

Recomputing numeric_absolute(x) multiple times should be avoided.
Compute it once and save the answer.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] proposal: multiple psql option -c

2015-11-02 Thread Robert Haas
On Sat, Oct 31, 2015 at 2:50 PM, Pavel Stehule  wrote:
> fixed patch attached

The documentation included in this patch doesn't really make it clear
why -g is different from or better than -c.


-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Robert Haas
On Mon, Nov 2, 2015 at 10:13 PM, Amit Kapila  wrote:
> I think that is the sensible way to deal with this and any other such
> parameters.  We already have a way to disallow setting of individual
> parameters (GUC_DISALLOW_IN_AUTO_FILE) via Alter System.
> Currently we disallow to set data_directory via this mechanism and I think
> we can do the same for other parameters if required.  Do you think we
> should do some investigation/analysis about un-safe parameters rather
> then doing it in retail fashion?

-1.

This was discussed before, and I feel about it now the same way I felt
about it then: disallowing all GUCs that could potentially cause the
server not to start would make ALTER SYSTEM a whole lot less useful.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] Freeze avoidance of very large table.

2015-11-02 Thread Amit Kapila
On Tue, Nov 3, 2015 at 5:04 AM, Robert Haas  wrote:
>
> On Sat, Oct 31, 2015 at 1:32 AM, Amit Kapila 
wrote:
> >
> > What is your main worry about changing the name of this map, is it
> > about more code churn or is it about that we might introduce new issues
> > or is it about that people are already accustomed to call this map as
> > visibility map?
>
> My concern is mostly that I think calling it the "visibility and
> freeze map" is excessively long and wordy.
>
> One observation that someone made previously is that there is a
> difference between "all-visible" and "index-only scan OK".  An
> all-visible page that has a HOT update is no longer all-visible (it
> needs vacuuming) but an index-only scan would still be OK (because
> only the non-indexed values in the tuple have changed, and every scan
> scan can see either the old or the new tuple but not both.  At
> present, the index-only scan will consult the heap page anyway,
> because all we know is that the page is not all-visible.  But maybe in
> the future somebody will decide to add a bit for that.  Then we'd have
> the "visibility, usable for index-only scans, and freeze map", but I
> think "_vufiosfm" will not be a good choice for a file suffix.
>

I think in that case we can call it as page info map or page state map, but
I find retaining visibility map name in this case or for future (if we want
to
add another bit) as confusing.  In-fact if you find "visibility and freeze
map",
as excessively long, then we can change it to "page info map" or "page state
map" now as well.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] a raft of parallelism-related bug fixes

2015-11-02 Thread Robert Haas
On Wed, Oct 28, 2015 at 10:23 AM, Robert Haas  wrote:
> On Sun, Oct 18, 2015 at 12:17 AM, Robert Haas  wrote:
>>> So reviewing patch 13 isn't possible without prior knowledge.
>>
>> The basic question for patch 13 is whether ephemeral record types can
>> occur in executor tuples in any contexts that I haven't identified.  I
>> know that a tuple table slot can contain have a column that is of type
>> record or record[], and those records can themselves contain
>> attributes of type record or record[], and so on as far down as you
>> like.  I *think* that's the only case.  For example, I don't believe
>> that a TupleTableSlot can contain a *named* record type that has an
>> anonymous record buried down inside of it somehow.  But I'm not
>> positive I'm right about that.
>
> I have done some more testing and investigation and determined that
> this optimism was unwarranted.  It turns out that the type information
> for composite and record types gets stored in two different places.
> First, the TupleTableSlot has a type OID, indicating the sort of the
> value it expects to be stored for that slot attribute.  Second, the
> value itself contains a type OID and typmod.  And these don't have to
> match.  For example, consider this query:
>
> select row_to_json(i) from int8_tbl i(x,y);
>
> Without i(x,y), the HeapTuple passed to row_to_json is labelled with
> the pg_type OID of int8_tbl.  But with the query as written, it's
> labeled as an anonymous record type.  If I jigger things by hacking
> the code so that this is planned as Gather (single-copy) -> SeqScan,
> with row_to_json evaluated at the Gather node, then the sequential
> scan kicks out a tuple with a transient record type and stores it into
> a slot whose type OID is still that of int8_tbl.  My previous patch
> failed to deal with that; the attached one does.
>
> The previous patch was also defective in a few other respects.  The
> most significant of those, maybe, is that it somehow thought it was OK
> to assume that transient typmods from all workers could be treated
> interchangeably rather than individually.  To fix this, I've changed
> the TupleQueueFunnel implemented by tqueue.c to be merely a
> TupleQueueReader which handles reading from a single worker only.
> nodeGather.c therefore creates one TupleQueueReader per worker instead
> of a single TupleQueueFunnel for all workers; accordingly, the logic
> for multiplexing multiple queues now lives in nodeGather.c.  This is
> probably how I should have done it originally - someone, I think Jeff
> Davis - complained previously that tqueue.c had no business embedding
> the round-robin policy decision, and he was right.  So this addresses
> that complaint as well.

Here is an updated version.  This is rebased over recent commits, and
I added a missing check for attisdropped.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From fa31300a884cc942d22c66d6a30fa4c2fcba3c6f Mon Sep 17 00:00:00 2001
From: Robert Haas 
Date: Wed, 7 Oct 2015 12:43:22 -0400
Subject: [PATCH 5/5] Modify tqueue infrastructure to support transient record
 types.

Commit 4a4e6893aa080b9094dadbe0e65f8a75fee41ac6, which introduced this
mechanism, failed to account for the fact that the RECORD pseudo-type
uses transient typmods that are only meaningful within a single
backend.  Transferring such tuples without modification between two
cooperating backends does not work.  This commit installs a system
for passing the tuple descriptors over the same shm_mq being used to
send the tuples themselves.  The two sides might not assign the same
transient typmod to any given tuple descriptor, so we must also
substitute the appropriate receiver-side typmod for the one used by
the sender.  That adds some CPU overhead, but still seems better than
being unable to pass records between cooperating parallel processes.

Along the way, move the logic for handling multiple tuple queues from
tqueue.c to nodeGather.c; tqueue.c now provides a TupleQueueReader,
which reads from a single queue, rather than a TupleQueueFunnel, which
potentially reads from multiple queues.  This change was suggested
previously as a way to make sure that nodeGather.c rather than tqueue.c
had policy control over the order in which to read from queues, but
it wasn't clear to me until now how good an idea it was.  typmod
mapping needs to be performed separately for each queue, and it is
much simpler if the tqueue.c code handles that and leaves multiplexing
multiple queues to higher layers of the stack.
---
 src/backend/executor/nodeGather.c | 138 --
 src/backend/executor/tqueue.c | 983 +-
 src/include/executor/tqueue.h |  12 +-
 src/include/nodes/execnodes.h |   4 +-
 src/tools/pgindent/typedefs.list  |   2 +-
 5 files changed, 986 insertions(+), 153 deletions(-)

diff --git a/src/backend/executor/nodeGather.c 

Re: [HACKERS] Building from git source on ubuntu with gssapi

2015-11-02 Thread Robbie Harwood
Jeff Janes  writes:

> I can't ./configure --with-gssapi from git on ubuntu 14.04.3 because:
>
> configure: error: gssapi.h header file is required for GSSAPI
>
> If I download the distribution-specific 9.3 source with apt, I find
> their secret sauce to make it work:
>
> ./debian/rules:LDFLAGS+= -Wl,--as-needed -L/usr/lib/mit-krb5
> -L/usr/lib/$(DEB_HOST_MULTIARCH)/mit-krb5
>
> ./debian/rules:CFLAGS+= -fPIC -pie -I/usr/include/mit-krb5
>
>
> Usually the packagers' secret sauce is there to change the
> installation locations and defaults and such, not to allow it to
> configure and compile at all.  It makes it a bit hard to test new code
> if you can't compile it without a bunch of messing around.
>
> Is there something we can and should do to make this compile directly
> out of git?

The preferred way to check for - and handle - GSSAPI/krb5 is with
pkg-config.  So, for instance, on my system:

rharwood@thriss:~$ pkg-config --exists krb5-gssapi
rharwood@thriss:~$ echo $?
0
rharwood@thriss:~$ pkg-config --libs krb5-gssapi
-L/usr/lib/x86_64-linux-gnu/mit-krb5 -lgssapi_krb5
rharwood@thriss:~$ pkg-config --cflags krb5-gssapi
-isystem /usr/include/mit-krb5 -isystem /usr/include/mit-krb5

In the past, this was done using krb5-config, but this has become
increasingly difficult to continue to make work (and krb5-config is not
nice code to work with).


signature.asc
Description: PGP signature


Re: [HACKERS] Building from git source on ubuntu with gssapi

2015-11-02 Thread Tom Lane
Jeff Janes  writes:
> I can't ./configure --with-gssapi from git on ubuntu 14.04.3 because:
> configure: error: gssapi.h header file is required for GSSAPI

> If I download the distribution-specific 9.3 source with apt, I find
> their secret sauce to make it work:
> ./debian/rules:LDFLAGS+= -Wl,--as-needed -L/usr/lib/mit-krb5
> -L/usr/lib/$(DEB_HOST_MULTIARCH)/mit-krb5
> ./debian/rules:CFLAGS+= -fPIC -pie -I/usr/include/mit-krb5

> Usually the packagers' secret sauce is there to change the
> installation locations and defaults and such, not to allow it to
> configure and compile at all.

Huh?  Platform-specific additions to -I and -L paths are quite common,
especially on platforms that don't have a policy of forcing installation
directly into /usr/include and /usr/lib whenever possible.

> Is there something we can and should do to make this compile directly
> out of git?

Don't think so.  It's up to the caller of configure to tell us where
to look, if there are places that aren't in the compiler's default
search paths.  If we tried to guess such paths, we'd end up destabilizing
about as many builds as we fixed.

regards, tom lane


-- 
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] ALTER SYSTEM vs symlink

2015-11-02 Thread Amit Kapila
On Mon, Nov 2, 2015 at 10:17 PM, Tom Lane  wrote:
>
> Stephen Frost  writes:
> > * Tom Lane (t...@sss.pgh.pa.us) wrote:
> >> How is it that we don't need rolcatupdate but we do need a way to shut
> >> off ALTER SYSTEM?  Doesn't compute, IMO.
>
> > I'd like the ability to control all of the above, ultimately.  I don't
> > believe that we should be allowing the superuser to always modify the
> > catalog directly- and things like the sepgsql module can actually
> > address that and limit when the superuser is allowed to with better
> > granularity then what rolcatupdate provided (or was ever likely to
> > provide, being a single boolean role attribute).
>
> Mumble.  I have no objection to sepgsql deciding to disallow ALTER SYSTEM
> --- after all, the entire point of that module is to enforce arbitrary
> annoying restrictions ;-).  But I am not convinced that we need any other
> way to turn it off.  As Robert points out, it's far *less* dangerous than
> most other superuser-only features.
>
> Also, disallowing ALTER SYSTEM altogether strikes me as an extremely
> brute-force solution to any of the specific issues you mention.  If you're
> worried about locking down shared_preload_libraries, for example, it would
> be far better to lock down just that one variable.
>

I think that is the sensible way to deal with this and any other such
parameters.  We already have a way to disallow setting of individual
parameters (GUC_DISALLOW_IN_AUTO_FILE) via Alter System.
Currently we disallow to set data_directory via this mechanism and I think
we can do the same for other parameters if required.  Do you think we
should do some investigation/analysis about un-safe parameters rather
then doing it in retail fashion?


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] ALTER SYSTEM vs symlink

2015-11-02 Thread Tom Lane
Robert Haas  writes:
> On Mon, Nov 2, 2015 at 10:13 PM, Amit Kapila  wrote:
>> I think that is the sensible way to deal with this and any other such
>> parameters.  We already have a way to disallow setting of individual
>> parameters (GUC_DISALLOW_IN_AUTO_FILE) via Alter System.
>> Currently we disallow to set data_directory via this mechanism and I think
>> we can do the same for other parameters if required.  Do you think we
>> should do some investigation/analysis about un-safe parameters rather
>> then doing it in retail fashion?

> -1.

> This was discussed before, and I feel about it now the same way I felt
> about it then: disallowing all GUCs that could potentially cause the
> server not to start would make ALTER SYSTEM a whole lot less useful.

I definitely agree that unconditionally disallowing "unsafe" parameters
in ALTER SYSTEM would be counterproductive.  data_directory is disallowed
there only because it's nonsensical: you can't even find the auto.conf
file until you've settled on the data_directory.

However, where I thought this discussion was going was to allow admins
to selectively disable particular parameters from being set that way.
Whether a particular installation finds locking down
shared_preload_libraries to be more useful than allowing it to be set
doesn't seem to me to be a one-size-fits-all question.  So at least in
principle I'd be okay with a feature to control that.  But maybe it's
sufficient to say "you can use sepgsql to restrict that".  Not every
feature needs to be in core.

regards, tom lane


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