Re: [HACKERS] Extracting fields from 'infinity'::TIMESTAMP[TZ]

2016-01-05 Thread Vitaly Burovoy
On 1/4/16, Alvaro Herrera  wrote:
> Vitaly Burovoy wrote:
>
>> Majority of the votes for NULL for "other things" except epoch.
>> Nobody answers about differences between monotonic and oscillating
>> values.
>>
>> I suppose behavior of monotonic values (julian, century, decade,
>> isoyear, millennium and year) should be the same as for epoch (which
>> obviously also monotonic value).
>> Proposed patch has that behavior: +/-infinity for epoch, julian,
>> century, decade, isoyear, millennium and year; NULL for other fields.
>
> It seems we got majority approval on the design of this patch, and no
> disagreement; the last submitted version appears to implement that.
> There's no documentation change in the patch though.  I'm marking it as
> Waiting on Author; please resubmit with necessary doc changes.
>
> Thanks,
>
> --
> Álvaro Herrerahttp://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>

Thank you!
Version 3 of the patch with touched documentation in the attachment.

I decided to mark it as a note, because that separation
(monotonic/oscillation fields) is not obvious and for most values the
function "extract" works as expected (e.g. does not give an error)
until special values are (casually?) passed.
-- 
Best regards,
Vitaly Burovoy


extract_from_infinite_timestamp-v3.patch
Description: Binary data

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


[HACKERS] Inconsistent error handling in START_REPLICATION command

2016-01-05 Thread Shulgin, Oleksandr
Hackers,

It looks like there's an inconsistency in error handling during
START_REPLICATION command of replication protocol:

$ psql postgres://localhost/psycopg2test?replication=database
psql (9.6devel)
Type "help" for help.

psycopg2test=# IDENTIFY_SYSTEM;
  systemid   | timeline |  xlogpos  |dbname
-+--+---+--
 6235978519197579707 |1 | 0/2CE0F78 | psycopg2test
(1 row)

psycopg2test=# START_REPLICATION SLOT "TEST1" LOGICAL 0/0 ("invalid"
"value");
ERROR:  syntax error

1) syntax errors are reported and client can retry with corrected command:

psycopg2test=# START_REPLICATION SLOT "TEST1" LOGICAL 0/0 ("invalid"
'value');
ERROR:  replication slot name "TEST1" contains invalid character
HINT:  Replication slot names may only contain lower case letters, numbers,
and the underscore character.

2) further errors are reported and we can retry:

psycopg2test=# START_REPLICATION SLOT "test1" LOGICAL 0/0 ("invalid"
'value');
ERROR:  replication slot "test1" does not exist

psycopg2test=# CREATE_REPLICATION_SLOT "test1" LOGICAL "test_decoding";
 slot_name | consistent_point | snapshot_name | output_plugin
---+--+---+---
 test1 | 0/2CF5340| 088C-1| test_decoding
(1 row)

psycopg2test=# START_REPLICATION SLOT "test1" LOGICAL 0/0 ("invalid"
'value');
unexpected PQresultStatus: 8

The last command results in the following output sent to the server log:

ERROR:  option "invalid" = "value" is unknown
CONTEXT:  slot "test1", output plugin "test_decoding", in the startup
callback

But the client has no way to learn about the error, nor can it restart with
correct one (the server has entered COPY protocol mode):

psycopg2test=# START_REPLICATION SLOT "test1" LOGICAL 0/0;
PQexec not allowed during COPY BOTH

I recall Craig Rigner mentioning this issue in context of the
pglogical_output plugin, but I thought that was something to do with the
startup packet.  The behavior above doesn't strike me as very consistent,
we should be able to detect and report errors during output plugin startup
and let the client retry with the corrected command as we do for syntax or
other errors.

I didn't look in the code yet, but if someone knows off top of the head the
reason to this behavior, I'd be glad to learn it.

Cheers.
-- 
*Oleksandr "Alex" Shulgin*
*Database Engineer*

Mobile: +49 160 84-90-639
Email: oleksandr.shul...@zalando.de


Re: [HACKERS] Inconsistent error handling in START_REPLICATION command

2016-01-05 Thread Shulgin, Oleksandr
On Tue, Jan 5, 2016 at 10:39 AM, Shulgin, Oleksandr <
oleksandr.shul...@zalando.de> wrote:

> Hackers,
>
> It looks like there's an inconsistency in error handling during
> START_REPLICATION command of replication protocol:
>
> $ psql postgres://localhost/psycopg2test?replication=database
> psql (9.6devel)
> Type "help" for help.
>
> psycopg2test=# IDENTIFY_SYSTEM;
>   systemid   | timeline |  xlogpos  |dbname
> -+--+---+--
>  6235978519197579707 |1 | 0/2CE0F78 | psycopg2test
> (1 row)
>
> psycopg2test=# START_REPLICATION SLOT "TEST1" LOGICAL 0/0 ("invalid"
> "value");
> ERROR:  syntax error
>
> 1) syntax errors are reported and client can retry with corrected command:
>
> psycopg2test=# START_REPLICATION SLOT "TEST1" LOGICAL 0/0 ("invalid"
> 'value');
> ERROR:  replication slot name "TEST1" contains invalid character
> HINT:  Replication slot names may only contain lower case letters,
> numbers, and the underscore character.
>
> 2) further errors are reported and we can retry:
>
> psycopg2test=# START_REPLICATION SLOT "test1" LOGICAL 0/0 ("invalid"
> 'value');
> ERROR:  replication slot "test1" does not exist
>
> psycopg2test=# CREATE_REPLICATION_SLOT "test1" LOGICAL "test_decoding";
>  slot_name | consistent_point | snapshot_name | output_plugin
> ---+--+---+---
>  test1 | 0/2CF5340| 088C-1| test_decoding
> (1 row)
>
> psycopg2test=# START_REPLICATION SLOT "test1" LOGICAL 0/0 ("invalid"
> 'value');
> unexpected PQresultStatus: 8
>
> The last command results in the following output sent to the server log:
>
> ERROR:  option "invalid" = "value" is unknown
> CONTEXT:  slot "test1", output plugin "test_decoding", in the startup
> callback
>
> But the client has no way to learn about the error, nor can it restart
> with correct one (the server has entered COPY protocol mode):
>
> psycopg2test=# START_REPLICATION SLOT "test1" LOGICAL 0/0;
> PQexec not allowed during COPY BOTH
>
> I recall Craig Rigner mentioning this issue in context of the
> pglogical_output plugin, but I thought that was something to do with the
> startup packet.  The behavior above doesn't strike me as very consistent,
> we should be able to detect and report errors during output plugin startup
> and let the client retry with the corrected command as we do for syntax or
> other errors.
>
> I didn't look in the code yet, but if someone knows off top of the head
> the reason to this behavior, I'd be glad to learn it.
>

Looks like the attached patch fixes it for me:

psycopg2test=# START_REPLICATION SLOT "test1" LOGICAL 0/0 ("invalid"
'value');
ERROR:  option "invalid" = "value" is unknown
CONTEXT:  slot "test1", output plugin "test_decoding", in the startup
callback

psycopg2test=# START_REPLICATION SLOT "test1" LOGICAL 0/0;
unexpected PQresultStatus: 8

There was a similar problem with PHYSICAL replication--if
the requested start LSN is in the future, the error is not reported to the
client and connection is unusable:

psycopg2test=# START_REPLICATION SLOT "test2" PHYSICAL 1FF0/0;
unexpected PQresultStatus: 8

psycopg2test=# START_REPLICATION SLOT "test2" PHYSICAL 0/0;
PQexec not allowed during COPY BOTH

After the patch:

psycopg2test=# START_REPLICATION SLOT "test2" PHYSICAL 1FF0/0;
ERROR:  requested starting point 1FF0/0 is ahead of the WAL flush position
of this server 0/2DE6C28

psycopg2test=# START_REPLICATION SLOT "test2" PHYSICAL 0/0;
unexpected PQresultStatus: 8


Actually, would it make sense to move the block that sends CopyBothResponse
message to the WalSndLoop() instead?  I think you can't get any closer than
that...

--
Alex
From 4ebfa71a1d4e4d43415ad9c2a8142f45444313ec Mon Sep 17 00:00:00 2001
From: Oleksandr Shulgin 
Date: Tue, 5 Jan 2016 11:34:37 +0100
Subject: [PATCH] Move CopyBothResponse closer to WalSndLoop()

This is required in order for any initialization errors to be properly
reported to the client and to avoid entering COPY BOTH mode
prematurely (the client will not be able to correct the error after
that even if it could learn about it somehow).
---
 src/backend/replication/walsender.c | 39 -
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index c03e045..3b83db7 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -629,13 +629,6 @@ StartReplication(StartReplicationCmd *cmd)
 		 */
 		WalSndSetState(WALSNDSTATE_CATCHUP);
 
-		/* Send a CopyBothResponse message, and start streaming */
-		pq_beginmessage(, 'W');
-		pq_sendbyte(, 0);
-		pq_sendint(, 0, 2);
-		pq_endmessage();
-		pq_flush();
-
 		/*
 		 * Don't allow a request to stream from a future point in WAL that
 		 * hasn't been flushed to disk in this server yet.
@@ -667,6 +660,18 @@ StartReplication(StartReplicationCmd *cmd)
 		/* 

Re: [HACKERS] GIN data corruption bug(s) in 9.6devel

2016-01-05 Thread Tomas Vondra

Hi,

On 12/23/2015 09:33 PM, Jeff Janes wrote:

On Mon, Dec 21, 2015 at 11:51 AM, Tomas Vondra
 wrote:



On 12/21/2015 07:41 PM, Jeff Janes wrote:


On Sat, Dec 19, 2015 at 3:19 PM, Tomas Vondra
 wrote:



...


So both patches seem to do the trick, but (2) is faster. Not sure
if this is expected. (BTW all the results are without asserts
enabled).



Do you know what the size of the pending list was at the end of each
test?

I think last one may be faster because it left a large mess behind
that someone needs to clean up later.



No. How do I measure it?


pageinspect's gin_metapage_info, or pgstattuple's pgstatginindex


Hmmm, so this turns out not very useful, because at the end the data I 
get from gin_metapage_info is almost exactly the same for both patches 
(more details below).






Also, do you have the final size of the indexes in each case?



No, I haven't realized the patches do affect that, so I haven't measured it.


There shouldn't be a difference between the two approaches (although I
guess there could be if one left a larger pending list than the other,
as pending lists is very space inefficient), but since you included
9.5 in your test I thought it would be interesting to see how either
patched version under 9.6 compared to 9.5.


Well, turns out there's a quite significant difference, actually. The 
index sizes I get (quite stable after multiple runs):


   9.5 : 2428 MB
   9.6 + alone cleanup : 730 MB
   9.6 + pending lock : 488 MB

So that's quite a significant difference, I guess. The load duration for 
each version look like this:


   9.5 : 1415 seconds
   9.6 + alone cleanup : 1310 seconds
   9.6 + pending lock  : 1380 seconds

I'd say I'm happy with sacrificing ~5% of time in exchange for ~35% 
reduction of index size.


The size of the index on 9.5 after VACUUM FULL (so pretty much the 
smallest index possible) is 440MB, which suggests the "pending lock" 
patch does a quite good job.


The gin_metapage_info at the end of one of the runs (pretty much all the 
runs look exactly the same) looks like this:


  pending lock   alone cleanup  9.5

 pending_head2   2   310460
 pending_tail  338 345   310806
 tail_free_size812 812  812
 n_pending_pages   330 339  347
 n_pending_tuples 10031037 1059
 n_total_pages   2   22
 n_entry_pages   1   11
 n_data_pages0   00
 n_entries   0   00
 version 2   22

So almost no difference, except for the pending_* attributes, and even 
in that case the values are only different for 9.5 branch. Not sure what 
conclusion to draw from this - maybe it's necessary to collect the 
function input while the load is running (but that'd be tricky to 
process, I guess).


regards

--
Tomas Vondra  http://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] Building pg_xlogdump reproducibly

2016-01-05 Thread Christoph Berg
Re: Alvaro Herrera 2016-01-04 <20160104175623.GA170910@alvherre.pgsql>
> > https://reproducible.debian.net/dbd/unstable/armhf/postgresql-9.4_9.4.5-2.diffoscope.html

9.5 was already tested as well, I just couldn't find the link
yesterday:

https://reproducible.debian.net/rb-pkg/experimental/armhf/postgresql-9.5.html

Again, the only real difference between the two builds there is in
pg_xlogdump, the remaining differences are about the checksums of the
surrounding containers (.deb files are really ar files containing
tarballs).

> Ugh.  I guess this output is helpful enough given that it mentions the
> offending executable; since our Makefiles are simple enough, we
> shouldn't have much trouble finding the problem spot.  I do wonder if
> the CMake conversion is going to cause problems.

cmake is writing makefiles, so I wouldn't expect much problems. (But
it could be the case that problems are harder to fix.)

> > > At least transform modules added in 9.5 (hstore_plpython et al) look
> > > like they might similar issues.
> 
> Hmm.  hstore_plperl uses $(wildcard) but only in the AIX and Win32
> cases, unless I'm misreading.
> 
> I don't see any other $(wildcard) used to build executables; it's used
> for tests and flags in many places, but that shouldn't matter.

Nod. Attached is a patch that covers all relevant $(wildcard)
occurrences in Makefiles for devel.

 contrib/hstore_plperl/Makefile   |2 !!
 contrib/hstore_plpython/Makefile |4 
 contrib/ltree_plpython/Makefile  |4 
 src/bin/pg_xlogdump/Makefile |2 !!
 4 files changed, 12 modifications(!)

Mit freundlichen Grüßen,
Christoph Berg
-- 
Senior Berater, Tel.: +49 (0)21 61 / 46 43-187
credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209
Hohenzollernstr. 133, 41061 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
pgp fingerprint: 5C48 FE61 57F4 9179 5970  87C6 4C5A 6BAB 12D2 A7AE
diff --git a/contrib/hstore_plperl/Makefile b/contrib/hstore_plperl/Makefile
new file mode 100644
index 8f7b171..b3b8654
*** a/contrib/hstore_plperl/Makefile
--- b/contrib/hstore_plperl/Makefile
*** endif
*** 32,38 
  ifeq ($(PORTNAME), win32)
  # these settings are the same as for plperl
  override CPPFLAGS += -DPLPERL_HAVE_UID_GID -Wno-comment
! SHLIB_LINK += ../hstore/libhstore.a $(wildcard ../../src/pl/plperl/libperl*.a)
  endif
  
  ifeq ($(PORTNAME), cygwin)
--- 32,38 
  ifeq ($(PORTNAME), win32)
  # these settings are the same as for plperl
  override CPPFLAGS += -DPLPERL_HAVE_UID_GID -Wno-comment
! SHLIB_LINK += ../hstore/libhstore.a $(sort $(wildcard ../../src/pl/plperl/libperl*.a))
  endif
  
  ifeq ($(PORTNAME), cygwin)
diff --git a/contrib/hstore_plpython/Makefile b/contrib/hstore_plpython/Makefile
new file mode 100644
index 2de00a2..c4dad6f
*** a/contrib/hstore_plpython/Makefile
--- b/contrib/hstore_plpython/Makefile
*** endif
*** 27,36 
  # dependency.  This does preclude pgxs builds.
  ifeq ($(PORTNAME), aix)
  rpathdir = $(pkglibdir):$(python_libdir)
! SHLIB_LINK += ../hstore/libhstore.exp $(python_libspec) $(python_additional_libs) $(wildcard ../../src/pl/plpython/libplpython*.exp)
  endif
  ifeq ($(PORTNAME), win32)
! SHLIB_LINK += ../hstore/libhstore.a $(wildcard ../../src/pl/plpython/libpython*.a) $(wildcard ../../src/pl/plpython/libplpython*.a)
  endif
  
  ifeq ($(PORTNAME), cygwin)
--- 27,36 
  # dependency.  This does preclude pgxs builds.
  ifeq ($(PORTNAME), aix)
  rpathdir = $(pkglibdir):$(python_libdir)
! SHLIB_LINK += ../hstore/libhstore.exp $(python_libspec) $(python_additional_libs) $(sort $(wildcard ../../src/pl/plpython/libplpython*.exp))
  endif
  ifeq ($(PORTNAME), win32)
! SHLIB_LINK += ../hstore/libhstore.a $(sort $(wildcard ../../src/pl/plpython/libpython*.a)) $(sort $(wildcard ../../src/pl/plpython/libplpython*.a))
  endif
  
  ifeq ($(PORTNAME), cygwin)
diff --git a/contrib/ltree_plpython/Makefile b/contrib/ltree_plpython/Makefile
new file mode 100644
index 7eacb40..08186f1
*** a/contrib/ltree_plpython/Makefile
--- b/contrib/ltree_plpython/Makefile
*** endif
*** 27,36 
  # dependency.  This does preclude pgxs builds.
  ifeq ($(PORTNAME), aix)
  rpathdir = $(pkglibdir):$(python_libdir)
! SHLIB_LINK += $(python_libspec) $(python_additional_libs) $(wildcard ../../src/pl/plpython/libplpython*.exp)
  endif
  ifeq ($(PORTNAME), win32)
! SHLIB_LINK += $(wildcard ../../src/pl/plpython/libpython*.a) $(wildcard ../../src/pl/plpython/libplpython*.a)
  endif
  
  ifeq ($(PORTNAME), cygwin)
--- 27,36 
  # dependency.  This does preclude pgxs builds.
  ifeq ($(PORTNAME), aix)
  rpathdir = $(pkglibdir):$(python_libdir)
! SHLIB_LINK += $(python_libspec) $(python_additional_libs) $(sort $(wildcard ../../src/pl/plpython/libplpython*.exp))
  endif
  ifeq ($(PORTNAME), win32)
! SHLIB_LINK += $(sort $(wildcard ../../src/pl/plpython/libpython*.a)) $(sort $(wildcard ../../src/pl/plpython/libplpython*.a))
  endif
  
  ifeq 

Re: [HACKERS] commitfest html - wrong closing tag

2016-01-05 Thread Magnus Hagander
On Sun, Jan 3, 2016 at 11:25 AM, Erik Rijkers  wrote:

> On 2016-01-03 10:06, Magnus Hagander wrote:
>
>> On Sun, Jan 3, 2016 at 9:26 AM, Erik Rijkers  wrote:
>>
>>> an erroneous ''. It should be ''.
>>>
>>
> Is there a particular thing you're trying to parse the data out for? As in
>> is there some data that we should already be providing in a structured
>> format instead of requiring parsing it out?
>>
>>
> I'm just trying to set up a way to compile and test all outstanding
> patches.
>
> It might perhaps be handy to have that patches table's columns somewhere
> (in tab-separated, perhaps?).
>
> Of course most of the work is downstream of that download, anyway. Compile
> & check (also rather simple) but especially to have some loading/testing
> (both general, and specific to the patch's goal).
>
>
Well, if you get everything else turned into something that works well and
is generally useful, then we can definitely look at putting up an API
endpoint that you can call to get the data in a structured format rather
than parsing it out of the HTML (which might randomly break if we make
changes..)

Actually having the ability to do that was one of the things we talked
about early on (an API and just having a job somewhere that could auto-post
the status), but nobody actually got around to doing it One of the most
basic ideas then would be to just add a state to a patch which could be set
to "fails to apply" or "fails to compile" for example, by a job, and an API
to basically pick patches off a queue (which would serve up new versions
etc automatically). If you're interested in turning it into something like
that, feel free to ping me off-list for a discussion of the (very small)
amount of things discussed back then.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] Making tab-complete.c easier to maintain

2016-01-05 Thread Tom Lane
I wrote:
> Michael Paquier  writes:
>> So, here are the commands that still remain with TailMatches to cover
>> this case, per gram.y:
>> - CREATE TABLE
>> - CREATE INDEX
>> - CREATE VIEW
>> - GRANT
>> - CREATE TRIGGER
>> - CREATE SEQUENCE
>> New patches are attached.

> I've reviewed and committed the first of these patches.

I've pushed the second patch now.  I made a few adjustments --- notably,
I didn't like the way you'd implemented '*' wildcards, because they
wouldn't have behaved very sanely in combination with '|'.  The case
doesn't come up in the current set of patterns, but we'll likely want it
sometime.

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] Refactoring of LWLock tranches

2016-01-05 Thread Bruce Momjian
On Tue, Jan  5, 2016 at 04:53:26PM +0100, Andres Freund wrote:
> On 2016-01-05 10:48:43 -0500, Bruce Momjian wrote:
> > On Tue, Jan  5, 2016 at 04:42:24PM +0100, Andres Freund wrote:
> > > On 2016-01-05 10:40:13 -0500, Bruce Momjian wrote:
> > > > On Tue, Jan  5, 2016 at 04:31:15PM +0100, Andres Freund wrote:
> > > > > On 2016-01-05 10:28:25 -0500, Bruce Momjian wrote:
> > > > > Yes? But it's ok sizewise on the common platforms?
> > > > 
> > > > What is the uncommon part?  I guess I missed that.
> > > 
> > > http://archives.postgresql.org/message-id/20151212181702.GH17938%40alap3.anarazel.de
> > 
> > Yes, I saw that, and the URL in the email, but what is the uncommon
> > case?
> 
> Are you asking which platforms s_lock is larger than a char? If so, grep
> s_lock.h for typedefs. If not, I'm not following what you're asking for?

Yes, that is the information I was looing for.  I see s_lock defined as
int-sized on Itanium, ARM, PowerPC, MIPS, m32r, PA-RISC, Windows,
Sunstudio.

-- 
  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] [PATCH] Refactoring of LWLock tranches

2016-01-05 Thread Bruce Momjian
On Tue, Jan  5, 2016 at 04:31:15PM +0100, Andres Freund wrote:
> On 2016-01-05 10:28:25 -0500, Bruce Momjian wrote:
> > On Sun, Dec 13, 2015 at 12:35:34PM +0100, Andres Freund wrote:
> > > > > One thing to call out is that an "oversized" s_lock can now make
> > > > > BufferDesc exceed 64 bytes, right now that's just the case when it's
> > > > > larger than 4 bytes.  I'm not sure if that's cause for real concern,
> > > > > given that it's not very concurrent or ancient platforms where that's
> > > > > the case.
> > > > > http://archives.postgresql.org/message-id/20150915020625.GI9666%40alap3.anarazel.de
> > > > > would alleviate that concern again, as it collapses flags, 
> > > > > usage_count,
> > > > > buf_hdr_lock and refcount into one 32 bit int...
> > > > 
> > > > I don't think that would be worth worrying about even if we didn't
> > > > have a plan in mind that would make it go away again, and even less so
> > > > given that we do have such a plan.
> > > 
> > > Ok cool. I'm not particularly concerned either, just didn't want to slip
> > > that in without having it called out.
> > 
> > Uh, didn't you and I work in 9.5 to make sure the BufferDesc was 64-byte
> > aligned to avoid double-CPU cache invalidation that was causing
> > performance problems on a server you were testing?
> 
> Yes? But it's ok sizewise on the common platforms?

What is the uncommon part?  I guess I missed that.

-- 
  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] [PATCH] Refactoring of LWLock tranches

2016-01-05 Thread Robert Haas
On Tue, Jan 5, 2016 at 8:54 AM, Jesper Pedersen
 wrote:
> LWLock *
> GetLWLockAddinTranche(const char *tranche_name)
>
> would be nicer to work with for extensions IMHO. Not likely worth the
> trouble though.

That change would be easy to make, but would tend to make performance
worse for anyone who uses more than 1 lock.

-- 
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] [PATCH] Refactoring of LWLock tranches

2016-01-05 Thread Jesper Pedersen

On 01/05/2016 08:04 AM, Amit Kapila wrote:

I am not aware of such cases, however the reason I have kept it was for
backward-compatability, but now I have removed it in the attached patch.

Apart from that, I have updated the docs to reflect the changes related
to new API's.



xfunc.sgml:

+after allocating LWLocks, verify that the number of 
allocated

+LWLocks is same as requested;

Did you mean to put this check in ?

lwlock.c:

+ * GetLWLockAddinTranche - returns the base address of LWLock from the
+ * specified tranche.
+ *
+ * Caller needs to retrieve the requested number of LWLocks starting from
+ * the base lock address returned by this API.  This can be used for
+ * tranches that are requested by using RequestAddinLWLockTranche() API.
+ */
+LWLockPadded *
+GetLWLockAddinTranche(const char *tranche_name)
+{

I understand why the signature is the way it is, but

LWLock *
GetLWLockAddinTranche(const char *tranche_name)

would be nicer to work with for extensions IMHO. Not likely worth the 
trouble though.


Thanks for working on this.

Best regards,
 Jesper



--
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] Refactoring of LWLock tranches

2016-01-05 Thread and...@anarazel.de
On 2016-01-05 10:48:43 -0500, Bruce Momjian wrote:
> On Tue, Jan  5, 2016 at 04:42:24PM +0100, Andres Freund wrote:
> > On 2016-01-05 10:40:13 -0500, Bruce Momjian wrote:
> > > On Tue, Jan  5, 2016 at 04:31:15PM +0100, Andres Freund wrote:
> > > > On 2016-01-05 10:28:25 -0500, Bruce Momjian wrote:
> > > > Yes? But it's ok sizewise on the common platforms?
> > > 
> > > What is the uncommon part?  I guess I missed that.
> > 
> > http://archives.postgresql.org/message-id/20151212181702.GH17938%40alap3.anarazel.de
> 
> Yes, I saw that, and the URL in the email, but what is the uncommon
> case?

Are you asking which platforms s_lock is larger than a char? If so, grep
s_lock.h for typedefs. If not, I'm not following what you're asking for?


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


[HACKERS] Optimizer questions

2016-01-05 Thread konstantin knizhnik
Hi hackers, 

I want to ask two questions about PostgreSQL optimizer.
I have the following query:

SELECT o.id as id,s.id as sid,o.owner,o.creator,o.parent_id as 
dir_id,s.mime_id,m.c_type,s.p_file,s.mtime,o.ctime,o.name,o.title,o.size,o.deleted,la.otime,la.etime,uo.login
 as owner_login,uc.login as creator_login,(CASE WHEN f.user_id IS NULL THEN 0 
ELSE 1 END) AS flagged,(select 'userid\\:'||string_agg(user_id,' userid\\:') 
from get_authorized_users(o.id)) as acl FROM objects s JOIN objects o ON 
s.original_file=o.id LEFT JOIN flags f ON o.id=f.obj_id AND o.owner=f.user_id 
LEFT JOIN objects_last_activity la ON o.id = la.obj_id AND o.owner = 
la.user_id, mime m, users uc , users uo WHERE (s.mime_id=904 or s.mime_id=908) 
AND m.mime_id = o.mime_id AND o.owner = uo.user_id AND o.creator = uc.user_id 
ORDER BY s.mtime LIMIT 9;

It is executed more than one hours. And the same query with "limit 8" is 
executed in just few seconds.
get_authorized_users is quite expensive function performing recursive query 
through several tables.

In first case (limit 9) sequential scan with sort is used  while in the second 
case index scan on "mtime" index is performed and so no sort is needed.
In the second case we can extract just 9 rows and calculate 
get_authorized_users  for them. In the first case we have to calculate this 
functions for ALL (~15M) rows.

I investigated plans of both queries and find out there are three reasons of 
the problem:

1. Wrong estimation of filter selectivity: 65549 vs. 154347 real.
2. Wrong estimation of joins selectivity: 284 vs. 65549 real
3. Wrong estimation of get_authorized_users  cost: 0.25 vs. 57.379 real 

Below is output of explain analyse:

Limit  (cost=1595355.77..1595355.79 rows=9 width=301) (actual 
time=3823128.752..3823128.755 rows=9 loops=1)
   ->  Sort  (cost=1595355.77..1595356.48 rows=284 width=301) (actual 
time=3823128.750..3823128.753 rows=9 loops=1)
 Sort Key: s.mtime
 Sort Method: top-N heapsort  Memory: 30kB
 ->  Nested Loop  (cost=1.96..1595349.85 rows=284 width=301) (actual 
time=75.453..3822829.640 rows=65549 loops=1)
   ->  Nested Loop Left Join  (cost=1.54..1589345.66 rows=284 
width=271) (actual time=1.457..59068.107 rows=65549 loops=1)
 ->  Nested Loop Left Join  (cost=0.98..1586923.67 rows=284 
width=255) (actual time=1.430..55661.926 rows=65549 loops=1)
   Join Filter: (((o.id)::text = (f.obj_id)::text) AND 
((o.owner)::text = (f.user_id)::text))
   Rows Removed by Join Filter: 132670698
   ->  Nested Loop  (cost=0.98..1576821.09 rows=284 
width=236) (actual time=0.163..27721.566 rows=65549 loops=1)
 Join Filter: ((o.mime_id)::integer = 
(m.mime_id)::integer)
 Rows Removed by Join Filter: 48768456
 ->  Seq Scan on mime m  (cost=0.00..13.45 
rows=745 width=30) (actual time=0.008..1.372 rows=745 loops=1)
 ->  Materialize  (cost=0.98..1573634.65 
rows=284 width=214) (actual time=0.004..22.918 rows=65549 loops=745)
   ->  Nested Loop  (cost=0.98..1573633.23 
rows=284 width=214) (actual time=0.142..7095.554 rows=65549 loops=1)
 ->  Nested Loop  
(cost=0.56..1570232.37 rows=406 width=184) (actual time=0.130..6468.376 
rows=65549 loops=1)
   ->  Seq Scan on objects s  
(cost=0.00..1183948.58 rows=45281 width=63) (actual time=0.098..5346.023 
rows=154347 loops=1)
 Filter: 
(((mime_id)::integer = 904) OR ((mime_id)::integer = 908))
 Rows Removed by 
Filter: 15191155
   ->  Index Scan using 
objects_pkey on objects o  (cost=0.56..8.52 rows=1 width=140) (actual 
time=0.006..0.007 rows=0 loops=154347)
 Index Cond: 
((id)::text = (s.original_file)::text)
 ->  Index Scan using users_pkey on 
users uc  (cost=0.42..8.37 rows=1 width=49) (actual time=0.008..0.009 rows=1 
loops=65549)
   Index Cond: ((user_id)::text 
= (o.creator)::text)
   ->  Materialize  (cost=0.00..48.36 rows=2024 
width=38) (actual time=0.001..0.133 rows=2024 loops=65549)
 ->  Seq Scan on flags f  (cost=0.00..38.24 
rows=2024 width=38) (actual time=0.009..0.462 rows=2024 loops=1)
 ->  Index Scan using objects_last_activity_unique_index on 
objects_last_activity la  (cost=0.56..8.52 rows=1 width=54) (actual 
time=0.044..0.046 rows=1 loops=65549)
   Index Cond: (((o.id)::text = (obj_id)::text) AND 
((o.owner)::text = 

Re: [HACKERS] [PATCH] Refactoring of LWLock tranches

2016-01-05 Thread Bruce Momjian
On Tue, Jan  5, 2016 at 04:42:24PM +0100, Andres Freund wrote:
> On 2016-01-05 10:40:13 -0500, Bruce Momjian wrote:
> > On Tue, Jan  5, 2016 at 04:31:15PM +0100, Andres Freund wrote:
> > > On 2016-01-05 10:28:25 -0500, Bruce Momjian wrote:
> > > Yes? But it's ok sizewise on the common platforms?
> > 
> > What is the uncommon part?  I guess I missed that.
> 
> http://archives.postgresql.org/message-id/20151212181702.GH17938%40alap3.anarazel.de

Yes, I saw that, and the URL in the email, but what is the uncommon
case?

-- 
  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] [PATCH] Refactoring of LWLock tranches

2016-01-05 Thread Bruce Momjian
On Sun, Dec 13, 2015 at 12:35:34PM +0100, Andres Freund wrote:
> > > One thing to call out is that an "oversized" s_lock can now make
> > > BufferDesc exceed 64 bytes, right now that's just the case when it's
> > > larger than 4 bytes.  I'm not sure if that's cause for real concern,
> > > given that it's not very concurrent or ancient platforms where that's
> > > the case.
> > > http://archives.postgresql.org/message-id/20150915020625.GI9666%40alap3.anarazel.de
> > > would alleviate that concern again, as it collapses flags, usage_count,
> > > buf_hdr_lock and refcount into one 32 bit int...
> > 
> > I don't think that would be worth worrying about even if we didn't
> > have a plan in mind that would make it go away again, and even less so
> > given that we do have such a plan.
> 
> Ok cool. I'm not particularly concerned either, just didn't want to slip
> that in without having it called out.

Uh, didn't you and I work in 9.5 to make sure the BufferDesc was 64-byte
aligned to avoid double-CPU cache invalidation that was causing
performance problems on a server you were testing?

-- 
  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] [PATCH] Refactoring of LWLock tranches

2016-01-05 Thread and...@anarazel.de
On 2016-01-05 10:40:13 -0500, Bruce Momjian wrote:
> On Tue, Jan  5, 2016 at 04:31:15PM +0100, Andres Freund wrote:
> > On 2016-01-05 10:28:25 -0500, Bruce Momjian wrote:
> > Yes? But it's ok sizewise on the common platforms?
> 
> What is the uncommon part?  I guess I missed that.

http://archives.postgresql.org/message-id/20151212181702.GH17938%40alap3.anarazel.de


-- 
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] Beginner hacker item: Fix to_reg*() input type

2016-01-05 Thread Tom Lane
Petr Korobeinikov  writes:
>> - Modify the functions in regproc.c. Take a look at how other text input
>> functions work to see what needs to happen here (you'll want to use
>> text_to_cstring() as part of that.)
>> 
>> - Modify the appropriate entries in src/include/catalog/pg_proc.h

> Let me try.
> `make check` says "All 160 tests passed.".

Pushed, thanks!

(I did make some small adjustments --- in this usage, PG_GETARG_TEXT_PP
is good enough and likely a bit more efficient than PG_GETARG_TEXT_P.)

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] bootstrap pg_shseclabel in relcache initialization

2016-01-05 Thread Alvaro Herrera
Alvaro Herrera wrote:

> I'm gonna push this to master only, which means you won't be able to
> rely on pg_shseclabel until 9.6.

Pushed, with one cosmetic change (update comment on formrdesc).  I also
bumped the catversion, but didn't verify whether this is critical.

-- 
Á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] Beginner hacker item: Fix to_reg*() input type

2016-01-05 Thread Petr Korobeinikov
2016-01-05 21:04 GMT+03:00 Tom Lane :

> (I did make some small adjustments --- in this usage, PG_GETARG_TEXT_PP
> is good enough and likely a bit more efficient than PG_GETARG_TEXT_P.)
>
Also I forgot to bump catalog version up. My mishit.


[HACKERS] pg_conversion seems rather strangely defined

2016-01-05 Thread Tom Lane
What is the point of pg_conversion.condefault (the flag that says whether
a conversion is "default")?  AFAICS, there is absolutely no way to invoke
a conversion that is not default, which means we might as well eliminate
the concept.

I do not see a lot of point in the namespacing of encoding conversions
either.  Does anyone really need or use search-path-dependent lookup of
conversions?  (If they do, it's probably broken anyway, since for example
we do not trouble to re-identify the client encoding conversion functions
when search_path changes.)

While actually removing pg_conversion.connamespace might not be worth
the trouble, it's mighty tempting to have just a single unique index on
(conforencoding, contoencoding), thereby enforcing that There Can Be Only
One conversion between any given pair of encodings, and then we can just
use that index to find the right entry without any fooling with search
path.

But in any case we should get rid of the concept of defaultness, because
it's pointless; all entries should be equally "default".

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] pg_receivexlog: spurious error message connecting to 9.3

2016-01-05 Thread Alvaro Herrera
Michael Paquier wrote:
> On Mon, Dec 7, 2015 at 11:37 PM, Fujii Masao  wrote:
> 
> > The latest patch has another problem; pg_receivexlog trying to connect to
> > the PostgreSQL >= 9.4 always reports the following message unexpectedly.
> >
> > could not identify system: got 1 rows and 4 fields, expected 1 rows
> > and 4 or more fields
> >
> > This problem happens because the patch incorrectly treats the case where
> > IDENTIFY_SYSTEM command returns NULL as database name, as an error case.
> >
> > Attached is the updated version of the patch, which fixes the problem.
> > Comments?
> 
> The patch looks good. The top comment of RunIdentifySystem is incorrect
> though. It should mention that a database name is returned and not a plugin
> name.

Right.  Pushed with that change to 9.5 and master.  Thanks!

-- 
Á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] dynloader.h missing in prebuilt package for Windows?

2016-01-05 Thread Chapman Flack
On 01/05/2016 09:18 AM, Chapman Flack wrote:
> On 01/05/2016 12:53 AM, Michael Paquier wrote:
> 
>> That's not a mandatory fix, but I think we had better do it. As long
>> as dynloader.h is copied in include/, there is no need to keep the
>> tweak of dfmgr.c to include the definitions those routines.
> 
> Looking at what you changed, all becomes clear.  :)

Out of curiosity, what happens (or what is supposed to happen)
to port/dynloader/.c ?

-Chap



-- 
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] dynloader.h missing in prebuilt package for Windows?

2016-01-05 Thread Chapman Flack
On 01/05/2016 12:53 AM, Michael Paquier wrote:

> That's not a mandatory fix, but I think we had better do it. As long
> as dynloader.h is copied in include/, there is no need to keep the
> tweak of dfmgr.c to include the definitions those routines.

Looking at what you changed, all becomes clear.  :)

-Chap



-- 
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] Building pg_xlogdump reproducibly

2016-01-05 Thread Tom Lane
Christoph Berg  writes:
> Re: Alvaro Herrera 2016-01-04 <20160104175623.GA170910@alvherre.pgsql>
>> I don't see any other $(wildcard) used to build executables; it's used
>> for tests and flags in many places, but that shouldn't matter.

> Nod. Attached is a patch that covers all relevant $(wildcard)
> occurrences in Makefiles for devel.

There was some upthread discussion of trying to centralize this logic
in a macro, but I think this patch is good as-is.  A macro wouldn't
really add any readability, nor would it do much to help us remember
to do the right thing in future places that might need this.

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] Refactoring of LWLock tranches

2016-01-05 Thread and...@anarazel.de
On 2016-01-05 10:28:25 -0500, Bruce Momjian wrote:
> On Sun, Dec 13, 2015 at 12:35:34PM +0100, Andres Freund wrote:
> > > > One thing to call out is that an "oversized" s_lock can now make
> > > > BufferDesc exceed 64 bytes, right now that's just the case when it's
> > > > larger than 4 bytes.  I'm not sure if that's cause for real concern,
> > > > given that it's not very concurrent or ancient platforms where that's
> > > > the case.
> > > > http://archives.postgresql.org/message-id/20150915020625.GI9666%40alap3.anarazel.de
> > > > would alleviate that concern again, as it collapses flags, usage_count,
> > > > buf_hdr_lock and refcount into one 32 bit int...
> > > 
> > > I don't think that would be worth worrying about even if we didn't
> > > have a plan in mind that would make it go away again, and even less so
> > > given that we do have such a plan.
> > 
> > Ok cool. I'm not particularly concerned either, just didn't want to slip
> > that in without having it called out.
> 
> Uh, didn't you and I work in 9.5 to make sure the BufferDesc was 64-byte
> aligned to avoid double-CPU cache invalidation that was causing
> performance problems on a server you were testing?

Yes? But it's ok sizewise on the common platforms?

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] Add schema-qualified relnames in constraint error messages.

2016-01-05 Thread Jim Nasby

On 1/5/16 9:16 PM, Tom Lane wrote:

Jim Nasby  writes:

FWIW, I suspect very few people know about the verbosity setting (I
didn't until a few months ago...) Maybe psql should hint about it the
first time an error is reported in a session.


Actually, what'd be really handy IMO is something to regurgitate the
most recent error in verbose mode, without making a permanent session
state change.  Something like

regression=# insert into bar values(1);
ERROR:  insert or update on table "bar" violates foreign key constraint 
"bar_f1_fkey"
DETAIL:  Key (f1)=(1) is not present in table "foo".
regression=# \saywhat
ERROR:  23503: insert or update on table "bar" violates foreign key constraint 
"bar_f1_fkey"
DETAIL:  Key (f1)=(1) is not present in table "foo".
SCHEMA NAME:  public
TABLE NAME:  bar
CONSTRAINT NAME:  bar_f1_fkey
LOCATION:  ri_ReportViolation, ri_triggers.c:3326
regression=#

Not sure how hard that would be to do within psql's current structure.


At first glance, it looks like it just means changing AcceptResult() to 
use PQresultErrorField in addition to PQresultErrorMessage, and stuffing 
the results somewhere. And of course adding \saywhat to the psql parser, 
but maybe someone more versed in psql code can verify that.


If it is that simple, looks like another good beginner hacker task. :)
--
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]WIP: Covering + unique indexes.

2016-01-05 Thread David Rowley
On 4 January 2016 at 21:49, David Rowley 
wrote:

> I've not tested the patch yet. I will send another email soon with the
> results of that.
>

Hi,

As promised I've done some testing on this, and I've found something which
is not quite right:

create table ab (a int,b int);
insert into ab select x,y from generate_series(1,20) x(x),
generate_series(10,1,-1) y(y);
create index on ab (a) including (b);
explain select * from ab order by a,b;
QUERY PLAN
--
 Sort  (cost=10.64..11.14 rows=200 width=8)
   Sort Key: a, b
   ->  Seq Scan on ab  (cost=0.00..3.00 rows=200 width=8)
(3 rows)

This is what I'd expect

truncate table ab;
insert into ab select x,y from generate_series(1,20) x(x),
generate_series(10,1,-1) y(y);
explain select * from ab order by a,b;
  QUERY PLAN

--
 Index Only Scan using ab_a_b_idx on ab  (cost=0.15..66.87 rows=2260
width=8)
(1 row)

This index, as we've defined it should not be able to satisfy the query's
order by, although it does give correct results, that's because the index
seems to be built wrongly in cases where the rows are added after the index
exists.

If we then do:

reindex table ab;
explain select * from ab order by a,b;
QUERY PLAN
--
 Sort  (cost=10.64..11.14 rows=200 width=8)
   Sort Key: a, b
   ->  Seq Scan on ab  (cost=0.00..3.00 rows=200 width=8)
(3 rows)

It looks normal again.

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


Re: [HACKERS] Function and view to retrieve WAL receiver status

2016-01-05 Thread Michael Paquier
On Wed, Jan 6, 2016 at 8:14 AM, Haribabu Kommi  wrote:
> Following are my observations on the latest patch.

Thanks for your review.

> + If no WAL receiver is present on the server queried,
> +   a single tuple filled with NULL values is returned instead.
> +  
>
> The above documentation change is not required if we change the system
> view.

Affirmative.

> +s.received_up_to_lsn,
>
> The column name can be changed as "received_lsn" similar to "received_tli".
> up_to may not be required.
>
> + XLogRecPtr received_up_lsn;
> + TimeLineID received_up_tli;
>
> same as like above comment.

Indeed, let's make the variable names more simple and consistent by
removing this _up_ portion everywhere.

> + /* lock? */
>
> I find out that walrcv data is updated only under mutex. it is better
> to take that mutex to provide a consistent snapshot data to user.

The lock is taken, the comment is just incorrect:
+/* lock? */
+SpinLockAcquire(>mutex);
[...]
+SpinLockRelease(>mutex);

I also found out that the description of those fields was not clear
enough actually: received_tli and received _lsn are related to what
has been received *and* flushed to disk, with an initial value being
their start equivalent. This deserves a clear description with all
those things addressed.

Attached is an updated patch.
-- 
Michael
From d924cb2f4f8594208ea6127b1135a213c48e0b89 Mon Sep 17 00:00:00 2001
From: Michael Paquier 
Date: Fri, 18 Dec 2015 22:44:21 +0900
Subject: [PATCH] Add system view and function to report WAL receiver activity

---
 doc/src/sgml/monitoring.sgml  |  91 
 src/backend/catalog/system_views.sql  |  16 
 src/backend/replication/walreceiver.c | 154 ++
 src/include/catalog/pg_proc.h |   2 +
 src/include/replication/walreceiver.h |   2 +
 src/test/regress/expected/rules.out   |  12 +++
 6 files changed, 277 insertions(+)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index c503636..85459d0 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -301,6 +301,14 @@ postgres   27093  0.0  0.0  30096  2752 ?Ss   11:34   0:00 postgres: ser
  
 
  
+  pg_stat_wal_receiverpg_stat_wal_receiver
+  Only one row, showing statistics about the WAL receiver from
+   that receiver's connected server.
+   See  for details.
+  
+ 
+
+ 
   pg_stat_sslpg_stat_ssl
   One row per connection (regular and replication), showing information about
SSL used on this connection.
@@ -833,6 +841,89 @@ postgres   27093  0.0  0.0  30096  2752 ?Ss   11:34   0:00 postgres: ser
listed; no information is available about downstream standby servers.
   
 
+  
+   pg_stat_wal_receiver View
+   
+
+
+  Column
+  Type
+  Description
+ 
+
+
+   
+
+ pid
+ integer
+ Process ID of the WAL receiver process
+
+
+ status
+ text
+ Activity status of the WAL receiver process
+
+
+ receive_start_lsn
+ pg_lsn
+ First transaction log position used when WAL receiver is
+  started
+
+
+ receive_start_tli
+ integer
+ First timeline number used when WAL receiver is started
+
+
+ received_lsn
+ pg_lsn
+ Last transaction log position already received and flushed to
+  disk, the initial value of this field being the first log position used
+  when WAL receiver is started
+
+
+ received_tli
+ integer
+ Timeline number of last transaction log position received and
+  flushed to disk, the initial value of this field being the timeline
+  number of the first log position used when WAL receiver is started
+ 
+
+
+ last_msg_send_time
+ timestamp with time zone
+ Send time of last message received from origin WAL sender
+
+
+ last_msg_receipt_time
+ timestamp with time zone
+ Receipt time of last message received from origin WAL sender
+
+
+ latest_end_lsn
+ pg_lsn
+ Last transaction log position reported to origin WAL sender
+
+
+ latest_end_time
+ timestamp with time zone
+ Time of last transaction log position reported to origin WAL sender
+
+
+ slot_name
+ text
+ Replication slot name used by this WAL receiver
+
+   
+   
+  
+
+  
+   The pg_stat_wal_receiver view will contain only
+   one row, showing statistics about the WAL receiver from that receiver's
+   connected server.
+  
+
   
pg_stat_ssl View

diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 2052afd..506a884 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -662,6 +662,22 @@ CREATE VIEW pg_stat_replication AS
 WHERE S.usesysid = U.oid AND
 S.pid = W.pid;
 
+CREATE VIEW 

Re: [HACKERS] No Issue Tracker - Say it Ain't So!

2016-01-05 Thread Alvaro Herrera
Joshua D. Drake wrote:
> On 01/05/2016 04:53 PM, Michael Paquier wrote:
> >On Wed, Jan 6, 2016 at 7:32 AM, Joshua D. Drake  
> >wrote:

> >>linuxhiker:  git interface! Bug tracker for this anywhere?
> >
> >Potential answer: Yes. As of now, pgsql-docs for doc issues, and
> >pgsql-bugs for actual bugs :)
> 
> Yes but that is a horrible answer.

I wouldn't even call that an answer actually.  To me, that just means
"no, we don't have one", which is kinda sad.

-- 
Á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] Additional role attributes && superuser review

2016-01-05 Thread Noah Misch
On Mon, Jan 04, 2016 at 12:55:16PM -0500, Stephen Frost wrote:
> * Noah Misch (n...@leadboat.com) wrote:
> > On Tue, Dec 29, 2015 at 08:35:50AM -0500, Stephen Frost wrote:

> I'm approaching this largely from a 3rd-party application perspective.
> There are two examples off-hand which I'm considering:
> 
> check_postgres
> pgbackrest
> 
> I'd like to be able to include, in both of those, a simple set of
> instructions for granting the necessary rights to the user who is
> running those processes.  A set of rights which an administrator can go
> look up and easily read and understand the result of those grants.  For
> example:
> 
> check_postgres:
> 
>   Most check_postgres sub-commands can be run without superuser
>   privileges by granting the pg_monitor role to the monitoring user:
> 
>   GRANT pg_monitor TO monitor;
> 
>   For information regarding the pg_monitor role, see:
>   http://www.postgresql.org/docs/current/static/roles/database-roles.html

Stop.  Even if PostgreSQL introduces pg_monitor, check_postgres will do itself
a favor by not using it.  The moment the two projects' sense of monitoring
privileges falls out of lockstep, benefits from using pg_monitor go negative.
check_postgres should instead furnish a script that creates a role holding
required privileges and/or SECURITY DEFINER helpers.  If check_postgres starts
using an object, say pgstattuple, it will wish to use it in all versions.
Since PostgreSQL will change pg_monitor privileges in major releases only,
check_postgres would wait 6-18 months to use a privilege in just one of five
supported versions.  If PostgreSQL hackers ever disagree with check_postgres
hackers about whether a privilege belongs in the pg_monitor role, then
check_postgres will wish it had never used pg_monitor.  For a sample
controversy, some monitoring tool may well call pg_read_file, but that's
arguably too much power to be giving _every_ monitoring tool.

By the way, the pg_monitor role you were ready to commit does not cover
today's check_postgres needs.  Among restricted objects, check_postgres uses
at least pg_ls_dir, pg_stats, pg_settings, and pg_stat_activity.  Having said
that, it remains premature to design predefined roles.  It would be a waste to
mobilize such a design effort with GRANT's limitation clouding the issue.

> and, to reiterate what I said above, I'd rather have one abstraction for
> these kinds of permissions instead of a mish-mash of instructions.  The
> difference I can imagine being between:
> 
> For backups and monitoring, you can use default roles:
> 
> GRANT pg_backup,pg_monitor to new_admin;
> 
> but for other regular privileges such as rotating logfiles, or sending
> signals to other processes, you have to explicitly GRANT permissions:
> 
> GRANT EXECUTE ON FUNCTION pg_rotate_logfile() TO new_admin;
> GRANT EXECUTE ON FUNCTION pg_signal_backend() TO new_admin;

I don't mind having those two ways to transfer privilege.  If I have to settle
for one or the other, I pick the latter.

> > > Further, I'm not convinced that adding support for dumping ACLs or, in
> > > general, encouraging users to define their own ACLs on catalog objects
> > > is a good idea.  We certainly have no mechanism in place today for those
> > > ACLs to be respected by SysCache and encouraging their use when we won't
> > > actually respect them is likely to be confusing.
> > 
> > What's this problem with syscache?  It sounds important.
> 
> CREATE TABLE and DROP TABLE aren't going to care one bit if you have
> access to pg_class or not.  The same goes for basically everything else.
> 
> If we really want to support ACLs on the catalog, we'd have to either
> caveat that none of the internal lookups will respect them or revamp
> SysCache and any other direct catalog access to do permission checks
> first, which I don't think we really want to do.

Oh, that.  Having internal lookups ignore the ACLs is more good than bad, and
users have little cause to expect something different.  You don't need INSERT
on pg_attribute to add a column, so why expect lack of SELECT on pg_attribute
to prevent dropping one?  I might document it like so:

While GRANT can modify the privileges of a system catalog table, that
affects only queries that address the catalog as an SQL table.  Internal,
system access to the same underlying data will proceed normally.  For
example, "REVOKE SELECT ON pg_proc FROM PUBLIC" does not preclude calling
functions or even preclude passing them to pg_get_functiondef.  It does
block queries that name pg_proc in a FROM clause.

> This entire discussion of privileges-on-catalog-objects should really
> also consider the ongoing discussion about providing policies for the
> catalog via RLS.  If we start pg_dump'ing the ACLs of catalog objects
> then we'd, presumably, also want to pg_dump out any policies defined
> against catalog objects.

I would have no qualms supporting ACL changes while not supporting added
policies, indexes, 

Re: [HACKERS] Add schema-qualified relnames in constraint error messages.

2016-01-05 Thread Tom Lane
Jim Nasby  writes:
> does psql do anything with those fields? ISTM the biggest use for this 
> info is someone sitting at psql or pgAdmin.

Sure, if you turn up the error verbosity.

regression=# create table foo (f1 int primary key);
CREATE TABLE
regression=# create table bar (f1 int references foo);
CREATE TABLE
regression=# insert into bar values(1);
ERROR:  insert or update on table "bar" violates foreign key constraint 
"bar_f1_fkey"
DETAIL:  Key (f1)=(1) is not present in table "foo".
regression=# \set VERBOSITY verbose
regression=# insert into bar values(1);
ERROR:  23503: insert or update on table "bar" violates foreign key constraint 
"bar_f1_fkey"
DETAIL:  Key (f1)=(1) is not present in table "foo".
SCHEMA NAME:  public
TABLE NAME:  bar
CONSTRAINT NAME:  bar_f1_fkey
LOCATION:  ri_ReportViolation, ri_triggers.c:3326

I can't speak to pgAdmin, but if it doesn't make this info available
the answer is to fix pgAdmin ...

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] Add schema-qualified relnames in constraint error messages.

2016-01-05 Thread Jim Nasby

On 1/5/16 8:41 PM, Tom Lane wrote:

Jim Nasby  writes:

>does psql do anything with those fields? ISTM the biggest use for this
>info is someone sitting at psql or pgAdmin.

Sure, if you turn up the error verbosity.


FWIW, I suspect very few people know about the verbosity setting (I 
didn't until a few months ago...) Maybe psql should hint about it the 
first time an error is reported in a session.

--
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] Add schema-qualified relnames in constraint error messages.

2016-01-05 Thread Tom Lane
Jim Nasby  writes:
> FWIW, I suspect very few people know about the verbosity setting (I 
> didn't until a few months ago...) Maybe psql should hint about it the 
> first time an error is reported in a session.

Actually, what'd be really handy IMO is something to regurgitate the
most recent error in verbose mode, without making a permanent session
state change.  Something like

regression=# insert into bar values(1);
ERROR:  insert or update on table "bar" violates foreign key constraint 
"bar_f1_fkey"
DETAIL:  Key (f1)=(1) is not present in table "foo".
regression=# \saywhat
ERROR:  23503: insert or update on table "bar" violates foreign key constraint 
"bar_f1_fkey"
DETAIL:  Key (f1)=(1) is not present in table "foo".
SCHEMA NAME:  public
TABLE NAME:  bar
CONSTRAINT NAME:  bar_f1_fkey
LOCATION:  ri_ReportViolation, ri_triggers.c:3326
regression=# 

Not sure how hard that would be to do within psql's current structure.

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] No Issue Tracker - Say it Ain't So!

2016-01-05 Thread Jim Nasby

On 1/5/16 8:19 PM, Stephen Frost wrote:

* Jim Nasby (jim.na...@bluetreble.com) wrote:

Which doesn't help anyone, because neither of those provide a list
of "hey, here's stuff you could do to contribute". The closest we
come to that is the TODO, which isn't well known and has almost no
items for newbies (and the newbie items that are there don't offer
much advice).


Agreed.  I've not given up on the bugs.p.o project, but I've gotten
wrapped up in migrating the buildfarm server on to our infrastructure.
Hopefully that will be completed as early as this week and I'll be able
to refocus my infra time to finishing bugs.p.o.


FWIW, I think that's a great example of why we'd be much better off with 
a focus on expanding the community beyond code contributors. There's a 
couple hundred people on the whole planet with your skill at expanding 
Postgres itself and probably millions of people that could work on 
infrastructure. Not a great allocation of resources... ;)

--
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] Add schema-qualified relnames in constraint error messages.

2016-01-05 Thread Jim Nasby

On 1/5/16 7:21 PM, Tom Lane wrote:

What seems more likely to be useful and to not break anything is to push
forward on adding PG_DIAG_SCHEMA_NAME and similar auxiliary fields to more
error messages (see errtable() and siblings).  That would allow
applications to extract this information automatically and reliably.
Only after that work is complete and there's been a reasonable amount of
time for clients to start relying on it can we really start thinking about
whacking the message texts around.


+1, but...

does psql do anything with those fields? ISTM the biggest use for this 
info is someone sitting at psql or pgAdmin.


Maybe schema info could be presented in HINT or DETAIL messages as well?
--
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] Multi-tenancy with RLS

2016-01-05 Thread Amit Langote
On 2016/01/06 10:17, Haribabu Kommi wrote:
> On Mon, Jan 4, 2016 at 10:43 PM, Haribabu Kommi
>>
>> Thanks for the test. Yes, the issue happens at backend startup itself.
>> I will give a try by separating the initialization of security
>> policies after init phase 3.
> 
> Here I attached updated patches with the fix of infinite recursion in
> RelationBuildRowSecurity function by checking with a variable that
> whether the build row security is already in progress for a system
> relation or not. If it is already in progress for a relation, then it doesn't
> build the row security description for this relation.

Thanks for updating the patch.

Patch 4_database_catalog_tenancy_v5 fails to apply:

patching file src/backend/commands/policy.c
Hunk #3 succeeded at 112 with fuzz 2 (offset 3 lines).
Hunk #4 succeeded at 269 with fuzz 1 (offset 13 lines).
Hunk #5 succeeded at 298 (offset 13 lines).
Hunk #6 succeeded at 365 (offset 12 lines).
Hunk #7 FAILED at 466.
Hunk #8 succeeded at 577 (offset 22 lines).
Hunk #9 succeeded at 607 with fuzz 2 (offset 22 lines).
Hunk #10 succeeded at 633 with fuzz 2 (offset 22 lines).
Hunk #11 FAILED at 801.
Hunk #12 FAILED at 813.
3 out of 12 hunks FAILED -- saving rejects to file
src/backend/commands/policy.c.rej

Thanks,
Amit





-- 
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] No Issue Tracker - Say it Ain't So!

2016-01-05 Thread Joshua D. Drake

On 01/05/2016 06:30 PM, Jim Nasby wrote:

On 1/5/16 8:19 PM, Stephen Frost wrote:

* Jim Nasby (jim.na...@bluetreble.com) wrote:

Which doesn't help anyone, because neither of those provide a list
of "hey, here's stuff you could do to contribute". The closest we
come to that is the TODO, which isn't well known and has almost no
items for newbies (and the newbie items that are there don't offer
much advice).


Agreed.  I've not given up on the bugs.p.o project, but I've gotten
wrapped up in migrating the buildfarm server on to our infrastructure.
Hopefully that will be completed as early as this week and I'll be able
to refocus my infra time to finishing bugs.p.o.


FWIW, I think that's a great example of why we'd be much better off with
a focus on expanding the community beyond code contributors. There's a
couple hundred people on the whole planet with your skill at expanding
Postgres itself and probably millions of people that could work on
infrastructure. Not a great allocation of resources... ;)


I have excellent people right now I could (and would) assign to the task 
if the task had guidelines.


JD

--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


--
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] Refactoring of LWLock tranches

2016-01-05 Thread Amit Kapila
On Tue, Jan 5, 2016 at 7:24 PM, Jesper Pedersen 
wrote:
>
> On 01/05/2016 08:04 AM, Amit Kapila wrote:
>>
>> I am not aware of such cases, however the reason I have kept it was for
>> backward-compatability, but now I have removed it in the attached patch.
>>
>> Apart from that, I have updated the docs to reflect the changes related
>> to new API's.
>>
>
> xfunc.sgml:
>
> +after allocating LWLocks, verify that the number of
allocated
> +LWLocks is same as requested;
>
> Did you mean to put this check in ?
>
Yes, it is good to check, right now I have added Assert in code as
I think it is a dev problem to over-allocate the lwlocks.


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


Re: [HACKERS] No Issue Tracker - Say it Ain't So!

2016-01-05 Thread Joshua D. Drake

Hello,

So I was on #postgresql today. I convinced a newer user that they could 
easily contribute to PostgreSQL even if it was just doc patches. I 
described the basic workflow and the individual was excited.


His first question?

linuxhiker:  git interface! Bug tracker for this anywhere?

Joshua D. Drake

--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


--
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] dynloader.h missing in prebuilt package for Windows?

2016-01-05 Thread Michael Paquier
On Tue, Jan 5, 2016 at 2:50 PM, Michael Paquier
 wrote:
> On Tue, Jan 5, 2016 at 2:27 PM, Tom Lane  wrote:
>> Michael Paquier  writes:
>>> The patch would put the buildfarm in red as it is incomplete anyway,
>>> with MSVC what is used instead of dynloader.h is
>>> port/dynloader/win32.h. Instead of this patch I would be incline to
>>> remove the #define stuff with dynloader.h that use WIN32_ONLY_COMPILER
>>> (see for example dfmgr.c) and just copy the header in include/. This
>>> way we use the same header for all platforms.
>>
>> Patch please?
>
> Sure, here you go. Bruce's patch simply forgot to copy the header file
> via Solution.pm, so installation just failed. The change of dfmgr.c is
> actually not mandatory but I think that as long as dynloader.h is
> copied in include/ we had better change that as well, and it makes the
> code cleaner.

By the way, it is definitely wiser to wait for after the release of
9.5.0 to push that or something similar...
-- 
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] dynloader.h missing in prebuilt package for Windows?

2016-01-05 Thread Michael Paquier
On Tue, Jan 5, 2016 at 11:24 PM, Chapman Flack  wrote:
> On 01/05/2016 09:18 AM, Chapman Flack wrote:
>> On 01/05/2016 12:53 AM, Michael Paquier wrote:
>>
>>> That's not a mandatory fix, but I think we had better do it. As long
>>> as dynloader.h is copied in include/, there is no need to keep the
>>> tweak of dfmgr.c to include the definitions those routines.
>>
>> Looking at what you changed, all becomes clear.  :)
>
> Out of curiosity, what happens (or what is supposed to happen)
> to port/dynloader/.c ?

For other platforms that go through ./configure, this file is
similarly copied as src/include/dynloader.h. MSVC is a special case..
-- 
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] Beginner hacker item: Fix to_reg*() input type

2016-01-05 Thread Tom Lane
Petr Korobeinikov  writes:
> 2016-01-05 21:04 GMT+03:00 Tom Lane :
>> (I did make some small adjustments --- in this usage, PG_GETARG_TEXT_PP
>> is good enough and likely a bit more efficient than PG_GETARG_TEXT_P.)

> Also I forgot to bump catalog version up. My mishit.

No, submitted patches generally should not touch catversion (though it's
not a bad idea to mention the need for a bump in the submission message).
If you put in a catversion change, it's generally just going to cause
merge failures, since patches seldom get applied instantly.

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] Function and view to retrieve WAL receiver status

2016-01-05 Thread Haribabu Kommi
On Sat, Dec 19, 2015 at 12:54 AM, Michael Paquier
 wrote:
> On Fri, Dec 18, 2015 at 8:39 AM, Robert Haas  wrote:
>> On Mon, Dec 14, 2015 at 7:23 PM, Michael Paquier
>>  wrote:
>>> On Tue, Dec 15, 2015 at 5:27 AM, Gurjeet Singh wrote:
 The function, maybe. But emitting an all-nulls row from a view seems
 counter-intuitive, at least when looking at it in context of relational
 database.
>>>
>>> OK, noted. Any other opinions?
>>
>> I wouldn't bother with the view.  If we're going to do it, I'd say
>> just provide the function and let people SELECT * from it if they want
>> to.
>
> OK, I took some time to write a patch for that as attached, added in
> the next CF here:
> https://commitfest.postgresql.org/8/447/
> I am fine switching to an SRF depending on other opinions of people
> here, it just seems like an overkill knowing the uniqueness of the WAL
> sender in a server.
>
> I have finished with a function and a system view, this came up more
> in line with the existing things like pg_stat_archiver, and this makes
> as well the documentation clearer, at least that was my feeling when
> hacking that.

I also feel showing NULL values may not be good, when there is
no walreceiver. Instead of SRF function to avoid showing NULL vallues
how about adding "WHERE s.pid IS NOT NULL" to the system view.
pid value cannot be NULL, until unless there is no walreceiver.


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] Function and view to retrieve WAL receiver status

2016-01-05 Thread Michael Paquier
On Tue, Jan 5, 2016 at 7:49 PM, Haribabu Kommi  wrote:
> On Sat, Dec 19, 2015 at 12:54 AM, Michael Paquier
>  wrote:
>> On Fri, Dec 18, 2015 at 8:39 AM, Robert Haas  wrote:
>>> On Mon, Dec 14, 2015 at 7:23 PM, Michael Paquier
>>>  wrote:
 On Tue, Dec 15, 2015 at 5:27 AM, Gurjeet Singh wrote:
> The function, maybe. But emitting an all-nulls row from a view seems
> counter-intuitive, at least when looking at it in context of relational
> database.

 OK, noted. Any other opinions?
>>>
>>> I wouldn't bother with the view.  If we're going to do it, I'd say
>>> just provide the function and let people SELECT * from it if they want
>>> to.
>>
>> OK, I took some time to write a patch for that as attached, added in
>> the next CF here:
>> https://commitfest.postgresql.org/8/447/
>> I am fine switching to an SRF depending on other opinions of people
>> here, it just seems like an overkill knowing the uniqueness of the WAL
>> sender in a server.
>>
>> I have finished with a function and a system view, this came up more
>> in line with the existing things like pg_stat_archiver, and this makes
>> as well the documentation clearer, at least that was my feeling when
>> hacking that.
>
> I also feel showing NULL values may not be good, when there is
> no walreceiver. Instead of SRF function to avoid showing NULL vallues
> how about adding "WHERE s.pid IS NOT NULL" to the system view.
> pid value cannot be NULL, until unless there is no walreceiver.

Yeah, I would not mind switching it to that. A couple of other stat
catalog views do it as well.
-- 
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] Optimizer questions

2016-01-05 Thread Tom Lane
konstantin knizhnik  writes:
> 1. The cost compared in grouping_planner doesn't take in account price of 
> get_authorized_users - it is not changed when I am altering function cost. Is 
> it correct behavior?

The general problem of accounting for tlist eval cost is not handled very
well now, but especially not with respect to the idea that different paths
might have different tlist costs.  I'm working on an upper-planner rewrite
which should make this better, or at least make it practical to make it
better.

In the meantime you can probably force things to work the way you want
by doing the sort/limit in a sub-select and evaluating the expensive
function only in the outer select.

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] pg_conversion seems rather strangely defined

2016-01-05 Thread Tom Lane
I wrote:
> What is the point of pg_conversion.condefault (the flag that says whether
> a conversion is "default")?  AFAICS, there is absolutely no way to invoke
> a conversion that is not default, which means we might as well eliminate
> the concept.

> I do not see a lot of point in the namespacing of encoding conversions
> either.  Does anyone really need or use search-path-dependent lookup of
> conversions?  (If they do, it's probably broken anyway, since for example
> we do not trouble to re-identify the client encoding conversion functions
> when search_path changes.)

> While actually removing pg_conversion.connamespace might not be worth
> the trouble, it's mighty tempting to have just a single unique index on
> (conforencoding, contoencoding), thereby enforcing that There Can Be Only
> One conversion between any given pair of encodings, and then we can just
> use that index to find the right entry without any fooling with search
> path.

> But in any case we should get rid of the concept of defaultness, because
> it's pointless; all entries should be equally "default".

After further poking around, I realized that there *used* to be a way to
get at non-default encoding conversions, but it was removed here:

commit 02138357ffc8c41a3d646035368712a5394f1f5f
Author: Andrew Dunstan 
Date:   Mon Sep 24 01:29:30 2007 +

Remove "convert 'blah' using conversion_name" facility, because if it
produces text it is an encoding hole and if not it's incompatible
with the spec, whatever the spec means (which we're not sure about anyway).

Now, the big problem there was that the function could produce values of
type "text" that violate the current database encoding.  We could have
retained the facility by making the function take and return bytea, as
convert() does today, but apparently nobody spoke up for the need then
or since.

Still, somebody might want it someday, and it would be pretty messy
to remove the concept of a default encoding only to have to put it
back later.

I still think however that search-path-based lookup of default encoding
conversions is a Bad Idea, and that we only ought to allow one default
conversion to exist for any pair of encodings.

I realized that we could implement that without too much trouble by
redefining pg_conversion.condefault as being true for default conversions
and NULL (not false) for non-default ones.  With this definition, a
unique index on pg_conversion (conforencoding, contoencoding, condefault)
would have the behavior we want --- sort of a poor man's partial unique
index, except that it would work correctly on a system catalog whereas
a true partial index wouldn't.

Thoughts?

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] Add scale(numeric)

2016-01-05 Thread Alvaro Herrera
Pavel Stehule wrote:
> Hi
> 
> 2015-11-19 3:27 GMT+01:00 Marko Tiikkaja :
> 
> > Hi,
> >
> > As suggested in 5643125e.1030...@joh.to, here's a patch for extracting
> > the scale out of a numeric.
> >
> > This is 2016-01 CF material, but if someone wants to bas^H^H^Hsay nice
> > things in the meanwhile, feel free.
> >
> 
> This patch is pretty trivial without any possible issues, but I miss a use
> case of this feature.
> 
> Why it should be in Postgres?

This was discussed in the original thread, and it seemed compelling
enough to me; there were no objections raised either.  I couldn't find
any fault in the submitted patch, so pushed.

Thanks,

-- 
Á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] use_remote_estimate usage for join pushdown in postgres_fdw

2016-01-05 Thread Ashutosh Bapat
On Wed, Dec 16, 2015 at 11:41 PM, Tom Lane  wrote:

> Robert Haas  writes:
> > I like option #2.  I don't really have a strong reason for that, but
> > it feels intuitive to me that we err on the side of using remote
> > estimates when in doubt.
>
> If we believe that, why isn't the default value of use_remote_estimate
> true?
> (Maybe it should be.)
>
> Another option that should be considered is that joins should pay
> attention only to the server-level setting and not to the individual
> tables' settings.  This would surely be cheaper to implement, and
> it avoids any questions about whether to OR or AND the individual
> settings.
>
>
To implement server-level setting, we will need to pull it out of
ForeignServer structure like
 442 foreach(lc, fpinfo->server->options)
 443 {
 444 DefElem*def = (DefElem *) lfirst(lc);
 445
 446 if (strcmp(def->defname, "use_remote_estimate") == 0)
 447 fpinfo->use_remote_estimate = defGetBoolean(def);
 ...
 455 }

whereas use_remote_estimate setting for joining relations is readily
available in PgFdwRelationInfo

 58 /* Options extracted from catalogs. */
 59 booluse_remote_estimate;
 60 Costfdw_startup_cost;
 61 Costfdw_tuple_cost;
 62 List   *shippable_extensions;   /* OIDs of whitelisted
extensions */
 ...
 76 } ;

This use_remote_estimate is true if server level option is true or table
level option is true.

ANDing or ORing use_remote_estimate from the joining relations'
PgFdwRelationInfo looks cheaper than pulling it out of ForeignServer
structure.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


Re: [HACKERS] [PATCH] Refactoring of LWLock tranches

2016-01-05 Thread Amit Kapila
On Mon, Jan 4, 2016 at 8:28 PM, Robert Haas  wrote:
>
> On Mon, Jan 4, 2016 at 1:20 AM, Amit Kapila 
wrote:
> > On Mon, Jan 4, 2016 at 4:49 AM, Robert Haas 
wrote:
> >> On Thu, Dec 31, 2015 at 3:36 AM, Amit Kapila 
> >> wrote:
> >> > LWLock *LWLockAssignFromTranche(const char *tranche_name) will
> >> > assign a lock for the specified tranche.  This also ensures that no
> >> > more than requested number of LWLocks can be assigned from a
> >> > specified tranche.
> >>
> >> However, this seems like an awkward API, because if there are many
> >> LWLocks you're going to need to look up the tranche name many times,
> >> and then you're going to need to make an array of pointers to them
> >> somehow, introducing a thoroughly unnecessary layer of indirection.
> >> Instead, I suggest just having a function LWLockPadded
> >> *GetLWLockAddinTranche(const char *tranche_name) that returns a
> >> pointer to the base of the array.
> >
> > If we do that way, then user of API needs to maintain the interlock
> > guarantee that the requested number of locks is same as allocations
> > done from that tranche and also if it is not allocating all the
> > LWLocks in one function, it needs to save the base address of the
> > array and then allocate from it by maintaining some counter.
> > I agree that looking up for tranche names multiple times is not good,
> > if there are many number of lwlocks, but that is done at startup
> > time and not at operation-level.  Also if we look currently at
> > the extensions in contrib, then just one of them allocactes one
> > LWLock in this fashion, now there could be extnsions apart from
> > extensions in contrib, but not sure if they require many number of
> > LWLocks, so I feel it is better to give an API which is more
> > convinient for user to use.
>
> Well, I agree with you that we should provide the most convenient API
> possible, but I don't agree that yours is more convenient than the one
> I proposed.  I think it is less convenient.  In most cases, if the
> user asks for a large number of LWLocks, they aren't going to be each
> for a different purpose - they will all be for the same purpose, like
> one per buffer or one per hash partition.  The case where the user
> wants to allocate 8 lwlocks from an extension, each for a different
> purpose, and spread those allocations across a bunch of different
> functions probably does not exist.

Probably, but the point is to make user of API do what he or she wants
to accomplish without much knowledge of underlying stuff.  However,
I think it is not too much details for user to know, so I have changed the
API as per your suggestion.

>
>   *Maybe* there is somebody
> allocating lwlocks from an extension for unrelated purposes, but I'd
> be willing to bet that, if so, all of those allocations happen one
> right after the other in a single function, because anything else
> would be completely nuts.
>
> >> > Also I have retained NUM_USER_DEFINED_LWLOCKS in
> >> > MainLWLockArray so that if any extensions want to assign a LWLock
> >> > after startup, it can be used from this pool.  The tranche for such
> >> > locks
> >> > will be reported as main.
> >>
>
> I'd be interested in knowing whether there are cases where useful
> extensions can be loaded apart from shared_preload_libraries because
> of NUM_USER_DEFINED_LWLOCKS and our tendency to allocate a little bit
> of extra shared memory, but my suspicion is that it rarely works out
> and is too flaky to be useful to anybody.
>

I am not aware of such cases, however the reason I have kept it was for
backward-compatability, but now I have removed it in the attached patch.

Apart from that, I have updated the docs to reflect the changes related
to new API's.

Fe things to Note -
Some change is needed in LWLockCounter[1] if this goes after 2
other patches (separate tranche for PGProcs and separate tranche
for ReplicationSlots).  Also, LWLockAssign() can be removed after
those patches


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


separate_tranche_extensions_v2.patch
Description: Binary data

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


Re: [HACKERS] No Issue Tracker - Say it Ain't So!

2016-01-05 Thread Michael Paquier
On Wed, Jan 6, 2016 at 7:32 AM, Joshua D. Drake  wrote:
> Hello,
>
> So I was on #postgresql today. I convinced a newer user that they could
> easily contribute to PostgreSQL even if it was just doc patches. I described
> the basic workflow and the individual was excited.
>
> His first question?
>
> linuxhiker:  git interface! Bug tracker for this anywhere?

Potential answer: Yes. As of now, pgsql-docs for doc issues, and
pgsql-bugs for actual bugs :)
-- 
Michael


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


[HACKERS] Add schema-qualified relnames in constraint error messages.

2016-01-05 Thread Petr Korobeinikov
Hackers,

At the moment schemas used as single-level namespaces.
It's quite convenient in large databases.

But error messages doesn’t contain schema names.

I have done a little patch with schema-qualified relation names in error
messages that produced by foreign key constraints and triggers.

Patch and usage example are attached.
Based on real bug hunting.

Pre-reviewed by Alexander Korotkov.
diff --git a/src/backend/utils/adt/ri_triggers.c 
b/src/backend/utils/adt/ri_triggers.c
index b476500..997e959 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -343,7 +343,7 @@ RI_FKey_check(TriggerData *trigdata)
ereport(ERROR,

(errcode(ERRCODE_FOREIGN_KEY_VIOLATION),
 errmsg("insert or 
update on table \"%s\" violates foreign key constraint \"%s\"",
-   
RelationGetRelationName(fk_rel),
+   
RelationGetNamespaceQualifiedRelationName(fk_rel),

NameStr(riinfo->conname)),
 errdetail("MATCH FULL 
does not allow mixing of null and nonnull key values."),
 
errtableconstraint(fk_rel,
@@ -2490,7 +2490,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, 
Relation pk_rel)
ereport(ERROR,
(errcode(ERRCODE_FOREIGN_KEY_VIOLATION),
 errmsg("insert or update on table 
\"%s\" violates foreign key constraint \"%s\"",
-   
RelationGetRelationName(fk_rel),
+   
RelationGetNamespaceQualifiedRelationName(fk_rel),

NameStr(fake_riinfo.conname)),
 errdetail("MATCH FULL does not allow 
mixing of null and nonnull key values."),
 errtableconstraint(fk_rel,
@@ -2767,7 +2767,7 @@ ri_FetchConstraintInfo(Trigger *trigger, Relation 
trig_rel, bool rel_is_pk)
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
  errmsg("no pg_constraint entry for trigger \"%s\" on table 
\"%s\"",
-trigger->tgname, 
RelationGetRelationName(trig_rel)),
+trigger->tgname, 
RelationGetNamespaceQualifiedRelationName(trig_rel)),
 errhint("Remove this referential integrity 
trigger and its mates, then do ALTER TABLE ADD CONSTRAINT.")));
 
/* Find or create a hashtable entry for the constraint */
@@ -2779,14 +2779,14 @@ ri_FetchConstraintInfo(Trigger *trigger, Relation 
trig_rel, bool rel_is_pk)
if (riinfo->fk_relid != trigger->tgconstrrelid ||
riinfo->pk_relid != RelationGetRelid(trig_rel))
elog(ERROR, "wrong pg_constraint entry for trigger 
\"%s\" on table \"%s\"",
-trigger->tgname, 
RelationGetRelationName(trig_rel));
+trigger->tgname, 
RelationGetNamespaceQualifiedRelationName(trig_rel));
}
else
{
if (riinfo->fk_relid != RelationGetRelid(trig_rel) ||
riinfo->pk_relid != trigger->tgconstrrelid)
elog(ERROR, "wrong pg_constraint entry for trigger 
\"%s\" on table \"%s\"",
-trigger->tgname, 
RelationGetRelationName(trig_rel));
+trigger->tgname, 
RelationGetNamespaceQualifiedRelationName(trig_rel));
}
 
return riinfo;
@@ -3225,9 +3225,9 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo,
ereport(ERROR,
(errcode(ERRCODE_INTERNAL_ERROR),
 errmsg("referential integrity query on \"%s\" 
from constraint \"%s\" on \"%s\" gave unexpected result",
-   RelationGetRelationName(pk_rel),
+   
RelationGetNamespaceQualifiedRelationName(pk_rel),
NameStr(riinfo->conname),
-   
RelationGetRelationName(fk_rel)),
+   
RelationGetNamespaceQualifiedRelationName(fk_rel)),
 errhint("This is most likely due to a rule 
having rewritten the query.")));
 
/*
@@ -3315,28 +3315,28 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo,
 

Re: [HACKERS] pg_conversion seems rather strangely defined

2016-01-05 Thread Tom Lane
I wrote:
> I still think however that search-path-based lookup of default encoding
> conversions is a Bad Idea, and that we only ought to allow one default
> conversion to exist for any pair of encodings.

> I realized that we could implement that without too much trouble by
> redefining pg_conversion.condefault as being true for default conversions
> and NULL (not false) for non-default ones.  With this definition, a
> unique index on pg_conversion (conforencoding, contoencoding, condefault)
> would have the behavior we want --- sort of a poor man's partial unique
> index, except that it would work correctly on a system catalog whereas
> a true partial index wouldn't.

Turns out that indeed that works just fine.  See attached draft patch.

regards, tom lane

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 97ef618..dcb8b8e 100644
*** a/doc/src/sgml/catalogs.sgml
--- b/doc/src/sgml/catalogs.sgml
***
*** 2538,2544 
condefault
bool

!   True if this is the default conversion
   
  
  
--- 2538,2545 
condefault
bool

!   True if this is the default conversion for these two encodings,
!else NULL
   
  
  
diff --git a/doc/src/sgml/charset.sgml b/doc/src/sgml/charset.sgml
index f8c7ac3..eb39a9f 100644
*** a/doc/src/sgml/charset.sgml
--- b/doc/src/sgml/charset.sgml
*** $ psql -l
*** 1102,1108 
   pg_conversion system catalog.  PostgreSQL
   comes with some predefined conversions, as shown in . You can create a new
!  conversion using the SQL command CREATE CONVERSION.
  
  
   
--- 1102,1108 
   pg_conversion system catalog.  PostgreSQL
   comes with some predefined conversions, as shown in . You can create a new
!  conversion using the SQL command .
  
  
   
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index a0b42c2..f57d891 100644
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 1488,1494 
  original encoding is specified by
  src_encoding. The
  string must be valid in this encoding.
! Conversions can be defined by CREATE CONVERSION.
  Also there are some predefined conversions. See  for available conversions.
 
--- 1488,1494 
  original encoding is specified by
  src_encoding. The
  string must be valid in this encoding.
! Conversions can be defined with .
  Also there are some predefined conversions. See  for available conversions.
 
***
*** 1525,1534 
 
 bytea
 
! Convert string to dest_encoding.
 
 convert_to('some text', 'UTF8')
!some text represented in the UTF8 encoding

  

--- 1525,1535 
 
 bytea
 
! Convert string from the database encoding
! to dest_encoding.
 
 convert_to('some text', 'UTF8')
!some text represented in UTF8 encoding

  

diff --git a/doc/src/sgml/ref/create_conversion.sgml b/doc/src/sgml/ref/create_conversion.sgml
index d2e2c01..1933107 100644
*** a/doc/src/sgml/ref/create_conversion.sgml
--- b/doc/src/sgml/ref/create_conversion.sgml
*** CREATE [ DEFAULT ] CONVERSION 
 CREATE CONVERSION defines a new conversion between
!character set encodings.  Also, conversions that
 are marked DEFAULT can be used for automatic encoding
 conversion between
 client and server. For this purpose, two conversions, from encoding A to
--- 28,34 
  

 CREATE CONVERSION defines a new conversion between
!character set encodings.  Conversions that
 are marked DEFAULT can be used for automatic encoding
 conversion between
 client and server. For this purpose, two conversions, from encoding A to
*** CREATE [ DEFAULT ] CONVERSION 
 The DEFAULT clause indicates that this conversion
 is the default for this particular source to destination
!encoding. There should be only one default encoding in a schema
 for the encoding pair.

   
--- 53,59 

 The DEFAULT clause indicates that this conversion
 is the default for this particular source to destination
!encoding. There can be only one default conversion in a database
 for the encoding pair.

   
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 8b105fe..98977f6 100644
*** a/src/backend/catalog/namespace.c
--- b/src/backend/catalog/namespace.c
***
*** 28,34 
  #include "catalog/pg_authid.h"
  #include "catalog/pg_collation.h"
  #include "catalog/pg_conversion.h"
- #include "catalog/pg_conversion_fn.h"
  #include "catalog/pg_namespace.h"
  #include "catalog/pg_opclass.h"
  #include "catalog/pg_operator.h"
--- 28,33 

Re: [HACKERS] Add schema-qualified relnames in constraint error messages.

2016-01-05 Thread Tom Lane
Petr Korobeinikov  writes:
> At the moment schemas used as single-level namespaces.
> It's quite convenient in large databases.

> But error messages doesn’t contain schema names.

> I have done a little patch with schema-qualified relation names in error
> messages that produced by foreign key constraints and triggers.

We have gone around on this before and pretty much concluded we didn't
want to do it; the demand is too small and the risk of breaking things
too large.  In particular, your patch as presented *would* break every
application that is still parsing primary error messages to extract
object names from them.

What seems more likely to be useful and to not break anything is to push
forward on adding PG_DIAG_SCHEMA_NAME and similar auxiliary fields to more
error messages (see errtable() and siblings).  That would allow
applications to extract this information automatically and reliably.
Only after that work is complete and there's been a reasonable amount of
time for clients to start relying on it can we really start thinking about
whacking the message texts around.

Aside from those points, it's quite unacceptable to format qualified names
as you propose here; they would really have to look more like "foo"."bar"
to prevent confusion.

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] Making tab-complete.c easier to maintain

2016-01-05 Thread Michael Paquier
On Wed, Jan 6, 2016 at 2:03 AM, Tom Lane  wrote:
> I wrote:
>> Michael Paquier  writes:
>>> So, here are the commands that still remain with TailMatches to cover
>>> this case, per gram.y:
>>> - CREATE TABLE
>>> - CREATE INDEX
>>> - CREATE VIEW
>>> - GRANT
>>> - CREATE TRIGGER
>>> - CREATE SEQUENCE
>>> New patches are attached.
>
>> I've reviewed and committed the first of these patches.
>
> I've pushed the second patch now.  I made a few adjustments --- notably,
> I didn't like the way you'd implemented '*' wildcards, because they
> wouldn't have behaved very sanely in combination with '|'.  The case
> doesn't come up in the current set of patterns, but we'll likely want it
> sometime.

Thanks! Let's consider this project as done then. That was a long way to it...
-- 
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] Optimizer questions

2016-01-05 Thread Alexander Korotkov
On Wed, Jan 6, 2016 at 12:08 AM, Tom Lane  wrote:

> konstantin knizhnik  writes:
> > 1. The cost compared in grouping_planner doesn't take in account price
> of get_authorized_users - it is not changed when I am altering function
> cost. Is it correct behavior?
>
> The general problem of accounting for tlist eval cost is not handled very
> well now, but especially not with respect to the idea that different paths
> might have different tlist costs.  I'm working on an upper-planner rewrite
> which should make this better, or at least make it practical to make it
> better.
>

Hmm... Besides costing it would be nice to postpone calculation of
expensive tlist functions after LIMIT.

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


Re: [HACKERS] Building pg_xlogdump reproducibly

2016-01-05 Thread Tom Lane
Christoph Berg  writes:
> Nod. Attached is a patch that covers all relevant $(wildcard)
> occurrences in Makefiles for devel.

Applied back to 9.3, which is as far as any of these cases exist.

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] Function and view to retrieve WAL receiver status

2016-01-05 Thread Haribabu Kommi
On Tue, Jan 5, 2016 at 10:24 PM, Michael Paquier
 wrote:
> On Tue, Jan 5, 2016 at 7:49 PM, Haribabu Kommi  
> wrote:
>> On Sat, Dec 19, 2015 at 12:54 AM, Michael Paquier
>>  wrote:
>>> On Fri, Dec 18, 2015 at 8:39 AM, Robert Haas  wrote:
 On Mon, Dec 14, 2015 at 7:23 PM, Michael Paquier
  wrote:
> On Tue, Dec 15, 2015 at 5:27 AM, Gurjeet Singh wrote:
>> The function, maybe. But emitting an all-nulls row from a view seems
>> counter-intuitive, at least when looking at it in context of relational
>> database.
>
> OK, noted. Any other opinions?

 I wouldn't bother with the view.  If we're going to do it, I'd say
 just provide the function and let people SELECT * from it if they want
 to.
>>>
>>> OK, I took some time to write a patch for that as attached, added in
>>> the next CF here:
>>> https://commitfest.postgresql.org/8/447/
>>> I am fine switching to an SRF depending on other opinions of people
>>> here, it just seems like an overkill knowing the uniqueness of the WAL
>>> sender in a server.
>>>
>>> I have finished with a function and a system view, this came up more
>>> in line with the existing things like pg_stat_archiver, and this makes
>>> as well the documentation clearer, at least that was my feeling when
>>> hacking that.
>>
>> I also feel showing NULL values may not be good, when there is
>> no walreceiver. Instead of SRF function to avoid showing NULL vallues
>> how about adding "WHERE s.pid IS NOT NULL" to the system view.
>> pid value cannot be NULL, until unless there is no walreceiver.
>
> Yeah, I would not mind switching it to that. A couple of other stat
> catalog views do it as well.

Following are my observations on the latest patch.

+ If no WAL receiver is present on the server queried,
+   a single tuple filled with NULL values is returned instead.
+  

The above documentation change is not required if we change the system
view.

+s.received_up_to_lsn,

The column name can be changed as "received_lsn" similar to "received_tli".
up_to may not be required.

+ XLogRecPtr received_up_lsn;
+ TimeLineID received_up_tli;

same as like above comment.

+ /* lock? */

I find out that walrcv data is updated only under mutex. it is better
to take that
mutex to provide a consistent snapshot data to user.


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


[HACKERS] Typo in create_tranform.sgml

2016-01-05 Thread Tatsuo Ishii
It seems there is a redundant word "function" in
doc/ref/create_tranform.sgml. Patch attached.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
diff --git a/doc/src/sgml/ref/create_transform.sgml b/doc/src/sgml/ref/create_transform.sgml
index d321dad..f44ee89 100644
--- a/doc/src/sgml/ref/create_transform.sgml
+++ b/doc/src/sgml/ref/create_transform.sgml
@@ -113,7 +113,7 @@ CREATE [ OR REPLACE ] TRANSFORM FOR type_name LANGUAG
type internal and return type internal.  The
actual argument will be of the type for the transform, and the function
should be coded as if it were.  (But it is not allowed to declare an
-   SQL-level function function returning internal without at
+   SQL-level function returning internal without at
least one argument of type internal.)  The actual return
value will be something specific to the language implementation.
   

-- 
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] No Issue Tracker - Say it Ain't So!

2016-01-05 Thread Jim Nasby

On 1/5/16 6:53 PM, Michael Paquier wrote:

On Wed, Jan 6, 2016 at 7:32 AM, Joshua D. Drake  wrote:

So I was on #postgresql today. I convinced a newer user that they could
easily contribute to PostgreSQL even if it was just doc patches. I described
the basic workflow and the individual was excited.

His first question?

linuxhiker:  git interface! Bug tracker for this anywhere?


Potential answer: Yes. As of now, pgsql-docs for doc issues, and
pgsql-bugs for actual bugs :)


Which doesn't help anyone, because neither of those provide a list of 
"hey, here's stuff you could do to contribute". The closest we come to 
that is the TODO, which isn't well known and has almost no items for 
newbies (and the newbie items that are there don't offer much advice).


The reason I this matters is because yesterday I posted a task for a new 
hacker with simple guidelines and 24 hours later it was completed[1]. 
That tells me there's people that would love to contribute but don't 
know how or what to contribute.


I realize a tracker *by itself* won't solve that, but it is the first 
place anyone that wants to contribute code is likely to look. So having 
one makes it more likely that new people will contribute.


On a related note, anyone interested in growing the community should 
take a look at [2]. tl;dr: best way to grow the community is to attract 
some folks that will make growing it their priority.


[1] http://www.postgresql.org/message-id/568ada20.7090...@bluetreble.com
[2] http://www.postgresql.org/message-id/568c6e6d.1040...@bluetreble.com
--
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] No Issue Tracker - Say it Ain't So!

2016-01-05 Thread Joshua D. Drake

On 01/05/2016 04:53 PM, Michael Paquier wrote:

On Wed, Jan 6, 2016 at 7:32 AM, Joshua D. Drake  wrote:

Hello,

So I was on #postgresql today. I convinced a newer user that they could
easily contribute to PostgreSQL even if it was just doc patches. I described
the basic workflow and the individual was excited.

His first question?

linuxhiker:  git interface! Bug tracker for this anywhere?


Potential answer: Yes. As of now, pgsql-docs for doc issues, and
pgsql-bugs for actual bugs :)


Yes but that is a horrible answer.

JD


--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


--
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] No Issue Tracker - Say it Ain't So!

2016-01-05 Thread Stephen Frost
Jim, all,

* Jim Nasby (jim.na...@bluetreble.com) wrote:
> Which doesn't help anyone, because neither of those provide a list
> of "hey, here's stuff you could do to contribute". The closest we
> come to that is the TODO, which isn't well known and has almost no
> items for newbies (and the newbie items that are there don't offer
> much advice).

Agreed.  I've not given up on the bugs.p.o project, but I've gotten
wrapped up in migrating the buildfarm server on to our infrastructure.
Hopefully that will be completed as early as this week and I'll be able
to refocus my infra time to finishing bugs.p.o.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] dynloader.h missing in prebuilt package for Windows?

2016-01-05 Thread Michael Paquier
On Wed, Jan 6, 2016 at 7:47 AM, Michael Paquier
 wrote:
> By the way, it is definitely wiser to wait for after the release of
> 9.5.0 to push that or something similar...

And I have added an entry in the CF app, let's not lose track of this item:
https://commitfest.postgresql.org/9/473/
-- 
Michael


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