Re: [HACKERS] PATCH: numeric timestamp in log_line_prefix

[David Rowley]

On 22 March 2015 at 12:47, Tomas Vondra 
wrote:

>
> I propose adding two new log_line_prefix escape sequences - %T and %M,
> doing the same thing as %t and %m, but formatting the value as a number.
>
>
Hi Tomas,

I just had a quick glance at this.
Is there a reason you didn't include code to support the space padding for
the new log_line_prefixes?
The others support  % in the prefix, to allow left or right
alignment of the item.

Also, what's the reason for timestamp_str? Could you not just use
appendStringInfo() and skip the temporary buffer?

Regards

David Rowley

Re: [HACKERS] [PATCH] Add transforms feature

[Pavel Stehule]

2015-03-22 5:45 GMT+01:00 Pavel Stehule :

>
>
> 2015-03-22 3:55 GMT+01:00 Peter Eisentraut :
>
>> Here is an updated patch.
>>
>> On 3/17/15 1:11 AM, Pavel Stehule wrote:
>> > 2015-03-17 2:51 GMT+01:00 Peter Eisentraut > > >:
>> >
>> > On 3/12/15 8:12 AM, Pavel Stehule wrote:
>> > > 1. fix missing semicolon pg_proc.h
>> > >
>> > > Oid protrftypes[1]; /* types for which to
>> apply
>> > > transforms */
>> >
>> > Darn, I thought I had fixed that.
>>
>> Fixed.
>>
>> > > 2. strange load lib by in sql scripts:
>> > >
>> > > DO '' LANGUAGE plperl;
>> > > SELECT NULL::hstore;
>> > >
>> > > use load plperl; load hstore; instead
>> >
>> > OK
>>
>> The reason I had actually not used LOAD is that LOAD requires a file
>> name, and the file name of those extensions is an implementation detail.
>>  So it is less of a violation to just execute something from those
>> modules rather than reach in and deal with the file directly.
>>
>> It's not terribly pretty either way, I admit.  A proper fix would be to
>> switch to lazy symbol resolution, but that would be a much bigger change.
>>
>> > > 3. missing documentation for new contrib modules,
>> >
>> > OK
>>
>> They actually are documented as part of the hstore and ltree modules
>> already.
>>
>> > > 4. pg_dump - wrong comment
>> > >
>> > > +<-><-->/*
>> > > +<-><--> * protrftypes was added at v9.4
>> > > +<-><--> */
>> >
>> > OK
>>
>> Fixed.
>>
>> > > 4. Why guc-use-transforms? Is there some possible negative side
>> effect
>> > > of transformations, so we have to disable it? If somebody don't
>> would to
>> > > use some transformations, then he should not to install some
>> specific
>> > > transformation.
>> >
>> > Well, there was extensive discussion last time around where people
>> > disagreed with that assertion.
>> >
>> >
>> > I don't like it, but I can accept it - it should not to impact a
>> > functionality.
>>
>> Removed.
>>
>> > > 5. I don't understand to motivation for introduction of
>> protrftypes in
>> > > pg_proc and TRANSFORM clause for CREATE FUNCTION - it is not
>> clean from
>> > > documentation, and examples in contribs works without it. Is it
>> this
>> > > functionality really necessary? Missing tests, missing examples.
>> >
>> > Again, this came out from the last round of discussion that people
>> > wanted to select which transforms to use and that the function
>> needs to
>> > remember that choice, so it doesn't depend on whether a transform
>> > happens to be installed or not.  Also, it's in the SQL standard
>> that way
>> > (by analogy).
>> >
>> >
>> > I am sorry, I didn't discuss this topic and I don't agree so it is good
>> > idea. I looked to standard, and I found CREATE TRANSFORM part there. But
>> > nothing else.
>> >
>> > Personally I am thinking, so it is terrible wrong idea, unclean,
>> > redundant. If we define TRANSFORM, then we should to use it. Not prepare
>> > bypass in same moment.
>> >
>> > Can be it faster, safer with it? I don't think.
>>
>> Well, I don't think there is any point in reopening this discussion.
>> This is a safety net of sorts that people wanted.  You can argue that it
>> would be more fun without it, but nobody else would agree.  There is
>> really no harm in keeping it.  All the function lookup is mostly cached
>> anyway.  The only time this is really important is for pg_dump to be
>> able to accurately restore function behavior.
>>
>
> 1. It add attribute to pg_proc, so impact is not minimal
>
> 2. Minimally it is not tested - there are no any test for this
> functionality
>

I am sorry, there is tests


>
> 3. I'll reread a discuss about this design - Now I am thinking so this
> duality (in design) is wrong - worse in relatively critical part of
> Postgres.
>
> I can mark this patch as "ready for commiter" with objection - It is task
> for commiter, who have to decide.
>
> Regards
>
> Pavel
>
>

Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates

[Andres Freund]

On March 22, 2015 6:17:28 AM GMT+01:00, Michael Paquier 
 wrote:
>On Sun, Mar 22, 2015 at 12:32 AM, Tom Lane  wrote:
>> Andres Freund  writes:
>>> Pushed with that additional change. Let's see if the buildfarm
>thinks.
>>
>> jacana, apparently alone among buildfarm members, does not like it.
>
>All the windows nodes don't pass tests with this patch, the difference
>is in the exponential precision: e+000 instead of e+00.

That's due to a different patch though, right? When I checked earlier only 
jacana had problems due to this, and it looked like random memory was being 
output. It's interesting that that's on the one windows (not cygwin) critter 
that does the 128bit dance...

-- 
Please excuse brevity and formatting - I am writing this on my mobile phone.

Andres Freund  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


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

Re: [HACKERS] INT64_MIN and _MAX

[Andres Freund]

On March 22, 2015 6:19:52 AM GMT+01:00, Andrew Gierth 
 wrote:
>> "Andrew" == Andrew Gierth  writes:
>
>> "Petr" == Petr Jelinek  writes:
>
 So wouldn't it make more sense to move these definitions into c.h
>and
> >>> standardize their usage?
>
>Petr> I was thinking the same when I've seen Peter's version of Numeric
> Petr> abbreviations patch. So +1 for that.
>
>Hm, it looks like the same could be said for INT32_MIN and _MAX; some
>places use INT_MIN etc., others say "we shouldn't assume int = int32"
>and use (-0x7fff - 1) or whatever instead.

I have been annoyed by this multiple times. I think we should make sure the C99 
defines are there (providing values if they aren't) and always use those. We've 
used them in parts of the tree long enough that it's unlikely to cause 
problems. Nothing is helped by using different things in other parts of the 
tree.

Willing to cook up a patch?


--- 
Please excuse brevity and formatting - I am writing this on my mobile phone.


-- 
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 toast table for pg_shseclabel but for pg_seclabel

[Andres Freund]

On March 22, 2015 3:15:07 AM GMT+01:00, Bruce Momjian  wrote:
>On Thu, Mar 19, 2015 at 11:50:36AM -0400, Bruce Momjian wrote:
>> > Then there's the other discussion about using the security labels
>> > structure for more than just security labels, which could end up
>with a
>> > lot of other use-cases where the "label" is even larger.
>> 
>> OK, the attached patch adds a TOAST table to the shared table
>> pg_shseclabel for use with long labels.  The new query output shows
>the
>> shared and non-shared seclabel tables now both have TOAST tables:
>> 
>>  test=> SELECT oid::regclass, reltoastrelid FROM pg_class WHERE
>relname IN ('pg_seclabel', 'pg_shseclabel');
>>oid  | reltoastrelid
>>  ---+---
>>   pg_seclabel   |  3598
>>   pg_shseclabel |  4060
>>  (2 rows)
>> 
>> Previously pg_shseclabel was zero.
>
>Patch applied.

Thanks.

-- 
Please excuse brevity and formatting - I am writing this on my mobile phone.

Andres Freund  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


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

Re: [HACKERS] Lets delete src/test/performance

[Andres Freund]

On March 22, 2015 3:21:34 AM GMT+01:00, Bruce Momjian  wrote:
>On Thu, Mar 19, 2015 at 09:57:05PM -0400, Bruce Momjian wrote:
>> > commit cf76759f34a172d424301cfa3723baee37f4a7ce
>> > Author: Vadim B. Mikheev 
>> > Date:   Fri Sep 26 14:55:21 1997 +
>> > 
>> > Start with performance suite.
>> 
>> Any objection if I remove the src/test/performance directory and all
>its
>> files?
>
>Done.

Thanks. I unfortunately had already completely forgotten about this.

-- 
Please excuse brevity and formatting - I am writing this on my mobile phone.

Andres Freund  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


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

Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates

[Michael Paquier]

On Sun, Mar 22, 2015 at 6:22 PM, Andres Freund  wrote:
> On March 22, 2015 6:17:28 AM GMT+01:00, Michael Paquier 
>  wrote:
>>On Sun, Mar 22, 2015 at 12:32 AM, Tom Lane  wrote:
>>> Andres Freund  writes:
 Pushed with that additional change. Let's see if the buildfarm
>>thinks.
>>>
>>> jacana, apparently alone among buildfarm members, does not like it.
>>
>>All the windows nodes don't pass tests with this patch, the difference
>>is in the exponential precision: e+000 instead of e+00.
>
> That's due to a different patch though, right? When I checked earlier only 
> jacana had problems due to this, and it looked like random memory was being 
> output. It's interesting that that's on the one windows (not cygwin) critter 
> that does the 128bit dance...

Yes, sorry, the e+000 stuff is from 959277a. This patch has visibly broken that:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2015-03-21%2003%3A01%3A21
-- 
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] Using 128-bit integers for sum, avg and statistics aggregates

[David Rowley]

On 22 March 2015 at 22:22, Andres Freund  wrote:

> On March 22, 2015 6:17:28 AM GMT+01:00, Michael Paquier <
> michael.paqu...@gmail.com> wrote:
> >On Sun, Mar 22, 2015 at 12:32 AM, Tom Lane  wrote:
> >> Andres Freund  writes:
> >>> Pushed with that additional change. Let's see if the buildfarm
> >thinks.
> >>
> >> jacana, apparently alone among buildfarm members, does not like it.
> >
> >All the windows nodes don't pass tests with this patch, the difference
> >is in the exponential precision: e+000 instead of e+00.
>
> That's due to a different patch though, right?


Yes this is due to cc0d90b.


> When I checked earlier only jacana had problems due to this, and it looked
> like random memory was being output. It's interesting that that's on the
> one windows (not cygwin) critter that does the 128bit dance...
>

Yeah, I can't recreate the issue locally on my windows machine, but I may
try with gcc if I can get some time.

Regards

David Rowley

Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates

[Andres Freund]

On March 22, 2015 10:34:04 AM GMT+01:00, Michael Paquier 
 wrote:
>On Sun, Mar 22, 2015 at 6:22 PM, Andres Freund 
>wrote:
>> On March 22, 2015 6:17:28 AM GMT+01:00, Michael Paquier
> wrote:
>>>On Sun, Mar 22, 2015 at 12:32 AM, Tom Lane  wrote:
 Andres Freund  writes:
> Pushed with that additional change. Let's see if the buildfarm
>>>thinks.

 jacana, apparently alone among buildfarm members, does not like it.
>>>
>>>All the windows nodes don't pass tests with this patch, the
>difference
>>>is in the exponential precision: e+000 instead of e+00.
>>
>> That's due to a different patch though, right? When I checked earlier
>only jacana had problems due to this, and it looked like random memory
>was being output. It's interesting that that's on the one windows (not
>cygwin) critter that does the 128bit dance...
>
>Yes, sorry, the e+000 stuff is from 959277a. This patch has visibly
>broken that:
>http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2015-03-21%2003%3A01%3A21

That's the stuff looking like random memory that I talk about above...

-- 
Please excuse brevity and formatting - I am writing this on my mobile phone.

Andres Freund  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


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

Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates

[Michael Paquier]

On Sun, Mar 22, 2015 at 6:34 PM, David Rowley  wrote:
> On 22 March 2015 at 22:22, Andres Freund  wrote:
>>
>> On March 22, 2015 6:17:28 AM GMT+01:00, Michael Paquier
>>  wrote:
>> >On Sun, Mar 22, 2015 at 12:32 AM, Tom Lane  wrote:
>> >> Andres Freund  writes:
>> >>> Pushed with that additional change. Let's see if the buildfarm
>> >thinks.
>> >>
>> >> jacana, apparently alone among buildfarm members, does not like it.
>> >
>> >All the windows nodes don't pass tests with this patch, the difference
>> >is in the exponential precision: e+000 instead of e+00.
>>
>> That's due to a different patch though, right?
>
>
> Yes this is due to cc0d90b.

Err, yes. This exp error is caused by the to_char patch (and that's
what I meant X)), bad copy-paste from here...
-- 
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] [PATCH] Add transforms feature

[Pavel Stehule]

2015-03-22 3:55 GMT+01:00 Peter Eisentraut :

> Here is an updated patch.
>
> On 3/17/15 1:11 AM, Pavel Stehule wrote:
> > 2015-03-17 2:51 GMT+01:00 Peter Eisentraut  > >:
> >
> > On 3/12/15 8:12 AM, Pavel Stehule wrote:
> > > 1. fix missing semicolon pg_proc.h
> > >
> > > Oid protrftypes[1]; /* types for which to apply
> > > transforms */
> >
> > Darn, I thought I had fixed that.
>
> Fixed.
>
> > > 2. strange load lib by in sql scripts:
> > >
> > > DO '' LANGUAGE plperl;
> > > SELECT NULL::hstore;
> > >
> > > use load plperl; load hstore; instead
> >
> > OK
>
> The reason I had actually not used LOAD is that LOAD requires a file
> name, and the file name of those extensions is an implementation detail.
>  So it is less of a violation to just execute something from those
> modules rather than reach in and deal with the file directly.
>
> It's not terribly pretty either way, I admit.  A proper fix would be to
> switch to lazy symbol resolution, but that would be a much bigger change.
>

ok, please, can comment the reason in test. little bit more verbose than
"make sure the prerequisite libraries are loaded". There is not clean, why
"LOAD" should not be used.



>
> > > 3. missing documentation for new contrib modules,
> >
> > OK
>
> They actually are documented as part of the hstore and ltree modules
> already.
>
> > > 4. pg_dump - wrong comment
> > >
> > > +<-><-->/*
> > > +<-><--> * protrftypes was added at v9.4
> > > +<-><--> */
> >
> > OK
>
> Fixed.
>
> > > 4. Why guc-use-transforms? Is there some possible negative side
> effect
> > > of transformations, so we have to disable it? If somebody don't
> would to
> > > use some transformations, then he should not to install some
> specific
> > > transformation.
> >
> > Well, there was extensive discussion last time around where people
> > disagreed with that assertion.
> >
> >
> > I don't like it, but I can accept it - it should not to impact a
> > functionality.
>
> Removed.
>
> > > 5. I don't understand to motivation for introduction of
> protrftypes in
> > > pg_proc and TRANSFORM clause for CREATE FUNCTION - it is not clean
> from
> > > documentation, and examples in contribs works without it. Is it
> this
> > > functionality really necessary? Missing tests, missing examples.
> >
> > Again, this came out from the last round of discussion that people
> > wanted to select which transforms to use and that the function needs
> to
> > remember that choice, so it doesn't depend on whether a transform
> > happens to be installed or not.  Also, it's in the SQL standard that
> way
> > (by analogy).
> >
> >
> > I am sorry, I didn't discuss this topic and I don't agree so it is good
> > idea. I looked to standard, and I found CREATE TRANSFORM part there. But
> > nothing else.
> >
> > Personally I am thinking, so it is terrible wrong idea, unclean,
> > redundant. If we define TRANSFORM, then we should to use it. Not prepare
> > bypass in same moment.
> >
> > Can be it faster, safer with it? I don't think.
>
> Well, I don't think there is any point in reopening this discussion.
> This is a safety net of sorts that people wanted.  You can argue that it
> would be more fun without it, but nobody else would agree.  There is
> really no harm in keeping it.  All the function lookup is mostly cached
> anyway.  The only time this is really important is for pg_dump to be
> able to accurately restore function behavior.
>

So I reread discussion about this topic and I can see some benefits of it.
Still - what I dislike is the behave of TRANSFORM ALL. The fact, so newly
installed transformations are not used for registered functions (and
reregistration is needed) is unhappy. I understand, so it can depends on
implementation requirements.

Isn't better doesn't support "TRANSFORM ALL" clause? If somebody would to
use transformations - then he have to explicitly enable it by "TRANSFORM
FOR TYPE" ? It is safe and without possible user unexpectations.

Small issue - explicitly setted transformation types should be visible in
\sf and \df+ commands.

Regards

Pavel

[HACKERS] Fix pgbench --progress report under (very) low rate

[Fabien COELHO]

When running with low rate, the --progress is only printed when there is 
some activity, which makes it quite irregular, including some catching up 
with stupid tps figures.


Shame on me for this "feature" (aka bug) in the first place.

This patch fixes this behavior by considering the next report time as a 
target to meet as well as transaction schedule times.


Before the patch:

 sh> ./pgbench -R 0.5 -T 10 -P 1 -S
 progress: 1.7 s, 0.6 tps, lat 6.028 ms stddev -nan, lag 1.883 ms
 progress: 2.2 s, 2.3 tps, lat 2.059 ms stddev -nan, lag 0.530 ms
 progress: 7.3 s, 0.4 tps, lat 2.944 ms stddev 1.192, lag 2.624 ms
 progress: 7.3 s, 1402.5 tps, lat 5.115 ms stddev 0.000, lag 0.000 ms
 progress: 7.3 s, 0.0 tps, lat -nan ms stddev -nan, lag inf ms
 progress: 7.3 s, 335.2 tps, lat 3.106 ms stddev 0.000, lag 0.000 ms
 progress: 8.8 s, 0.0 tps, lat -nan ms stddev -nan, lag inf ms
 progress: 8.8 s, 307.6 tps, lat 4.855 ms stddev -nan, lag 0.000 ms
 progress: 10.0 s, 0.0 tps, lat -nan ms stddev -nan, lag -nan ms

After the patch:

 sh> ./pgbench -R 0.5 -T 10 -P 1 -S
 progress: 1.0 s, 0.0 tps, lat -nan ms stddev -nan, lag -nan ms
 progress: 2.0 s, 1.0 tps, lat 5.980 ms stddev 0.000, lag 0.733 ms
 progress: 3.0 s, 1.0 tps, lat 1.905 ms stddev 0.000, lag 0.935 ms
 progress: 4.0 s, 1.0 tps, lat 3.966 ms stddev 0.000, lag 0.623 ms
 progress: 5.0 s, 2.0 tps, lat 2.527 ms stddev 1.579, lag 0.512 ms
 progress: 6.0 s, 0.0 tps, lat -nan ms stddev -nan, lag -nan ms
 progress: 7.0 s, 0.0 tps, lat -nan ms stddev -nan, lag -nan ms
 progress: 8.0 s, 1.0 tps, lat 1.750 ms stddev 0.000, lag 0.767 ms
 progress: 9.0 s, 0.0 tps, lat -nan ms stddev -nan, lag -nan ms
 progress: 10.0 s, 2.0 tps, lat 2.403 ms stddev 1.386, lag 0.357 ms

To answer a question before it is asked: I run low rates because I'm 
looking at latency (rather than throughput) under different conditions. 
For instance with the above tests, the latency is about 3 ms, but it 
varies with the tps: (0.5 tps => 3 ms, 10 tps => 1 ms, 50 tps => 0.8 ms,

100 tps => 0.5 ms, 200 tps => 0.75 ms, 1000 tps => 0.5 ms...).

--
Fabien.diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 706fdf5..3536782 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -3584,6 +3584,28 @@ threadRun(void *arg)
 maxsock = sock;
 		}
 
+		/* also meet the next progress report time if needed */
+		if (progress && min_usec > 0
+#ifndef PTHREAD_FORK_EMULATION
+			&& thread->tid == 0
+#endif /* !PTHREAD_FORK_EMULATION */
+			)
+		{
+			/* get current time if nedded */
+			if (now_usec == 0)
+			{
+instr_time	now;
+
+INSTR_TIME_SET_CURRENT(now);
+now_usec = INSTR_TIME_GET_MICROSEC(now);
+			}
+
+			if (now_usec >= next_report)
+min_usec = 0;
+			else if ((next_report - now_usec) < min_usec)
+min_usec = next_report - now_usec;
+		}
+
 		if (min_usec > 0 && maxsock != -1)
 		{
 			int			nsocks; /* return from select(2) */

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

Re: [HACKERS] proposal: searching in array function - array_position

[Dean Rasheed]

On 22 March 2015 at 06:11, Pavel Stehule  wrote:
> Hi
>
> here is updated patch with array_position, array_positions implementation.
>
> It is based on committed code - so please, revert commit
> 13dbc7a824b3f905904cab51840d37f31a07a9ef and apply this patch
>

I checked this and the changes look good, except that you missed a
couple of places in src/include/utils/array.h that need updating.

In the public docs, you should s/position/subscript because that's the
term used throughout the docs for an index into an array. I still like
the name array_position() for the function though, because it's
consistent with the existing position() functions.

Regards,
Dean


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

Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates

[Petr Jelinek]

On 22/03/15 10:35, Andres Freund wrote:

On March 22, 2015 10:34:04 AM GMT+01:00, Michael Paquier 
 wrote:

On Sun, Mar 22, 2015 at 6:22 PM, Andres Freund 

That's due to a different patch though, right? When I checked earlier

only jacana had problems due to this, and it looked like random memory
was being output. It's interesting that that's on the one windows (not
cygwin) critter that does the 128bit dance...

Yes, sorry, the e+000 stuff is from 959277a. This patch has visibly
broken that:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2015-03-21%2003%3A01%3A21


That's the stuff looking like random memory that I talk about above...



If you look at it closely, it's actually not random memory. At least in 
the first 2 failing tests which are not obfuscated by aggregates on top 
of aggregates. It looks like first NumericDigit is ok and the second one 
is corrupted (there are only 2 NumericDigits in those numbers). Of 
course the conversion to Numeric is done from the end so it looks like 
only the last computation/pointer change/something stays ok while the 
rest got corrupted.


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


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

Re: [HACKERS] proposal: searching in array function - array_position

[Pavel Stehule]

2015-03-22 11:30 GMT+01:00 Dean Rasheed :

> On 22 March 2015 at 06:11, Pavel Stehule  wrote:
> > Hi
> >
> > here is updated patch with array_position, array_positions
> implementation.
> >
> > It is based on committed code - so please, revert commit
> > 13dbc7a824b3f905904cab51840d37f31a07a9ef and apply this patch
> >
>
> I checked this and the changes look good, except that you missed a
> couple of places in src/include/utils/array.h that need updating.
>
> In the public docs, you should s/position/subscript because that's the
> term used throughout the docs for an index into an array. I still like
> the name array_position() for the function though, because it's
> consistent with the existing position() functions.
>

updated patch




>
> Regards,
> Dean
>
commit 86ec9f27c473de3de0b1fee3c90d40a610fbbdcd
Author: Pavel Stehule 
Date:   Sun Mar 22 11:55:24 2015 +0100

doc and header files fix

diff --git a/doc/src/sgml/array.sgml b/doc/src/sgml/array.sgml
index 9ea1068..5e4130a 100644
--- a/doc/src/sgml/array.sgml
+++ b/doc/src/sgml/array.sgml
@@ -600,6 +600,25 @@ SELECT * FROM sal_emp WHERE pay_by_quarter && ARRAY[1];
   index, as described in .
  
 
+ 
+  You can also search for specific values in an array using the array_position
+  and array_positions functions. The former returns the subscript of
+  the first occurrence of a value in an array; the latter returns an array with the
+  subscripts of all occurrences of the value in the array.  For example:
+
+
+SELECT array_position(ARRAY['sun','mon','tue','wed','thu','fri','sat'], 'mon');
+ array_positions
+-
+ 2
+
+SELECT array_positions(ARRAY[1, 4, 3, 1, 3, 4, 2, 1], 1);
+ array_positions
+-
+ {1,4,8}
+
+ 
+
  
   
Arrays are not sets; searching for specific array elements
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index c89f343..8b12c26 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -11480,6 +11480,12 @@ SELECT NULLIF(value, '(none)') ...
 array_lower
   
   
+array_position
+  
+  
+array_positions
+  
+  
 array_prepend
   
   
@@ -11599,6 +11605,32 @@ SELECT NULLIF(value, '(none)') ...

 
  
+  array_position(anyarray, anyelement , int)
+ 
+
+int
+returns the subscript of the first occurrence of the second
+argument in the array, starting at the element indicated by the third
+argument or at the first element (array must be one-dimensional)
+array_position(ARRAY['sun','mon','tue','wed','thu','fri','sat'], 'mon')
+2
+   
+   
+
+ 
+  array_positions(anyarray, anyelement)
+ 
+
+int[]
+returns an array of subscripts of all occurrences of the second
+argument in the array given as first argument (array must be
+one-dimensional)
+array_positions(ARRAY['A','A','B','A'], 'A')
+{1,2,4}
+   
+   
+
+ 
   array_prepend(anyelement, anyarray)
  
 
@@ -11708,6 +11740,23 @@ NULL baz(3 rows)
 
 

+In array_position and array_positions,
+each array element is compared to the searched value using
+IS NOT DISTINCT FROM semantics.
+   
+
+   
+In array_position, NULL is returned
+if the value is not found.
+   
+
+   
+In array_positions, NULL is returned
+only if the array is NULL; if the value is not found in
+the array, an empty array is returned instead.
+   
+
+   
 In string_to_array, if the delimiter parameter is
 NULL, each character in the input string will become a separate element in
 the resulting array.  If the delimiter is an empty string, then the entire
diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c
index 6679333..c0bfd33 100644
--- a/src/backend/utils/adt/array_userfuncs.c
+++ b/src/backend/utils/adt/array_userfuncs.c
@@ -12,9 +12,14 @@
  */
 #include "postgres.h"
 
+#include "catalog/pg_type.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
+#include "utils/typcache.h"
+
+
+static Datum array_position_common(FunctionCallInfo fcinfo);
 
 
 /*
@@ -652,3 +657,298 @@ array_agg_array_finalfn(PG_FUNCTION_ARGS)
 
 	PG_RETURN_DATUM(result);
 }
+
+/*-
+ * array_position, array_position_start :
+ *			return the offset of a value in an array.
+ *
+ * IS NOT DISTINCT FROM semantics are used for comparisons.  Return NULL when
+ * the value is not found.
+ *-
+ */
+Datum
+array_position(PG_FUNCTION_ARGS)
+{
+	return array_position_common(fcinfo);
+}
+
+Datum
+array_position_start(PG_FUNCTION_ARGS)
+{
+	return array_position_common(fcinfo);
+}
+
+/*
+ * array_position_common
+ * 		Common code for array_position and array_position_start
+ *
+ * The

Re: [HACKERS] printing table in asciidoc with psql

[Michael Paquier]

On Sun, Mar 22, 2015 at 10:09 AM, Bruce Momjian  wrote:
> On Sat, Mar 21, 2015 at 09:20:03PM +0900, Michael Paquier wrote:
>> This does not work:
>> =# create table "5 2.2+^.^" ();
>> CREATE TABLE
>> =# \pset format asciidoc
>> Output format is asciidoc.
>> =# \d
>>
>> .List of relations
>> [options="header",cols="> |
>> ^l|Schema ^l|Name ^l|Type ^l|Owner
>> |public|5 2.2+^.^|table|ioltas
>> |
>>
>> 
>> (1 row)
>> 
>>
>> I think that we should really put additional spaces on the left side
>> of the column separators "|". For example, this line:
>> |public|5 2.2+^.^|table|ioltas
>> should become that:
>> |public |5 2.2+^.^ |table |ioltas
>> And there is no problem.
>
> I have updated the attached patch to do as you suggested.  Please also
> test the \x output.  Thanks.

Indeed. If I use a specific column name like this one, I am seeing
problems with the expanded mode:
=# create table "5 2.2+^.^" ("5 2.2+^.^" int);
CREATE TABLE
=# \x
Expanded display is on.
=# INSERT INTO "5 2.2+^.^" VALUES (1);
INSERT 0 1
=# table "5 2.2+^.^";

[cols="h,l",frame="none"]
|
2+^|Record 1
<|5 2.2+^.^ >|1
|

In this case the record is printed like that:
5 2.2+.
While it should show up like that:
5 2.2+^.^

Regards,
-- 
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] PITR failing to stop before DROP DATABASE

[Christoph Berg]

Re: Bruce Momjian 2015-03-20 <20150320223549.gz6...@momjian.us>
> > > >So my suggestion for a simple fix would be to make DROP DATABASE
> > > >execute a short fake transaction before it starts deleting files and
> > > >then continue as before. This would serve as a stopping point for
> > > >recovery_target_time to run into. (We could still fix this properly
> > > >later, but this idea seems like a good fix for a practical problem
> > > >that doesn't break anything else.)
> > > 
> > > Yeah, seems reasonable.
> > 
> > Here's a first shot at a patch. It's not working yet because I think
> > the commit isn't doing anything because no work was done in the
> > transaction yet.
> 
> Where are we on this?

I guess my patch could be fixed by forcing it to acquire a transaction
id (SELECT txid_current() or whatever seems suitable), but my insight
into the gory backend details is limited, so it'd be better if someone
with more clue tried to fix it.


Re: Heikki Linnakangas 2014-11-25 <5474b848.3060...@vmware.com>
> >db1 is registered in pg_database, but the directory is missing on
> >disk.
> 
> Yeah, DROP DATABASE cheats. It deletes all the files first, and commits the
> transaction only after that. There's this comment at the end of dropdb()
> function:
> 
> > /*
> >  * Force synchronous commit, thus minimizing the window between removal 
> > of
> >  * the database files and commital of the transaction. If we crash 
> > before
> >  * committing, we'll have a DB that's gone on disk but still there
> >  * according to pg_database, which is not good.
> >  */
> 
> So you could see the same after crash recovery, but it's a lot easier to
> reproduce with PITR.
> 
> This could be fixed by doing DROP DATABASE the same way we do DROP TABLE. At
> the DROP DATABASE command, just memorize the OID of the dropped database,
> but don't delete anything yet. Perform the actual deletion after flushing
> the commit record to disk. But then you would have the opposite problem -
> you might be left with a database that's dropped according to pg_database,
> but the files are still present on disk.

This seems to be the better idea anyway (and was mentioned again when
I talked to Heikki and Andres about it at FOSDEM).

The "opposite" problem wouldn't be so bad I guess - it might be
visible in practise where you could easily clean up later, but the
original problem is pretty bad if you hit it when trying to do PITR
because something bad happened. (And we treat DROP TABLE the same.)

Christoph
-- 
c...@df7cb.de | http://www.df7cb.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] PITR failing to stop before DROP DATABASE

[Andres Freund]

On March 22, 2015 12:17:57 PM GMT+01:00, Christoph Berg  wrote:
>Re: Bruce Momjian 2015-03-20 <20150320223549.gz6...@momjian.us>
>> > > >So my suggestion for a simple fix would be to make DROP DATABASE
>> > > >execute a short fake transaction before it starts deleting files
>and
>> > > >then continue as before. This would serve as a stopping point
>for
>> > > >recovery_target_time to run into. (We could still fix this
>properly
>> > > >later, but this idea seems like a good fix for a practical
>problem
>> > > >that doesn't break anything else.)
>> > > 
>> > > Yeah, seems reasonable.
>> > 
>> > Here's a first shot at a patch. It's not working yet because I
>think
>> > the commit isn't doing anything because no work was done in the
>> > transaction yet.
>> 
>> Where are we on this?
>
>I guess my patch could be fixed by forcing it to acquire a transaction
>id (SELECT txid_current() or whatever seems suitable), but my insight
>into the gory backend details is limited, so it'd be better if someone
>with more clue tried to fix it.

It'd need to do a GetTopTransactionId().

>Re: Heikki Linnakangas 2014-11-25 <5474b848.3060...@vmware.com>
>> >db1 is registered in pg_database, but the directory is missing on
>> >disk.
>> 
>> Yeah, DROP DATABASE cheats. It deletes all the files first, and
>commits the
>> transaction only after that. There's this comment at the end of
>dropdb()
>> function:
>> 
>> >/*
>> > * Force synchronous commit, thus minimizing the window between
>removal of
>> > * the database files and commital of the transaction. If we crash
>before
>> > * committing, we'll have a DB that's gone on disk but still there
>> > * according to pg_database, which is not good.
>> > */
>> 
>> So you could see the same after crash recovery, but it's a lot easier
>to
>> reproduce with PITR.
>> 
>> This could be fixed by doing DROP DATABASE the same way we do DROP
>TABLE. At
>> the DROP DATABASE command, just memorize the OID of the dropped
>database,
>> but don't delete anything yet. Perform the actual deletion after
>flushing
>> the commit record to disk. But then you would have the opposite
>problem -
>> you might be left with a database that's dropped according to
>pg_database,
>> but the files are still present on disk.
>
>This seems to be the better idea anyway (and was mentioned again when
>I talked to Heikki and Andres about it at FOSDEM).


Actually allowing for things like this was one of the reasons for the 
commit/abort extensibility stuff I committed a few days ago.

--- 
Please excuse brevity and formatting - I am writing this on my mobile phone.


-- 
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] New functions

[Dmitry Voronin]

Hello, Michael.

Please, attach new version of my patch to commitfest page.

-- 
Best regards, Dmitry Voronin
*** /dev/null
--- b/contrib/sslinfo/sslinfo--1.0--1.1.sql
***
*** 0 
--- 1,21 
+ /* contrib/sslinfo/sslinfo--1.0--1.1.sql */
+ 
+ -- complain if script is sourced in psql, rather than via CREATE EXTENSION
+ \echo Use "ALTER EXTENSION sslinfo UPDATE TO '1.1'" to load this file. \quit
+ 
+ CREATE OR REPLACE FUNCTION ssl_extension_value(text) RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_extension_value'
+ LANGUAGE C STRICT;
+ 
+ CREATE OR REPLACE FUNCTION ssl_extension_is_critical(text) RETURNS boolean
+ AS 'MODULE_PATHNAME', 'ssl_extension_is_critical'
+ LANGUAGE C STRICT;
+ 
+ CREATE TYPE extension AS (
+ name text,
+ value text
+ );
+ 
+ CREATE OR REPLACE FUNCTION ssl_extension_names() RETURNS SETOF extension 
+ AS 'MODULE_PATHNAME', 'ssl_extension_names'
+ LANGUAGE C STRICT;
*** /dev/null
--- b/contrib/sslinfo/sslinfo--1.1.sql
***
*** 0 
--- 1,58 
+ /* contrib/sslinfo/sslinfo--1.1.sql */
+ 
+ -- complain if script is sourced in psql, rather than via CREATE EXTENSION
+ \echo Use "CREATE EXTENSION sslinfo" to load this file. \quit
+ 
+ CREATE FUNCTION ssl_client_serial() RETURNS numeric
+ AS 'MODULE_PATHNAME', 'ssl_client_serial'
+ LANGUAGE C STRICT;
+ 
+ CREATE FUNCTION ssl_is_used() RETURNS boolean
+ AS 'MODULE_PATHNAME', 'ssl_is_used'
+ LANGUAGE C STRICT;
+ 
+ CREATE FUNCTION ssl_version() RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_version'
+ LANGUAGE C STRICT;
+ 
+ CREATE FUNCTION ssl_cipher() RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_cipher'
+ LANGUAGE C STRICT;
+ 
+ CREATE FUNCTION ssl_client_cert_present() RETURNS boolean
+ AS 'MODULE_PATHNAME', 'ssl_client_cert_present'
+ LANGUAGE C STRICT;
+ 
+ CREATE FUNCTION ssl_client_dn_field(text) RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_client_dn_field'
+ LANGUAGE C STRICT;
+ 
+ CREATE FUNCTION ssl_issuer_field(text) RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_issuer_field'
+ LANGUAGE C STRICT;
+ 
+ CREATE FUNCTION ssl_client_dn() RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_client_dn'
+ LANGUAGE C STRICT;
+ 
+ CREATE FUNCTION ssl_issuer_dn() RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_issuer_dn'
+ LANGUAGE C STRICT;
+ 
+ /* new in 1.1 */
+ CREATE OR REPLACE FUNCTION ssl_extension_value(text) RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_extension_value'
+ LANGUAGE C STRICT;
+ 
+ CREATE OR REPLACE FUNCTION ssl_extension_is_critical(text) RETURNS boolean
+ AS 'MODULE_PATHNAME', 'ssl_extension_is_critical'
+ LANGUAGE C STRICT;
+ 
+ CREATE TYPE extension AS (
+ name text,
+ value text
+ );
+ 
+ CREATE OR REPLACE FUNCTION ssl_extension_names() RETURNS SETOF extension 
+ AS 'MODULE_PATHNAME', 'ssl_extension_names'
+ LANGUAGE C STRICT;
*** a/contrib/sslinfo/sslinfo.c
--- b/contrib/sslinfo/sslinfo.c
***
*** 14,21 
--- 14,23 
  #include "miscadmin.h"
  #include "utils/builtins.h"
  #include "mb/pg_wchar.h"
+ #include "funcapi.h"
  
  #include 
+ #include 
  #include 
  
  PG_MODULE_MAGIC;
***
*** 23,28  PG_MODULE_MAGIC;
--- 25,31 
  static Datum X509_NAME_field_to_text(X509_NAME *name, text *fieldName);
  static Datum X509_NAME_to_text(X509_NAME *name);
  static Datum ASN1_STRING_to_text(ASN1_STRING *str);
+ static bool get_extension(X509 *cert, const char *ext_name, X509_EXTENSION **extension);
  
  
  /*
***
*** 354,356  ssl_issuer_dn(PG_FUNCTION_ARGS)
--- 357,581 
  		PG_RETURN_NULL();
  	return X509_NAME_to_text(X509_get_issuer_name(MyProcPort->peer));
  }
+ 
+ 
+ /*
+  * Returns extension object by given certificate and extension's name.
+  *
+  * Try to get extension from certificate by extension's name.
+  * We returns at extension param pointer to X509_EXTENSION.
+  *
+  * Returns true, if we have found extension in certificate and false, if we not.
+  */
+ static bool get_extension(X509* cert, const char *ext_name, X509_EXTENSION **extension)
+ {
+ 	int 	nid = 0;
+ 	int 	loc = 0;
+ 
+ 	/* try to convert extension name to ObjectID */
+ 	nid = OBJ_txt2nid(ext_name);
+ 	/* Not success ? */
+ 	if (nid == NID_undef)
+ 		return false;
+ 
+ 	loc = X509_get_ext_by_NID(cert, nid, -1);
+ 
+ 	/* palloc memory for extension and copy it */
+ 	*extension = (X509_EXTENSION *) palloc(sizeof(X509_EXTENSION *));
+ 	memcpy(*extension, X509_get_ext(cert, loc), sizeof(X509_EXTENSION));
+ 
+ 	return true;
+ }
+ 
+ 
+ /* Returns value of extension
+  *
+  * This function returns value of extension by given name in client certificate.
+  *
+  * Returns text datum.
+  */
+ PG_FUNCTION_INFO_V1(ssl_extension_value);
+ Datum
+ ssl_extension_value(PG_FUNCTION_ARGS)
+ {
+ 	X509			   *cert = MyProcPort->peer;
+ 	X509_EXTENSION	   *ext = NULL;
+ 	char			   *ext_name = text_to_cstring(PG_GETARG_TEXT_P(0));
+ 	BIO   *membuf = NULL;
+ 	char			   *val = NULL;
+ 	charnullterm = '\0';
+ 	boolerror = false;
+ 
+ 	/* If we have no ssl security */
+ 	if (cert == NULL)
+ 		PG_RET

Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates

[Andreas Karlsson]

On 03/22/2015 11:47 AM, Petr Jelinek wrote:

On 22/03/15 10:35, Andres Freund wrote:

http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2015-03-21%2003%3A01%3A21



That's the stuff looking like random memory that I talk about above...



If you look at it closely, it's actually not random memory. At least in
the first 2 failing tests which are not obfuscated by aggregates on top
of aggregates. It looks like first NumericDigit is ok and the second one
is corrupted (there are only 2 NumericDigits in those numbers). Of
course the conversion to Numeric is done from the end so it looks like
only the last computation/pointer change/something stays ok while the
rest got corrupted.


Would this mean the bug is most likely somewhere in 
int128_to_numericvar()? Maybe that version of gcc has a bug in some 
__int128 operator or I messed up the code there somehow.


Andreas



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

Re: [HACKERS] Remove fsync ON/OFF as a visible option?

[Andres Freund]

On 2015-03-21 13:53:47 -0700, Josh Berkus wrote:
> Coincidentally, I am just at this moment performance testing "running
> with scissors mode" for PostgreSQL on AWS.   When intentional, this mode
> is useful for spinning up lots of read-only replicas which are intended
> mainly as cache support, something I've done at various dot-coms.

Which is where fsync=off barely has any effect?

> So, -1 on removing the setting; it is useful to some users.

Agreed on that.

> Further, full_page_writes=off is supposedly safe on any copy-on-write
> filesystem, such as ZFS.

FWIW, I think that's a myth. One I heard various versions of by now. As
long as the OSs page size (4kb nearly everywhere) is different from
postgres' (8kb) you can have torn pages. Even if individual filesystem
page writes are atomic.

> The proposal that we make certain settings "only available via ALTER
> SYSTEM" also doesn't make sense; ALTER SYSTEM isn't capable of writing
> any settings which aren't available in postgresql.conf.

+1. I think it's a pretty absurd sugestion.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Order of enforcement of CHECK constraints?

[David Steele]

On 3/20/15 3:37 PM, Tom Lane wrote:
> =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?=  writes:
>> On Fri, Mar 20, 2015 at 4:19 PM, Peter Geoghegan  wrote:
>>> On Fri, Mar 20, 2015 at 12:15 PM, Tom Lane  wrote:
 We could fix it by, say, having CheckConstraintFetch() sort the
 constraints by name after loading them.
> 
>>> What not by OID, as with indexes? Are you suggesting that this would
>>> become documented behavior?
> 
>> I think they should be executed in alphabetical order like triggers.
> 
> Yeah.  We already have a comparable, and documented, behavior for
> triggers, so if we're going to do anything about this I'd vote for
> sorting by name (or more specifically, by strcmp()).
> 
>   regards, tom lane

+1 for strcmp() ordering.  Not only is this logical and consistent with
the way triggers are fired, names can be manipulated by the user to
force order when desired.  Not sure if that's as important for check
constraints as it is for triggers but it might be useful, even if only
for things like unit tests.

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



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Order of enforcement of CHECK constraints?

[David G. Johnston]

On Sunday, March 22, 2015, David Steele  wrote:

> On 3/20/15 3:37 PM, Tom Lane wrote:
> > =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?=  > writes:
> >> On Fri, Mar 20, 2015 at 4:19 PM, Peter Geoghegan  > wrote:
> >>> On Fri, Mar 20, 2015 at 12:15 PM, Tom Lane  > wrote:
>  We could fix it by, say, having CheckConstraintFetch() sort the
>  constraints by name after loading them.
> >
> >>> What not by OID, as with indexes? Are you suggesting that this would
> >>> become documented behavior?
> >
> >> I think they should be executed in alphabetical order like triggers.
> >
> > Yeah.  We already have a comparable, and documented, behavior for
> > triggers, so if we're going to do anything about this I'd vote for
> > sorting by name (or more specifically, by strcmp()).
> >
> >   regards, tom lane
>
> +1 for strcmp() ordering.  Not only is this logical and consistent with
> the way triggers are fired, names can be manipulated by the user to
> force order when desired.  Not sure if that's as important for check
> constraints as it is for triggers but it might be useful, even if only
> for things like unit tests.
>
> --
> - David Steele
> da...@pgmasters.net 
>
>
It would be nice to know that, at scale, the added comparisons are
negligible since it almost all cases all checks are run and pass and the
order is irrelevant.  Would it be possible for all check constraints to
always be run and the resultant error outputs stored in an array which
could then be sorted before it is printed?  You get better and stable
output for errors while minimally impacting good inserts.

David J.

Re: [HACKERS] Zero-padding and zero-masking fixes for to_char(float)

[Bruce Momjian]

On Sun, Mar 22, 2015 at 01:42:56AM -0400, Tom Lane wrote:
> David Rowley  writes:
> > This seems to have broken jacana.  Looks like MSVC by default has a 3 digit
> > exponent.
> 
> jacana was broken before this patch; but some other Windows critters
> are now unhappy as well.
> 
> > Going by this:
> > https://msdn.microsoft.com/en-us/library/0fatw238(v=vs.80).aspx it seems
> > that it can quite easily be set back to 2.
> 
> > I've attached a patch which seems to fix the issue.
> 
> That seems likely to have side-effects far beyond what's appropriate.
> We have gone out of our way to accommodate 3-digit exponents in other
> tests.  What I want to know is why this patch created a 3-digit output
> where there was none before.

I was wondering the same thing too, but when I grep'ed the regression
output files looking for exponents, I found float4-exp-three-digits.out
and int8-exp-three-digits.out.  I think this means we have had this
issue before, and we are going to have to either create a
numeric-exp-three-digits.out alternate output file, move the test to one
of the existing exp-three-digits files, or remove the tests.  Sorry, I
didn't expect any problems with this patch as it was so small and
localized.

What has me more concerned is the Solaris 10 failure.  This query:

SELECT to_char(float8 '999', 'D' || repeat('9', 
1000));

expects this:

999.000...

but on Solaris 10 gets this:

.00

Yes, the nines are gone, and only this query is failing.  Oddly, this
query did not fail, though the only difference is fewer decimal digits:

SELECT to_char(float8 '999', 'D');

This smells like a libc bug, e.g. OmniOS 5.11 passed the test.

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

  + Everyone has their own god. +


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

Re: [HACKERS] Remove fsync ON/OFF as a visible option?

[Euler Taveira]

On 21-03-2015 17:53, Josh Berkus wrote:
> Now, I have *long* been an advocate that we should ship a "stripped"
> PostgreSQL.conf which has only the most commonly used settings, and
> leave the rest of the settings in the docs and
> share/postgresql/postgresql.conf.advanced.  Here's my example of such a
> file, tailored to PostgreSQL 9.3:
> 
+1. I agree that common used settings in a postgresql.conf file is
useful for newbies. How do we ship it?

(i) replace postgresql.conf with newbie.conf at $PGDATA;
(ii) let postgresql.conf alone and include newbie.conf at the end;
(iii) install newbie.conf at share directory and let packagers decide to
replace postgresql.conf with it or not;
(iv) install newbie.conf at share directory and add an option in initdb
to select it as postgresql.conf, say, --config=simple.

As a starting point, (i) could be too radical because some DBAs are used
to check that occasional parameter at postgresql.conf. (ii) will
advocate the newbie configuration file. However, do we want a new
configuration file? We already have two and another one could be very
confusing. The option (iii) will be effective if popular distributions
decided to use newbie.conf as postgresql.conf. An the last option is a
flexible way to install a basic configuration (and imo is the way to
satisfy those that want basic configuration file available). It also has
a way to extend that option to other setups like one-file-per-section or
developer-conf.

The downside of the proposed newbie.conf is that we need to maintain
another configuration file. During beta time, some parameters could be
added/removed to/from newbie.conf.

> https://github.com/pgexperts/accidentalDBA/blob/master/vagrant/setup/postgres/postgresql.conf
> 
Your example is an good resource for newbies. I would like to see an
archive section (separated from replication) and some more log options
(where is log_file_prefix and log_duration?). port? A ssl section?
track_function? That could have others but those are on my preference list.

> While we likely wouldn't want to ship all of the advice in the comments
> in that file (the calculations, in particular, have been questioned
> since they were last tested with PostgreSQL 8.3), that gives you an
> example of what a simple/mainstream pg.conf could look like.  I would
> further advocate that that file be broken up into segments (resources,
> logging, connects, etc.) and placed in conf.d/
> 
IMHO too many files could confuse simple installations.

> All that being said, what *actually* ships with PostgreSQL is up to the
> packagers, so anything we do with pg.conf will have a limited impact
> unless we get them on board with the idea.
> 
In my experience, packagers tend to follow the default postgresql.conf.
They don't add or remove parameters but replace values.


-- 
   Euler Taveira   Timbira - http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento


-- 
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: numeric timestamp in log_line_prefix

[Fabien COELHO]

About the feature: I find it is a good thing. It may help scripting over 
the logs, for instance to compute delays between events, whereas the full 
date-time-tz syntax is maybe nice but heavier to work with afterwards.


In addition to the comments already made (typo in doc, padding...):

+sprintf(timestamp_str, "%ld.%.03d",
+tv.tv_sec, (int)(tv.tv_usec / 1000));

I'm not sure that the "." in "%.03d" is useful. ISTM that it is used for 
floatting point formatting, but is not needed with integers.


--
Fabien.


--
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] Zero-padding and zero-masking fixes for to_char(float)

[Noah Misch]

On Sun, Mar 22, 2015 at 11:22:26AM -0400, Bruce Momjian wrote:
> What has me more concerned is the Solaris 10 failure.  This query:
> 
>   SELECT to_char(float8 '999', 'D' || repeat('9', 
> 1000));
> 
> expects this:
> 
>   999.000...
> 
> but on Solaris 10 gets this:
> 
>   .00
> 
> Yes, the nines are gone, and only this query is failing.  Oddly, this
> query did not fail, though the only difference is fewer decimal digits:
> 
>   SELECT to_char(float8 '999', 'D');
> 
> This smells like a libc bug, e.g. OmniOS 5.11 passed the test.

Use of the "f" conversion specifier with precision greater than 512 is not
portable; I get a similar diff on AIX 7.  Until this patch, PostgreSQL would
not use arbitrarily-large precisions on that conversion specifier.  (Who would
guess, but the "e" conversion specifier is not affected.)

I recommend adding a "configure" test to use our snprintf.c replacements if
sprintf("%.*f", 65536, 999.0) gives unexpected output.


-- 
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] New functions

[Воронин Дмитрий]

>  Please, attach new version of my patch to commitfest page.

Micheal, I found a way to attach patch. sorry to trouble.
-- 
Best regards, Dmitry Voronin


-- 
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] INT64_MIN and _MAX

[Andrew Gierth]

> "Andres" == Andres Freund  writes:

 >> Hm, it looks like the same could be said for INT32_MIN and _MAX;
 >> some places use INT_MIN etc., others say "we shouldn't assume int =
 >> int32" and use (-0x7fff - 1) or whatever instead.

 Andres> I have been annoyed by this multiple times. I think we should
 Andres> make sure the C99 defines are there (providing values if they
 Andres> aren't) and always use those. We've used them in parts of the
 Andres> tree long enough that it's unlikely to cause problems. Nothing
 Andres> is helped by using different things in other parts of the tree.

 Andres> Willing to cook up a patch?

How's this one?

This replaces the one I posted before; it does both INT64_MIN/MAX and
INT32_MIN/MAX, and also int16/int8/uint*. Uses of 0x7fff in code
have been replaced unless there was a reason not to, with either INT_MAX
or INT32_MAX according to the type required.

What I have _not_ done yet is audit uses of INT_MIN/MAX to see which
ones should really be INT32_MIN/MAX.

-- 
Andrew (irc:RhodiumToad)

diff --git a/contrib/btree_gist/btree_ts.c b/contrib/btree_gist/btree_ts.c
index b9c2b49..d472d49 100644
--- a/contrib/btree_gist/btree_ts.c
+++ b/contrib/btree_gist/btree_ts.c
@@ -153,7 +153,7 @@ ts_dist(PG_FUNCTION_ARGS)
 		p->day = INT_MAX;
 		p->month = INT_MAX;
 #ifdef HAVE_INT64_TIMESTAMP
-		p->time = INT64CONST(0x7FFF);
+		p->time = INT64_MAX;
 #else
 		p->time = DBL_MAX;
 #endif
@@ -181,7 +181,7 @@ tstz_dist(PG_FUNCTION_ARGS)
 		p->day = INT_MAX;
 		p->month = INT_MAX;
 #ifdef HAVE_INT64_TIMESTAMP
-		p->time = INT64CONST(0x7FFF);
+		p->time = INT64_MAX;
 #else
 		p->time = DBL_MAX;
 #endif
diff --git a/contrib/intarray/_int_gist.c b/contrib/intarray/_int_gist.c
index 876a7b9..07108eb 100644
--- a/contrib/intarray/_int_gist.c
+++ b/contrib/intarray/_int_gist.c
@@ -3,6 +3,8 @@
  */
 #include "postgres.h"
 
+#include 
+
 #include "access/gist.h"
 #include "access/skey.h"
 
@@ -191,7 +193,7 @@ g_int_compress(PG_FUNCTION_ARGS)
 		cand = 1;
 		while (len > MAXNUMRANGE * 2)
 		{
-			min = 0x7fff;
+			min = INT_MAX;
 			for (i = 2; i < len; i += 2)
 if (min > (dr[i] - dr[i - 1]))
 {
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 706fdf5..822adfd 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -49,10 +49,6 @@
 #include 		/* for getrlimit */
 #endif
 
-#ifndef INT64_MAX
-#define INT64_MAX	INT64CONST(0x7FFF)
-#endif
-
 #ifndef M_PI
 #define M_PI 3.14159265358979323846
 #endif
@@ -453,7 +449,7 @@ strtoint64(const char *str)
 		 */
 		if (strncmp(ptr, "9223372036854775808", 19) == 0)
 		{
-			result = -INT64CONST(0x7fff) - 1;
+			result = INT64_MIN;
 			ptr += 19;
 			goto gotdigits;
 		}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e2d187f..656d55b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1408,7 +1408,7 @@ WALInsertLockAcquireExclusive(void)
 	{
 		LWLockAcquireWithVar(&WALInsertLocks[i].l.lock,
 			 &WALInsertLocks[i].l.insertingAt,
-			 UINT64CONST(0x));
+			 UINT64_MAX);
 	}
 	LWLockAcquireWithVar(&WALInsertLocks[i].l.lock,
 		 &WALInsertLocks[i].l.insertingAt,
diff --git a/src/backend/tsearch/wparser_def.c b/src/backend/tsearch/wparser_def.c
index ca5b543..d95fdb1 100644
--- a/src/backend/tsearch/wparser_def.c
+++ b/src/backend/tsearch/wparser_def.c
@@ -14,6 +14,8 @@
 
 #include "postgres.h"
 
+#include 
+
 #include "catalog/pg_collation.h"
 #include "commands/defrem.h"
 #include "tsearch/ts_locale.h"
@@ -2047,7 +2049,7 @@ hlCover(HeadlineParsedText *prs, TSQuery query, int *p, int *q)
 	int			pos = *p;
 
 	*q = -1;
-	*p = 0x7fff;
+	*p = INT_MAX;
 
 	for (j = 0; j < query->size; j++)
 	{
@@ -2258,7 +2260,7 @@ mark_hl_fragments(HeadlineParsedText *prs, TSQuery query, int highlight,
 	for (f = 0; f < max_fragments; f++)
 	{
 		maxitems = 0;
-		minwords = 0x7fff;
+		minwords = INT32_MAX;
 		minI = -1;
 
 		/*
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 56d909a..b3a1191 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -78,7 +78,7 @@ scanint8(const char *str, bool errorOK, int64 *result)
 		 */
 		if (strncmp(ptr, "9223372036854775808", 19) == 0)
 		{
-			tmp = -INT64CONST(0x7fff) - 1;
+			tmp = INT64_MIN;
 			ptr += 19;
 			goto gotdigits;
 		}
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index d77799a..585da1e 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -190,7 +190,7 @@ pg_lltoa(int64 value, char *a)
 	 * Avoid problems with the most negative integer not being representable
 	 * as a positive integer.
 	 */
-	if (value == (-INT64CONST(0x7FFF) - 1))
+	if (value == INT64_MIN)
 	{
 		memcpy(a, "-9223372036854775808", 21);
 		return;
diff --git a/src/backend/utils/adt/timest

Re: [HACKERS] Remove fsync ON/OFF as a visible option?

[Florian Weimer]

* David G. Johnston:

> "​enables or disables data durability ​promise of ACID." ?

“fsync = on” only works if the storage stack doesn't do funny things.
Depending on the system, it might not be sufficient.


-- 
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] inherit support for foreign tables

[Tom Lane]

Etsuro Fujita  writes:
> [ fdw-inh-8.patch ]

I've committed this with some substantial rearrangements, notably:

* I thought that if we were doing this at all, we should go all the way
and allow foreign tables to be both inheritance parents and children.

* As I mentioned earlier, I got rid of a few unnecessary restrictions on
foreign tables so as to avoid introducing warts into inheritance behavior.
In particular, we now allow NOT VALID CHECK constraints (and hence ALTER
... VALIDATE CONSTRAINT), ALTER SET STORAGE, and ALTER SET WITH/WITHOUT
OIDS.  These are probably no-ops anyway for foreign tables, though
conceivably an FDW might choose to implement some behavior for STORAGE
or OIDs.

* I did not like the EXPLAIN changes at all; in the first place they
resulted in invalid JSON output (there could be multiple fields of the
Update plan object with identical labels), and in the second place it
seemed like a bad idea to rely on FDWs to change the behavior of their
ExplainModifyTarget functions.  I've refactored that so that explain.c
remains responsible for getting the grouping right.  Also, as I said
earlier, it seemed like a good idea to produce subgroups identifying
all the target tables not only the foreign ones.

* I fooled around with the PlanRowMark changes some more, mainly with
the idea that we might soon allow FDWs to use rowmark methods other than
ROW_MARK_COPY.  The planner now has just one place where a rel's rowmark
method is chosen, so as to centralize anything we need to do there.

regards, tom lane


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

Re: [HACKERS] Patch: Add launchd Support

[David E. Wheeler]

On Mar 20, 2015, at 4:11 PM, David E. Wheeler  wrote:

> No one replied. Want a new patch with that?

Here it is.

Best,

David


launchd2.patch
Description: Binary data




smime.p7s
Description: S/MIME cryptographic signature

Re: [HACKERS] inherit support for foreign tables

[Robert Haas]

On Sun, Mar 22, 2015 at 1:57 PM, Tom Lane  wrote:
> Etsuro Fujita  writes:
>> [ fdw-inh-8.patch ]
>
> I've committed this with some substantial rearrangements, notably:

I'm really glad this is going in!  Thanks to to Shigeru Hanada and
Etsuro Fujita for working on this, to you (Tom) for putting in the
time to get it committed, and of course to the reviewers Ashutosh
Bapat and Kyotaro Horiguchi for their time and effort.

In a way, I believe we can think of this as the beginnings of a
sharding story for PostgreSQL.  A lot more work is needed, of course
-- join and aggregate pushdown are high on my personal list -- but
it's a start.

-- 
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: numeric timestamp in log_line_prefix

[Tomas Vondra]

On 22.3.2015 16:58, Fabien COELHO wrote:
> 
> About the feature: I find it is a good thing. It may help scripting over
> the logs, for instance to compute delays between events, whereas the
> full date-time-tz syntax is maybe nice but heavier to work with afterwards.
> 
> In addition to the comments already made (typo in doc, padding...):
> 
> +sprintf(timestamp_str, "%ld.%.03d",
> +tv.tv_sec, (int)(tv.tv_usec / 1000));
> 
> I'm not sure that the "." in "%.03d" is useful. ISTM that it is used for
> floatting point formatting, but is not needed with integers.

It is needed for integers, because you need to make sure 1 millisecond
is formatted as .001 and not .1.



-- 
Tomas Vondrahttp://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] PATCH: numeric timestamp in log_line_prefix

[Tomas Vondra]

On 22.3.2015 08:14, David Rowley wrote:
> Hi Tomas,
> 
> I just had a quick glance at this.
> Is there a reason you didn't include code to support the space padding
> for the new log_line_prefixes?
> The others support  % in the prefix, to allow left or
> right alignment of the item.

Didn't realize that, will fix in the next version.

> 
> Also, what's the reason for timestamp_str? Could you not just use 
> appendStringInfo() and skip the temporary buffer?

Yeah, that's probably a good idea too.

> 
> Regards
> 
> David Rowley 



-- 
Tomas Vondrahttp://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] PATCH: numeric timestamp in log_line_prefix

[Fabien COELHO]

I'm not sure that the "." in "%.03d" is useful. ISTM that it is used for
floatting point formatting, but is not needed with integers.


It is needed for integers, because you need to make sure 1 millisecond
is formatted as .001 and not .1.


ISTM that the "03" does that on its own:

 sh> printf "%03d\n" 0 1 2
 000
 001
 002

--
Fabien.


--
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: numeric timestamp in log_line_prefix

[Andres Freund]

On 2015-03-22 00:47:12 +0100, Tomas Vondra wrote:
> from time to time I need to correlate PostgreSQL logs to other logs,
> containing numeric timestamps - a prime example of that is pgbench. With
> %t and %m that's not quite trivial, because of timezones etc.

I have a hard time seing this is sufficient cause for adding more format
codes. They're not free runtime and documentation wise. -0.5 from me.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Remove fsync ON/OFF as a visible option?

[David G. Johnston]

On Sunday, March 22, 2015, Florian Weimer  wrote:

> * David G. Johnston:
>
> > "​enables or disables data durability ​promise of ACID." ?
>
> “fsync = on” only works if the storage stack doesn't do funny things.
> Depending on the system, it might not be sufficient.
>

Allows for (underlying storage not withstanding) or disables, then.

But that distinction is present no matter what so from the standpoint the
alternative is no worse and at least tells the user that a key promise of
RDBMS is being voluntarily waived if they disable this setting.

Given the existence of developer settings I would add this to that list.
People wanting specialized configurations where this would be disabled will
learn about it elsewhere, confirm its existence in the docs, and then add
it to their custom configuration.  Those who do not learn elsewhere
probably shouldn't be changing it in the first place.

David J.

Re: [HACKERS] PATCH: numeric timestamp in log_line_prefix

[Fabien COELHO]

On 2015-03-22 00:47:12 +0100, Tomas Vondra wrote:

from time to time I need to correlate PostgreSQL logs to other logs,
containing numeric timestamps - a prime example of that is pgbench. With
%t and %m that's not quite trivial, because of timezones etc.


I have a hard time seing this is sufficient cause for adding more format
codes. They're not free runtime and documentation wise. -0.5 from me.


The proposed format is much simpler to manage in a script, and if you're 
interested in runtime, its formatting would be less expensive than %t and 
%m.


--
Fabien.


--
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: numeric timestamp in log_line_prefix

[Tom Lane]

Fabien COELHO  writes:
>> On 2015-03-22 00:47:12 +0100, Tomas Vondra wrote:
>>> from time to time I need to correlate PostgreSQL logs to other logs,
>>> containing numeric timestamps - a prime example of that is pgbench. With
>>> %t and %m that's not quite trivial, because of timezones etc.

>> I have a hard time seing this is sufficient cause for adding more format
>> codes. They're not free runtime and documentation wise. -0.5 from me.

> The proposed format is much simpler to manage in a script, and if you're 
> interested in runtime, its formatting would be less expensive than %t and 
> %m.

Maybe, but do we really need two?  How about just %M?

Also, having just one would open the door to calling it something like
%u (for Unix timestamp), which would avoid introducing the concept of
upper case meaning something-different-from-but-related-to into
log_line_prefix format codes.  We don't have any upper case codes in
there now, and I'd prefer not to go there if we don't have to.

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: numeric timestamp in log_line_prefix

[Jeff Janes]

On Sat, Mar 21, 2015 at 4:47 PM, Tomas Vondra 
wrote:

> Hi,
>
> from time to time I need to correlate PostgreSQL logs to other logs,
> containing numeric timestamps - a prime example of that is pgbench. With
> %t and %m that's not quite trivial, because of timezones etc.
>
> I propose adding two new log_line_prefix escape sequences - %T and %M,
> doing the same thing as %t and %m, but formatting the value as a number.
>
> Patch attached, I'll add it to CF 2015-06.
>

I've wanted this before as well.  But what is the point of %T?  Does
printing the milliseconds cause
some kind of detectable performance hit?

I don't think I've ever thought myself "You know, I really wish I hadn't
included the milliseconds in that timestamp".

Same question for %t, but that ship has already sailed.

Cheers,

Jeff

Re: [HACKERS] PATCH: numeric timestamp in log_line_prefix

[Fabien COELHO]

[oops, stalled because of wrong From, resending just to the list]

On Sun, 22 Mar 2015, Tom Lane wrote:


The proposed format is much simpler to manage in a script, and if you're
interested in runtime, its formatting would be less expensive than %t and
%m.


Maybe, but do we really need two?  How about just %M?


Yep, truncating or rounding if needed is quite easy.


Also, having just one would open the door to calling it something like
%u (for Unix timestamp),


Should be ok as well.

--
Fabien.


--
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: numeric timestamp in log_line_prefix

[Tomas Vondra]

On 22.3.2015 19:45, Fabien COELHO wrote:
> 
>>> I'm not sure that the "." in "%.03d" is useful. ISTM that it is used for
>>> floatting point formatting, but is not needed with integers.
>>
>> It is needed for integers, because you need to make sure 1 millisecond
>> is formatted as .001 and not .1.
> 
> ISTM that the "03" does that on its own:
> 
>  sh> printf "%03d\n" 0 1 2
>  000
>  001
>  002

Oh, right - one dot too many. Thanks, will fix that.


-- 
Tomas Vondrahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] Remove fsync ON/OFF as a visible option?

[Josh Berkus]

On 03/22/2015 06:45 AM, Andres Freund wrote:
> On 2015-03-21 13:53:47 -0700, Josh Berkus wrote:
>> Coincidentally, I am just at this moment performance testing "running
>> with scissors mode" for PostgreSQL on AWS.   When intentional, this mode
>> is useful for spinning up lots of read-only replicas which are intended
>> mainly as cache support, something I've done at various dot-coms.
> 
> Which is where fsync=off barely has any effect?

Well, I'll admit that I'm not testing each setting independantly.   I
have enough tests to run already.

>> So, -1 on removing the setting; it is useful to some users.
> 
> Agreed on that.
> 
>> Further, full_page_writes=off is supposedly safe on any copy-on-write
>> filesystem, such as ZFS.
> 
> FWIW, I think that's a myth. One I heard various versions of by now. As
> long as the OSs page size (4kb nearly everywhere) is different from
> postgres' (8kb) you can have torn pages. Even if individual filesystem
> page writes are atomic.

ZFS's block size is larger than Linux's memory page size.  That is, ZFS
on Linux uses a 8kB to 128kB block size depending on which blocks you're
looking at and how you have it configured.  Does that make a difference
at all, given that Linux's memory page size is still 4kB?

FYI, the BTRFS folks are also claiming to be torn-page-proof, so it
would be really nice to settle this.  Not sure how to force the issue
through testing though.

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


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

Re: [HACKERS] PATCH: numeric timestamp in log_line_prefix

[Tomas Vondra]

On 22.3.2015 20:25, Fabien COELHO wrote:
> 
>>> The proposed format is much simpler to manage in a script, and if you're
>>> interested in runtime, its formatting would be less expensive than %t
>>> and
>>> %m.
>>
>> Maybe, but do we really need two?  How about just %M?
> 
> I guess Tomas put 2 formats because there was 2 time formats to
> begin with, but truncating/rouding if someone really wants seconds is
> quite easy.

Yes, that's why I added two - to reflect %t and %m. I'm OK with using
just one of them - I don't really care for the milliseconds at this
moment, but I'd probably choose that option.

>> Also, having just one would open the door to calling it something
>> like %u (for Unix timestamp),
> 
> I guess that is okay as well.

Whatever, I don't really care. It's slightly confusing because unix
timestams are usually integers, but IMHO that's minor difference.


-- 
Tomas Vondrahttp://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] Abbreviated keys for Numeric

[Andrew Gierth]

So here's a version 3 patch:

1. #ifdefism is reduced to a minimum by (a) desupporting values of NBASE
other than 1, and (b) keeping the 32bit code, so that there is now
no longer any question of abbreviation not being used (it always is).
So the only #ifs in the code body (rather than declarations) are to
select which implementation of numeric_abbrev_convert_var to use.

2. Some (but not all) stylistic changes have been made following Peter's
version.

3. Buffer management has been simplified by simply allocating the
maximum needed size upfront (since it's only needed for short varlenas).
The truncation behavior has been removed.

4. Updated oid choice etc.

The substance of the code is unchanged from my original patch.  I didn't
add diagnostic output to numeric_abbrev_abort, see my separate post
about the suggestion of a GUC for that.

-- 
Andrew (irc:RhodiumToad)

diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index ff9bfcc..a27de6b 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -29,6 +29,7 @@
 #include "access/hash.h"
 #include "catalog/pg_type.h"
 #include "funcapi.h"
+#include "lib/hyperloglog.h"
 #include "libpq/pqformat.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
@@ -36,6 +37,21 @@
 #include "utils/builtins.h"
 #include "utils/int8.h"
 #include "utils/numeric.h"
+#include "utils/sortsupport.h"
+
+/* XXX these should be in c.h - remove when that patch goes in */
+#ifndef INT64_MIN
+#define INT64_MIN	(-INT64CONST(0x7FFF) - 1)
+#endif
+#ifndef INT64_MAX
+#define INT64_MAX	INT64CONST(0x7FFF)
+#endif
+#ifndef INT32_MIN
+#define INT32_MIN	(-0x7FFF - 1)
+#endif
+#ifndef INT32_MAX
+#define INT32_MAX	(0x7FFF)
+#endif
 
 /* --
  * Uncomment the following to enable compilation of dump_numeric()
@@ -57,6 +73,12 @@
  * are easy.  Also, it's actually more efficient if NBASE is rather less than
  * sqrt(INT_MAX), so that there is "headroom" for mul_var and div_var_fast to
  * postpone processing carries.
+ *
+ * Values of NBASE other than 1 are considered of historical interest only
+ * and are no longer supported in any sense; no mechanism exists for the client
+ * to discover the base, so every client supporting binary mode expects the
+ * base-1 format.  If you plan to change this, also note the numeric
+ * abbreviation code, which assumes NBASE=1.
  * --
  */
 
@@ -90,6 +112,10 @@ typedef signed char NumericDigit;
 typedef int16 NumericDigit;
 #endif
 
+#if DEC_DIGITS != 4
+#error "Numeric bases other than 1 are no longer supported"
+#endif
+
 /*
  * The Numeric type as stored on disk.
  *
@@ -275,6 +301,30 @@ typedef struct
 
 
 /* --
+ * Sort support.
+ * --
+ */
+typedef struct
+{
+	void			   *buf;			/* buffer for short varlenas */
+	int64input_count;	/* number of non-null values seen */
+	boolestimating;		/* true if estimating cardinality */
+
+	hyperLogLogState	abbr_card;		/* cardinality estimator */
+} NumericSortSupport;
+
+#ifdef USE_FLOAT8_BYVAL
+#define NUMERIC_ABBREV_BITS 64
+#define DatumGetNumericAbbrev(d) DatumGetInt64(d)
+#define NUMERIC_ABBREV_NAN Int64GetDatum(INT64_MIN)
+#else
+#define NUMERIC_ABBREV_BITS 32
+#define DatumGetNumericAbbrev(d) DatumGetInt32(d)
+#define NUMERIC_ABBREV_NAN Int32GetDatum(INT32_MIN)
+#endif
+
+
+/* --
  * Some preinitialized constants
  * --
  */
@@ -409,6 +459,13 @@ static void int128_to_numericvar(int128 val, NumericVar *var);
 static double numeric_to_double_no_overflow(Numeric num);
 static double numericvar_to_double_no_overflow(NumericVar *var);
 
+static Datum numeric_abbrev_convert(Datum original_datum, SortSupport ssup);
+static bool  numeric_abbrev_abort(int memtupcount, SortSupport ssup);
+static int   numeric_fast_cmp(Datum x, Datum y, SortSupport ssup);
+static int   numeric_cmp_abbrev(Datum x, Datum y, SortSupport ssup);
+
+static Datum numeric_abbrev_convert_var(NumericVar *var, NumericSortSupport *nss);
+
 static int	cmp_numerics(Numeric num1, Numeric num2);
 static int	cmp_var(NumericVar *var1, NumericVar *var2);
 static int cmp_var_common(const NumericDigit *var1digits, int var1ndigits,
@@ -1507,9 +1564,393 @@ compute_bucket(Numeric operand, Numeric bound1, Numeric bound2,
  * Note: btree indexes need these routines not to leak memory; therefore,
  * be careful to free working copies of toasted datums.  Most places don't
  * need to be so careful.
+ *
+ * Sort support:
+ *
+ * We implement the sortsupport strategy routine in order to get the benefit of
+ * abbreviation. The ordinary numeric comparison can be quite slow as a result
+ * of palloc/pfree cycles (due to detoasting packed values for alignment);
+ * while this could be worked on itself, the abbreviation strategy gives more
+ * speedup in many common cases.
+ *
+ * Two different representations are used for the abbreviated form, one in
+ * int32 and one in int64, whichever fits into a by-value Datum

Re: [HACKERS] Zero-padding and zero-masking fixes for to_char(float)

[Noah Misch]

When you posted this, I made a note to review it.

On Wed, Mar 18, 2015 at 05:52:44PM -0400, Bruce Momjian wrote:
> This "junk" digit zeroing matches the Oracle behavior:
> 
>   SELECT to_char(1.123456789123456789123456789d, 
> '9.9') as x from dual;
>   --
>   1.12345678912345680
> 
> Our output with the patch would be:
> 
>   SELECT to_char(float8 '1.123456789123456789123456789', 
> '9.9');
>   --
>   1.12345678912345000
> 
> which is pretty close.

PostgreSQL 9.4 returns "1.12345678912346".  Your patch truncates digits past
DBL_DIG, whereas both Oracle and PostgreSQL 9.4 round to the nearest DBL_DIG
digits.  PostgreSQL must continue to round.

These outputs show Oracle treating 17 digits as significant while PostgreSQL
treats 15 digits as significant.  Should we match Oracle in this respect while
we're breaking compatibility anyway?  I tend to think yes.

> *** int4_to_char(PG_FUNCTION_ARGS)
> *** 5214,5221 
>   /* we can do it easily because float8 won't lose any precision 
> */
>   float8  val = (float8) value;
>   
> ! orgnum = (char *) palloc(MAXDOUBLEWIDTH + 1);
> ! snprintf(orgnum, MAXDOUBLEWIDTH + 1, "%+.*e", Num.post, val);
>   
>   /*
>* Swap a leading positive sign for a space.
> --- 5207,5213 
>   /* we can do it easily because float8 won't lose any precision 
> */
>   float8  val = (float8) value;
>   
> ! orgnum = psprintf("%+.*e", Num.post, val);

Neither the submission notes nor the commit messages mentioned it, but this
improves behavior for to_char(int4, text).  Specifically, it helps 
formats with more than about 500 decimal digits:

  SELECT length(to_char(1, '9D' || repeat('9', 800) || ''));


-- 
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] Using 128-bit integers for sum, avg and statistics aggregates

[Petr Jelinek]

On 22/03/15 13:59, Andreas Karlsson wrote:

On 03/22/2015 11:47 AM, Petr Jelinek wrote:

On 22/03/15 10:35, Andres Freund wrote:

http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2015-03-21%2003%3A01%3A21




That's the stuff looking like random memory that I talk about above...



If you look at it closely, it's actually not random memory. At least in
the first 2 failing tests which are not obfuscated by aggregates on top
of aggregates. It looks like first NumericDigit is ok and the second one
is corrupted (there are only 2 NumericDigits in those numbers). Of
course the conversion to Numeric is done from the end so it looks like
only the last computation/pointer change/something stays ok while the
rest got corrupted.


Would this mean the bug is most likely somewhere in
int128_to_numericvar()? Maybe that version of gcc has a bug in some
__int128 operator or I messed up the code there somehow.



Yeah that's what I was thinking also, and I went through the function 
and didn't find anything suspicious (besides it's same as the 64 bit 
version except for the int128 use).


It really might be some combination of arithmetic + the conversion to 
16bit uint bug in the compiler. Would be worth to try to produce test 
case and try it standalone maybe?


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


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

Re: [HACKERS] Remove fsync ON/OFF as a visible option?

[Florian Weimer]

* David G. Johnston:

> On Sunday, March 22, 2015, Florian Weimer  wrote:
>
>> * David G. Johnston:
>>
>> > "​enables or disables data durability ​promise of ACID." ?
>>
>> “fsync = on” only works if the storage stack doesn't do funny things.
>> Depending on the system, it might not be sufficient.
>>
>
> Allows for (underlying storage not withstanding) or disables, then.

Maybe.

> But that distinction is present no matter what so from the standpoint the
> alternative is no worse and at least tells the user that a key promise of
> RDBMS is being voluntarily waived if they disable this setting.

I don't think this will matter in the end.  The existing
postgresql.conf template does not suggest at all that fsync=off makes
things go substantially faster.  Administrators obviously get the idea
from somewhere else, and they will continue to follow that advice no
matter what the configuration template says.


-- 
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] Using 128-bit integers for sum, avg and statistics aggregates

[Andres Freund]

On 2015-03-22 22:00:13 +0100, Petr Jelinek wrote:
> On 22/03/15 13:59, Andreas Karlsson wrote:
> >Would this mean the bug is most likely somewhere in
> >int128_to_numericvar()? Maybe that version of gcc has a bug in some
> >__int128 operator or I messed up the code there somehow.

Yes, or a compiler bug. I looked through the code again and found and
fixed one minor bug, but that doesnt' explain the problem.

> Yeah that's what I was thinking also, and I went through the function and
> didn't find anything suspicious (besides it's same as the 64 bit version
> except for the int128 use).
> 
> It really might be some combination of arithmetic + the conversion to 16bit
> uint bug in the compiler. Would be worth to try to produce test case and try
> it standalone maybe?

A compiler bug looks like a not unreasonable bet at this point. I've
asked Andrew to recompile without optimizations... We'll see whether
that makes a difference. Jacana is the only compiler with gcc 4.8.1 (or
is it 4.8.0? there's conflicting output). There've been a number of bugs
fixed that affect loop unrolling and such.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates

[Andreas Karlsson]

On 03/22/2015 10:20 PM, Andres Freund wrote:

Yes, or a compiler bug. I looked through the code again and found and
fixed one minor bug, but that doesnt' explain the problem.


Strangely enough the bug looks like it has been fixed at jacana after 
your fix of my copypasto. Maybe the bug is random, or your fix really 
fixed it.


http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2015-03-22%2020%3A41%3A54

Andreas


--
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] Using 128-bit integers for sum, avg and statistics aggregates

[Andres Freund]

On 2015-03-22 22:20:49 +0100, Andres Freund wrote:
> A compiler bug looks like a not unreasonable bet at this point. I've
> asked Andrew to recompile without optimizations... We'll see whether
> that makes a difference. Jacana is the only compiler with gcc 4.8.1 (or
> is it 4.8.0? there's conflicting output). There've been a number of bugs
> fixed that affect loop unrolling and such.

And indeed, a run of jacana with -O0 fixed it. As you can see in
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2015-03-22%2020%3A41%3A54
, it still fails, but just because of the exp problem.

My guess it's
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2015-03-22%2020%3A41%3A54

Do we feel the need to find a workaround if this if it's indeed 4.8.0
and 4.8.1 (and some other releases at the same time) that are
problematic? Given that gcc 4.8.2 was released October 16, 2013 I'm
inclined not to.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates

[Andres Freund]

On 2015-03-22 22:28:01 +0100, Andreas Karlsson wrote:
> On 03/22/2015 10:20 PM, Andres Freund wrote:
> >Yes, or a compiler bug. I looked through the code again and found and
> >fixed one minor bug, but that doesnt' explain the problem.
> 
> Strangely enough the bug looks like it has been fixed at jacana after your
> fix of my copypasto. Maybe the bug is random, or your fix really fixed it.

I bet it's actually the -O0 Andrew added at my behest.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Remove fsync ON/OFF as a visible option?

[Greg Stark]

On Sun, Mar 22, 2015 at 3:34 PM, Euler Taveira  wrote:
> On 21-03-2015 17:53, Josh Berkus wrote:
>> Now, I have *long* been an advocate that we should ship a "stripped"
>> PostgreSQL.conf which has only the most commonly used settings, and
>> leave the rest of the settings in the docs and
>> share/postgresql/postgresql.conf.advanced.  Here's my example of such a
>> file, tailored to PostgreSQL 9.3:
>>
> +1. I agree that common used settings in a postgresql.conf file is
> useful for newbies. How do we ship it?


Fwiw I disagree. I'm a fan of the idea that the default should be an
empty config file. You should only have to put things in
postgresql.conf if you want something unusual or specific. We're a
long way from there but I would rather move towards there than keep
operating under the assumption that nobody will run Postgres without
first completing the rite of passage of reviewing every option in
postgresql.conf to see if it's relevant to their setup.

Apache used to ship with a config full of commented out options and
going through and figuring out which options needed to be enabled or
changed was the first step in installing Apache. It was awful. When
they adopted a strict policy that the default config was empty so the
only things you need in your config are options to specify what you
want to serve it became so much easier. I would argue it also
represents a more professional attitude where the job of the admin is
to declare only what he wants to happen and how it should differ from
the norm and the job of the software is to go about its business
without needing to be set up for normal uses.

-- 
greg


-- 
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] INT64_MIN and _MAX

[Peter Geoghegan]

On Sun, Mar 22, 2015 at 2:26 AM, Andres Freund  wrote:
> I have been annoyed by this multiple times. I think we should make sure the 
> C99 defines are there (providing values if they aren't) and always use those. 
> We've used them in parts of the tree long enough that it's unlikely to cause 
> problems. Nothing is helped by using different things in other parts of the 
> tree.


+1

-- 
Peter Geoghegan


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

[HACKERS] barnacle (running CLOBBER_CACHE_RECURSIVELY) seems stuck since November

[Tomas Vondra]

Hi,

I've been checking progress on barnacle, one of the animals running with
CLOBBER_CACHE_RECURSIVELY. It's running for ~170 days (250k minutes).

This time I've however checked the log, and what caught my eye is that
the last log message is from November. There were regular messages until
then (a few messages per day), but since Nov 19 there's nothing.

The last few messages are these:

2014-11-28 22:19:24 CET 7953 [540097d4.1f11:532] LOG:  statement: CREATE
FUNCTION city_insert() RETURNS trigger LANGUAGE plpgsql AS $$
2014-11-28 22:19:27 CET 7953 [540097d4.1f11:533] LOG:  statement: CREATE
TRIGGER city_insert_trig INSTEAD OF INSERT ON city_view
2014-11-28 22:25:13 CET 7953 [540097d4.1f11:534] LOG:  statement: CREATE
FUNCTION city_delete() RETURNS trigger LANGUAGE plpgsql AS $$
2014-11-28 22:25:15 CET 7953 [540097d4.1f11:535] LOG:  statement: CREATE
TRIGGER city_delete_trig INSTEAD OF DELETE ON city_view
2014-11-29 03:10:01 CET 7953 [540097d4.1f11:536] LOG:  statement: CREATE
FUNCTION city_update() RETURNS trigger LANGUAGE plpgsql AS $$
2014-11-29 03:10:03 CET 7953 [540097d4.1f11:537] LOG:  statement: CREATE
TRIGGER city_update_trig INSTEAD OF UPDATE ON city_view
2014-11-29 07:44:50 CET 7953 [540097d4.1f11:538] LOG:  statement: INSERT
INTO city_view(city_name) VALUES('Tokyo') RETURNING *;

which matches commands halfway-through 'triggers' tests.

There are currently two queries running - one from 'triggers', one from
'updatable_views':

-[ RECORD 1 ]+--
 datid| 16384
 datname  | regression
 pid  | 7953
 usesysid | 10
 usename  | pgbuild
 application_name | pg_regress
 client_addr  |
 client_hostname  |
 client_port  | -1
 backend_start| 2014-08-29 17:10:12.243979+02
 xact_start   | 2014-11-29 07:44:50.452678+01
 query_start  | 2014-11-29 07:44:50.452678+01
 state_change | 2014-11-29 07:44:50.45268+01
 waiting  | f
 state| active
 backend_xid  |
 backend_xmin | 3989
 query| INSERT INTO city_view(city_name) VALUES('Tokyo')
RETURNING *;
 -[ RECORD 2 ]+--
 datid| 16384
 datname  | regression
 pid  | 7956
 usesysid | 10
 usename  | pgbuild
 application_name | pg_regress
 client_addr  |
 client_hostname  |
 client_port  | -1
 backend_start| 2014-08-29 17:10:12.272223+02
 xact_start   | 2014-10-06 15:35:25.822488+02
 query_start  | 2014-10-06 15:35:25.822488+02
 state_change | 2014-10-06 15:35:25.822491+02
 waiting  | f
 state| active
 backend_xid  |
 backend_xmin | 3862
 query| UPDATE rw_view2 SET b='Row three' WHERE a=3 RETURNING *;

Top currently looks like this:

  PID USER  PR  NI  VIRT  RES  SHR S %CPU %MEMTIME+  COMMAND
 7956 pgbuild   20   0  242m  13m  10m R 98.8  0.2 251152:27 postgres:
pgbuild regression [local] UPDATE
 7953 pgbuild   20   0 1023m 785m  11m R 98.4 10.0 248806:18 postgres:
pgbuild regression [local] INSERT

Both backends started on August 29, but the 'updatable_views' query is
running since October 6. 5 months seems like a rather long runtime, even
with CLOBBER_CACHE_RECURSIVELY).

Not sure if it's relevant, but this is the machine with Intel C compiler
(2013).

Attached is a memory context info for the 7953 backend (with more than
1GB VIRT) - not sure if it's relevant, so just in case.


-- 
Tomas Vondrahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
TopMemoryContext: 160096 total in 21 blocks; 11208 free (32 chunks); 14 used
  PL/pgSQL function context: 122880 total in 4 blocks; 100064 free (121 chunks); 22816 used
  TopTransactionContext: 8192 total in 1 blocks; 5920 free (19 chunks); 2272 used
ExecutorState: 8192 total in 1 blocks; 7288 free (0 chunks); 904 used
  ExprContext: 8192 total in 1 blocks; 8160 free (0 chunks); 32 used
SPI Exec: 65536 total in 4 blocks; 52904 free (271 chunks); 12632 used
  ExecutorState: 57344 total in 3 blocks; 46704 free (343 chunks); 10640 used
ExprContext: 0 total in 0 blocks; 0 free (0 chunks); 0 used
ExprContext: 65704 total in 4 blocks; 39744 free (113 chunks); 25960 used
SPI Proc: 57344 total in 3 blocks; 38752 free (206 chunks); 18592 used
  SPI TupTable: 8192 total in 1 blocks; 6744 free (0 chunks); 1448 used
  PL/pgSQL function context: 57344 total in 3 blocks; 34912 free (5 chunks); 22432 used
  PL/pgSQL function context: 24576 total in 2 blocks; 14784 free (89 chunks); 9792 used
  PL/pgSQL function context: 57344 total in 3 blocks; 34656 free (12 chunks); 22688 used
  PL/pgSQL function context: 57344 total in 3 blocks; 31464 free (21 chunks); 25880 used
  PL/pgSQL function context: 57344 total in 3 blocks; 45504 free (160 chunks); 11840 used
  PL/pgSQL function context: 24576 total in 2 blocks; 12768 free (10

Re: [HACKERS] Abbreviated keys for Numeric

[Peter Geoghegan]

On Sat, Mar 21, 2015 at 3:33 PM, Andrew Gierth
 wrote:
> Was there some reason why you added #include "utils/memutils.h"?
> Because I don't see anything in your patch that actually needs it.

MaxAllocSize is defiined there.

-- 
Peter Geoghegan


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

Re: [HACKERS] debug_sortsupport GUC?

[Peter Geoghegan]

On Sat, Mar 21, 2015 at 8:28 PM, Andrew Gierth
 wrote:
> So if these debugging elogs are to be kept at all, I propose that rather
> than being compile-time options they should be controlled by a
> debug_sortsupport GUC. Opinions?

This seems like a reasonable idea. Why wouldn't it just be under the
existing trace_sort GUC, though? That's been enabled by default since
8.1. It's already defined in pg_config_manual.h.

-- 
Peter Geoghegan


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

Re: [HACKERS] Remove fsync ON/OFF as a visible option?

[Mark Kirkwood]

On 22/03/15 08:14, Jaime Casanova wrote:


El mar 21, 2015 2:00 AM, "Mark Kirkwood" mailto:mark.kirkw...@catalyst.net.nz>> escribió:
 >
 > On 21/03/15 19:28, Jaime Casanova wrote:
 >>
 >> what about not removing it but not showing it in postgresql.conf? as a
 >> side note, i wonder why trace_sort is not in postgresql.conf...
 >> other option is to make it a compile setting, that why if you want to
 >> have it you need to compile and postgres' developers do that routinely
 >> anyway
 >>
 >
 > -1
 >
 > Personally I'm against hiding *any* settings. Choosing sensible
defaults - yes! Hiding them - that reeks of secret squirrel nonsense and
overpaid Oracle dbas that knew the undocumented settings for various
capabilities. I think/hope that no open source project will try to
emulate that meme!
 >

That ship has already sailed.

http://www.postgresql.org/docs/9.4/static/runtime-config-developer.html



Not really - they are documented in the official doc repo (that was the 
point I was making I think), and +1 for adding or improving the 
documentation for some of the more dangerous ones!


While I'm against removing or hiding settings, I have no problem with 
shipping/generating a postgresql.conf that has *only* the non default 
settings therein, as that requires people to look at the docs where (of 
course) we have some sensible discussion about how to set the rest of 'em.


I note that Mysql ship a pretty minimal confile files there days (5.5, 
5.6) on Ubuntu, and that seems to cause no particular problem.


regards

Mark


--
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] Abbreviated keys for Numeric

[Peter Geoghegan]

On Fri, Mar 20, 2015 at 11:58 PM, Andrew Gierth
 wrote:
> Comparisons between nulls and nulls, or between nulls and non-nulls, are
> cheap; only comparisons between non-nulls and non-nulls can be
> expensive.
>
> The purpose of abbreviation is to replace expensive comparisons by cheap
> ones where possible, and therefore the cost model used for abbreviation
> should ignore nulls entirely; all that matters is the number of non-null
> values and the probability of saving time by abbreviating them.

I don't think it's that simple. The actual point of abbreviation is to
amortize the total cost of performing O(n log n) comparisons (the
average case, which is usually assumed by infrastructure like
sortsupport), by performing an encoding process n times. That encoding
process may be more expensive than each of the comparisons. Sometimes
significantly more expensive.

Forget about abbreviation entirely for a minute - consider plain old
sorting of strxfrm() blobs, which for example the glibc documentation
recommends (with some caveats) as the preferred approach to sorting
strings while respecting collation. Suppose that your applications
happens to frequently encounter fully sorted arrays of strings when
passed an array of strings to sort. If you were using our qsort()
implementation, which, rather questionably, has a pre-check for fully
sorted input (that throws away work when the pre-check fails), then
you'll only have n comparisons. Even if an encoding is only as
expensive as an actual comparison, the potential to lose with encoding
clearly exists. Similarly, we don't abbreviate for top-n heap sorts,
even though all of those abbreviated keys may be used for at least one
comparison.

Now, I hear what you're saying about weighing the costs and the
benefits -- you point out that abbreviation of NULL values is not a
cost, which is certainly true. But if the total number of NULL values
is so high that they dominate, then abbreviation might well be the
wrong thing for that reason alone. In general, string transformation
is not recommended for sorting very small lists of strings.

Where I think your argument is stronger is around the cost of actually
aborting in particular (even though not much work is thrown away).
Scanning through the memtuples array once more certainly isn't free.
And you could take the view that it's always worth the risk, since
it's at most a marginal cost saved if the first ~10k tuples are
representative, but a large cost incurred when they are not. So on
reflection, I am inclined to put the check for non-null values back
in. I also concede that the cost of abbreviation is lower with your
numeric encoding scheme than it is for that of the text opclass, which
favors this approach.

-- 
Peter Geoghegan


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

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

[Kouhei Kaigai]

> On Wed, Mar 18, 2015 at 9:33 AM, Kouhei Kaigai  wrote:
> >> On Wed, Mar 18, 2015 at 2:34 AM, Kouhei Kaigai  
> >> wrote:
> >> > So, overall consensus for the FDW hook location is just before the 
> >> > set_chepest()
> >> > at standard_join_search() and merge_clump(), isn't it?
> >>
> >> Yes, I think so.
> >>
> >> > Let me make a design of FDW hook to reduce code duplications for each 
> >> > FDW driver,
> >> > especially, to identify baserel/joinrel to be involved in this join.
> >>
> >> Great, thanks!
> >>
> >> One issue, which I think Ashutosh alluded to upthread, is that we need
> >> to make sure it's not unreasonably difficult for foreign data wrappers
> >> to construct the FROM clause of an SQL query to be pushed down to the
> >> remote side.  It should be simple when there are only inner joins
> >> involved, but when there are all outer joins it might be a bit
> >> complex.  It would be very good if someone could try to write that
> >> code, based on the new hook locations, and see how it turns out, so
> >> that we can figure out how to address any issues that may crop up
> >> there.
> >>
> > Here is an idea that provides a common utility function that break down
> > the supplied RelOptInfo of joinrel into a pair of join-type and a list of
> > baserel/joinrel being involved in the relations join. It intends to be
> > called by FDW driver to list up underlying relations.
> > IIUC, root->join_info_list will provide information of how relations are
> > combined to the upper joined relations, thus, I expect it is not
> > unreasonably complicated way to solve.
> > Once a RelOptInfo of the target joinrel is broken down into multiple sub-
> > relations (N>=2 if all inner join, elsewhere N=2), FDW driver can
> > reference the RestrictInfo to be used in relations join.
> >
> > Anyway, I'll try to investigate the existing code for more detail today,
> > to clarify whether the above approach is feasible.
> 
> Sounds good.  Keep in mind that, while the parse tree will obviously
> reflect the way that the user actually specified the join
> syntactically, it's not the job of the join_info_list to make it
> simple to reconstruct that information.  To the contrary,
> join_info_list is supposed to be structured in a way that makes it
> easy to determine whether *a particular join order is one of the legal
> join orders* not *whether it is the specific join order selected by
> the user*.  See join_is_legal().
> 
> For FDW pushdown, I think it's sufficient to be able to identify *any
> one* legal join order, not necessarily the same order the user
> originally entered.  For exampe, if the user entered A LEFT JOIN B ON
> A.x = B.x LEFT JOIN C ON A.y = C.y and the FDW generates a query that
> instead does A LEFT JOIN C ON a.y = C.y LEFT JOIN B ON A.x = B.x, I
> suspect that's just fine.  Particular FDWs might wish to try to be
> smart about what they emit based on knowledge of what the remote
> side's optimizer is likely to do, and that's fine.  If the remote side
> is PostgreSQL, it shouldn't matter much.
>
Sorry for my response late. It was not easy to code during business trip.

The attached patch adds a hook for FDW/CSP to replace entire join-subtree
by a foreign/custom-scan, according to the discussion upthread.

GetForeignJoinPaths handler of FDW is simplified as follows:
  typedef void (*GetForeignJoinPaths_function) (PlannerInfo *root,
RelOptInfo *joinrel);

It takes PlannerInfo and RelOptInfo of the join-relation to be replaced
if available. RelOptInfo contains 'relids' bitmap, so FDW driver will be
able to know the relations to be involved and construct a remote join query.
However, it is not obvious with RelOptInfo to know how relations are joined.

The function below will help implement FDW driver that support remote join.

  List *
  get_joinrel_broken_down(PlannerInfo *root, RelOptInfo *joinrel,
  SpecialJoinInfo **p_sjinfo)

It returns a list of RelOptInfo to be involved in the relations join that
is represented with 'joinrel', and also set a SpecialJoinInfo on the third
argument if not inner join.
In case of inner join, it returns multiple (more than or equal to 2)
relations to be inner-joined. Elsewhere, it returns two relations and
a valid SpecialJoinInfo.

The #if 0 ... #endif block is just for confirmation purpose to show
how hook is invoked and the joinrel is broken down with above service
routine.

postgres=# select * from t0 left join t1 on t1.aid=bid
left join t2 on t1.aid=cid
left join t3 on t1.aid=did
left join t4 on t1.aid=eid;
INFO:  LEFT JOIN: t0, t1
INFO:  LEFT JOIN: (t0, t1), t2
INFO:  LEFT JOIN: (t0, t1), t3
INFO:  LEFT JOIN: (t0, t1), t4
INFO:  LEFT JOIN: (t0, t1, t3), t2
INFO:  LEFT JOIN: (t0, t1, t4), t2
INFO:  LEFT JOIN: (t0, t1, t4), t3
INFO:  LEFT JOIN: (t0, t1, t3, t4), t2

In this case, joinrel is broken down into (t0, t1, t3

Re: [HACKERS] Parallel Seq Scan

[Amit Langote]

On 20-03-2015 PM 09:06, Amit Kapila wrote:
> On Mon, Mar 16, 2015 at 12:58 PM, Amit Langote <
> langote_amit...@lab.ntt.co.jp> wrote:
>> Actually I meant "currently the last" or:
>>
>> funnel->nextqueue == funnel->nqueue - 1
>>
>> So the code you quote would only take care of subset of the cases.
>>
> 
> Fixed this issue by resetting funnel->next queue to zero (as per offlist
> discussion with Robert), so that it restarts from first queue in such
> a case.
> 
>>
>>
>> How about shm_mq_detach() called from ParallelQueryMain() right after
>> exec_parallel_stmt() returns? Doesn't that do the SetLatch() that needs
> to be
>> done by a worker?
>>
> 
> Fixed this issue by not going for Wait incase of detached queues.
> 

Thanks for fixing. I no longer see the problems.

Regards,
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] Abbreviated keys for Numeric

[Andrew Gierth]

> "Peter" == Peter Geoghegan  writes:

 Peter> I don't think it's that simple. The actual point of abbreviation
 Peter> is to amortize the total cost of performing O(n log n)
 Peter> comparisons (the average case, which is usually assumed by
 Peter> infrastructure like sortsupport), by performing an encoding
 Peter> process n times. That encoding process may be more expensive
 Peter> than each of the comparisons. Sometimes significantly more
 Peter> expensive.

But the cost of the encoding depends only on the number of non-null
values. Again, the nulls turn out to be irrelevant.

 Peter> Forget about abbreviation entirely for a minute - consider plain
 Peter> old sorting of strxfrm() blobs, which for example the glibc
 Peter> documentation recommends (with some caveats) as the preferred
 Peter> approach to sorting strings while respecting collation. Suppose
 Peter> that your applications happens to frequently encounter fully
 Peter> sorted arrays of strings when passed an array of strings to
 Peter> sort. If you were using our qsort() implementation, which,
 Peter> rather questionably, has a pre-check for fully sorted input
 Peter> (that throws away work when the pre-check fails), then you'll
 Peter> only have n comparisons. Even if an encoding is only as
 Peter> expensive as an actual comparison, the potential to lose with
 Peter> encoding clearly exists. Similarly, we don't abbreviate for
 Peter> top-n heap sorts, even though all of those abbreviated keys may
 Peter> be used for at least one comparison.

Nothing in the above paragraph has the slightest relevance to even the
text abbreviation code, much less the numeric one.

 Peter> Now, I hear what you're saying about weighing the costs and the
 Peter> benefits -- you point out that abbreviation of NULL values is
 Peter> not a cost, which is certainly true.

It's not a cost because it DOESN'T HAPPEN. The abbreviation function is
not called for null inputs.

 Peter> But if the total number of NULL values is so high that they
 Peter> dominate, then abbreviation might well be the wrong thing for
 Peter> that reason alone.

No. This is the point that you're getting wrong. The amount of time
saved by the abbreviation code depends ONLY on the number and
cardinality of the NON-NULL values. Also, the time _lost_ by the
abbreviation code if we fail to abort in pathological cases STILL
depends only on the number of non-null values, so there's no benefit in
aborting even in a case when we have 1000 equal non-null values mixed in
with tens of millions of nulls.

Let me give you an actual example:

create table numtest2 as
  select floor(random() * 200)::numeric as a
from generate_series(1,100);
create table numtest3 as
  select a from (select floor(random() * 200)::numeric as a
   from generate_series(1,100)
  union all
 select null from generate_series(1,400)) s
  order by random();

So one table has a million non-null rows with 200 distinct values, and
the other has the same plus 4 million null rows.

Using my code, which ignores nulls in the cost model:

select * from numtest2 order by a offset 1;  -- 1.5 seconds
select * from numtest3 order by a offset 1;  -- 3.6 seconds

With abbreviation disabled entirely:

select * from numtest2 order by a offset 1;  -- 4.5 seconds
select * from numtest3 order by a offset 1;  -- 6.8 seconds

Notice that while proportionally the speedup is only <2x rather than 3x
for the version with nulls, the absolute difference in time is the same
in both cases - abbreviation saved us ~3 seconds in both cases.

(This relation remains true for larger values too: make it 19 million
nulls rather than 4 million and the timings are 11.5 sec / 14.5 sec,
still a 3 sec difference.)

Your version would have aborted abbrevation on that second query, thus
losing a 3 second speedup. How on earth is that supposed to be
justified? It's not like there's any realistically possible case where
your version performs faster than mine by more than a tiny margin.

 Peter> Where I think your argument is stronger is around the cost of
 Peter> actually aborting in particular (even though not much work is
 Peter> thrown away).  Scanning through the memtuples array once more
 Peter> certainly isn't free.

Yes, the cost of actually aborting abbreviation goes up with the number
of nulls. But your version of the code makes that WORSE, by making it
more likely that we will abort unnecessarily.

If we use the raw value of memtuplecount for anything, it should be to
make it LESS likely that we abort abbreviations (since we'd be paying a
higher cost), not more. But benchmarking doesn't suggest that this is
necessary, at least not for numerics.

 Peter> And you could take the view that it's always worth the risk,
 Peter> since it's at most a marginal cost saved if the first ~10k
 Peter> tuples are representative, but a large cost incurred when they
 Peter> are not. So on refl

Re: [HACKERS] Abbreviated keys for Numeric

[Andrew Gierth]

> "Peter" == Peter Geoghegan  writes:

 >> Was there some reason why you added #include "utils/memutils.h"?
 >> Because I don't see anything in your patch that actually needs it.

 Peter> MaxAllocSize is defiined there.

So it is. (That seems to me to be another mistake along the same lines
as putting the context callbacks in memutils; it's a definition that
callers of palloc need to use, even if they're not creating contexts
themselves.)

However, given that this is a buffer for aligning VARATT_IS_SHORT
datums, allowing it to expand to MaxAllocSize seems like overkill; the
updated patch I just posted preallocates it at the max size of a short
varlena (plus long varlena header).

-- 
Andrew (irc:RhodiumToad)


-- 
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] debug_sortsupport GUC?

[Andrew Gierth]

> "Peter" == Peter Geoghegan  writes:

 >> So if these debugging elogs are to be kept at all, I propose that
 >> rather than being compile-time options they should be controlled by
 >> a debug_sortsupport GUC. Opinions?

 Peter> This seems like a reasonable idea. Why wouldn't it just be under
 Peter> the existing trace_sort GUC, though? That's been enabled by
 Peter> default since 8.1. It's already defined in pg_config_manual.h.

Ungh... yes, it's defined by default, but it clearly still requires
keeping the #ifdefs in there in order to still build if someone manually
undefines it. Was hoping to avoid the #ifdefs entirely - perhaps the
existing #ifdefs should just be removed? If it's been enabled since 8.1
it seems unlikely to be causing anyone any issues.

-- 
Andrew (irc:RhodiumToad)


-- 
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] Abbreviated keys for Numeric

[Peter Geoghegan]

On Sun, Mar 22, 2015 at 6:48 PM, Andrew Gierth
 wrote:
> Your version would have aborted abbrevation on that second query, thus
> losing a 3 second speedup. How on earth is that supposed to be
> justified? It's not like there's any realistically possible case where
> your version performs faster than mine by more than a tiny margin.

As I said, that's really why you won the argument on this particular
point. Why are you still going on about it?

>  Peter> Where I think your argument is stronger is around the cost of
>  Peter> actually aborting in particular (even though not much work is
>  Peter> thrown away).  Scanning through the memtuples array once more
>  Peter> certainly isn't free.
>
> Yes, the cost of actually aborting abbreviation goes up with the number
> of nulls. But your version of the code makes that WORSE, by making it
> more likely that we will abort unnecessarily.
>
> If we use the raw value of memtuplecount for anything, it should be to
> make it LESS likely that we abort abbreviations (since we'd be paying a
> higher cost), not more. But benchmarking doesn't suggest that this is
> necessary, at least not for numerics.
>
>  Peter> And you could take the view that it's always worth the risk,
>  Peter> since it's at most a marginal cost saved if the first ~10k
>  Peter> tuples are representative, but a large cost incurred when they
>  Peter> are not. So on reflection, I am inclined to put the check for
>  Peter> non-null values back in.
>
> Right result but the wrong reasoning.

I think that there is an argument to be made against abbreviation when
we simply have a small list of strings to begin with (e.g. 50), as per
the glibc strxfrm() docs (which, as I said, may not apply with numeric
to the same extent). It didn't end up actually happening that way for
the text opclass for various reasons, mostly because the cost model
was already complicated enough. Ideally, the number of comparisons per
key is at least 10 when we abbreviate with text, which works out at
about 1,000 tuples (as costed by cost_sort()). For that reason,
leaving aside the risk of aborting when the sampled tuples are not
representative (which is a big issue, that does clearly swing things
in favor of always disregarding NULLs), having few actual values to
sort is in theory a reason to not encode/abbreviate.

-- 
Peter Geoghegan


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

Re: [HACKERS] debug_sortsupport GUC?

[Peter Geoghegan]

On Sun, Mar 22, 2015 at 6:59 PM, Andrew Gierth
 wrote:
> Ungh... yes, it's defined by default, but it clearly still requires
> keeping the #ifdefs in there in order to still build if someone manually
> undefines it. Was hoping to avoid the #ifdefs entirely - perhaps the
> existing #ifdefs should just be removed? If it's been enabled since 8.1
> it seems unlikely to be causing anyone any issues.

I'd vote for removing the #ifdefs, and using trace_sort.

-- 
Peter Geoghegan


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

Re: [HACKERS] Zero-padding and zero-masking fixes for to_char(float)

[Bruce Momjian]

On Sun, Mar 22, 2015 at 12:46:08PM -0400, Noah Misch wrote:
> On Sun, Mar 22, 2015 at 11:22:26AM -0400, Bruce Momjian wrote:
> > What has me more concerned is the Solaris 10 failure.  This query:
> > 
> > SELECT to_char(float8 '999', 'D' || repeat('9', 
> > 1000));
> > 
> > expects this:
> > 
> > 999.000...
> > 
> > but on Solaris 10 gets this:
> > 
> > .00
> > 
> > Yes, the nines are gone, and only this query is failing.  Oddly, this
> > query did not fail, though the only difference is fewer decimal digits:
> > 
> > SELECT to_char(float8 '999', 'D');
> > 
> > This smells like a libc bug, e.g. OmniOS 5.11 passed the test.
> 
> Use of the "f" conversion specifier with precision greater than 512 is not
> portable; I get a similar diff on AIX 7.  Until this patch, PostgreSQL would
> not use arbitrarily-large precisions on that conversion specifier.  (Who would
> guess, but the "e" conversion specifier is not affected.)

Yeah.

> I recommend adding a "configure" test to use our snprintf.c replacements if
> sprintf("%.*f", 65536, 999.0) gives unexpected output.

Do we really want to go to our /port snprintf just to handle 512+
digits?

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

  + Everyone has their own god. +


-- 
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] Zero-padding and zero-masking fixes for to_char(float)

[Bruce Momjian]

On Sun, Mar 22, 2015 at 04:41:19PM -0400, Noah Misch wrote:
> When you posted this, I made a note to review it.
> 
> On Wed, Mar 18, 2015 at 05:52:44PM -0400, Bruce Momjian wrote:
> > This "junk" digit zeroing matches the Oracle behavior:
> > 
> > SELECT to_char(1.123456789123456789123456789d, 
> > '9.9') as x from dual;
> > --
> > 1.12345678912345680
> > 
> > Our output with the patch would be:
> > 
> > SELECT to_char(float8 '1.123456789123456789123456789', 
> > '9.9');
> > --
> > 1.12345678912345000
> > 
> > which is pretty close.
> 
> PostgreSQL 9.4 returns "1.12345678912346".  Your patch truncates digits past
> DBL_DIG, whereas both Oracle and PostgreSQL 9.4 round to the nearest DBL_DIG
> digits.  PostgreSQL must continue to round.

Ah, that rounding is a big problem.  I can't just round because if the
digit to be rounded up is '9', I have to set that to zero and increment
the previous digit, and that could cascade and shift the entire string
one over.  I think I have to go back to the original code that does the
weird computations to figure out how many digits are on the left of the
decimal point, then set the format string after.  I was hoping my patch
could clean that all up, but we now need snprintf to do that rounding
for us.  :-(

> These outputs show Oracle treating 17 digits as significant while PostgreSQL
> treats 15 digits as significant.  Should we match Oracle in this respect while
> we're breaking compatibility anyway?  I tend to think yes.

Uh, I am hesistant to adjust our precision to match Oracle as I don't
know what they are using internally.

> > *** int4_to_char(PG_FUNCTION_ARGS)
> > *** 5214,5221 
> > /* we can do it easily because float8 won't lose any precision 
> > */
> > float8  val = (float8) value;
> >   
> > !   orgnum = (char *) palloc(MAXDOUBLEWIDTH + 1);
> > !   snprintf(orgnum, MAXDOUBLEWIDTH + 1, "%+.*e", Num.post, val);
> >   
> > /*
> >  * Swap a leading positive sign for a space.
> > --- 5207,5213 
> > /* we can do it easily because float8 won't lose any precision 
> > */
> > float8  val = (float8) value;
> >   
> > !   orgnum = psprintf("%+.*e", Num.post, val);
> 
> Neither the submission notes nor the commit messages mentioned it, but this
> improves behavior for to_char(int4, text).  Specifically, it helps 
> formats with more than about 500 decimal digits:
> 
>   SELECT length(to_char(1, '9D' || repeat('9', 800) || ''));

Wow, that is surprising.

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

  + Everyone has their own god. +


-- 
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] Resetting crash time of background worker

[Amit Khandekar]

On 17 March 2015 at 19:12, Robert Haas  wrote:

> On Tue, Mar 17, 2015 at 1:33 AM, Amit Khandekar 
> wrote:
> > I think we either have to retain the knowledge that the worker has
> crashed
> > using some new field, or else, we should reset the crash time only if it
> is
> > not flagged BGW_NEVER_RESTART.
>
> I think you're right, and I think we should do the second of those.
> Thanks for tracking this down.
>

Thanks. Attached a patch accordingly. Put this into the June 2015
commitfest.
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 85a3b3a..1536691 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -397,9 +397,9 @@ BackgroundWorkerStopNotifications(pid_t pid)
 /*
  * Reset background worker crash state.
  *
- * We assume that, after a crash-and-restart cycle, background workers should
- * be restarted immediately, instead of waiting for bgw_restart_time to
- * elapse.
+ * We assume that, after a crash-and-restart cycle, background workers without
+ * the never-restart flag should be restarted immediately, instead of waiting
+ * for bgw_restart_time to elapse.
  */
 void
 ResetBackgroundWorkerCrashTimes(void)
@@ -411,7 +411,14 @@ ResetBackgroundWorkerCrashTimes(void)
 		RegisteredBgWorker *rw;
 
 		rw = slist_container(RegisteredBgWorker, rw_lnode, iter.cur);
-		rw->rw_crashed_at = 0;
+
+		/*
+		 * For workers that should not be restarted, we don't want to loose
+		 * the information that they have crashed, otherwise they would be
+		 * treated as new workers.
+		 */
+		if (rw->rw_worker.bgw_restart_time != BGW_NEVER_RESTART)
+			rw->rw_crashed_at = 0;
 	}
 }
 

-- 
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] Zero-padding and zero-masking fixes for to_char(float)

[Bruce Momjian]

On Sun, Mar 22, 2015 at 10:53:12PM -0400, Bruce Momjian wrote:
> > > *** int4_to_char(PG_FUNCTION_ARGS)
> > > *** 5214,5221 
> > >   /* we can do it easily because float8 won't lose any 
> > > precision */
> > >   float8  val = (float8) value;
> > >   
> > > ! orgnum = (char *) palloc(MAXDOUBLEWIDTH + 1);
> > > ! snprintf(orgnum, MAXDOUBLEWIDTH + 1, "%+.*e", Num.post, 
> > > val);
> > >   
> > >   /*
> > >* Swap a leading positive sign for a space.
> > > --- 5207,5213 
> > >   /* we can do it easily because float8 won't lose any 
> > > precision */
> > >   float8  val = (float8) value;
> > >   
> > > ! orgnum = psprintf("%+.*e", Num.post, val);
> > 
> > Neither the submission notes nor the commit messages mentioned it, but this
> > improves behavior for to_char(int4, text).  Specifically, it helps 
> > formats with more than about 500 decimal digits:
> > 
> >   SELECT length(to_char(1, '9D' || repeat('9', 800) || ''));
> 
> Wow, that is surprising.

Ah, yes.  There was a problem where were clipping off int4  output
to MAXDOUBLEWIDTH, which made no sense, so there was the fix for that as
well.

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

  + Everyone has their own god. +


-- 
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] New functions

[Michael Paquier]

On Mon, Mar 23, 2015 at 1:48 AM, Воронин Дмитрий
 wrote:
>
>>  Please, attach new version of my patch to commitfest page.
>
> Michael, I found a way to attach patch. sorry to trouble.

Cool. Thanks. I am seeing your patch entry here:
https://commitfest.postgresql.org/5/192/
I'll try to take a look at it for the next commit fest, but please do
not expect immediate feedback things are getting wrapped up for 9.5.
Regards,
-- 
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] Zero-padding and zero-masking fixes for to_char(float)

[Tom Lane]

Bruce Momjian  writes:
> On Sun, Mar 22, 2015 at 12:46:08PM -0400, Noah Misch wrote:
>> I recommend adding a "configure" test to use our snprintf.c replacements if
>> sprintf("%.*f", 65536, 999.0) gives unexpected output.

> Do we really want to go to our /port snprintf just to handle 512+
> digits?

I'd rather not go that direction (that is, to using a configure test).
It assumes that all copies of libc on a particular platform behave the
same, which seems like a bad bet to me.  I think we'd be better off to
avoid asking libc to do anything that might not work everywhere.

On the other hand, this line of thought might end up having you
reimplement in formatting.c the same logic I put into snprintf.c
recently, which seems a bit silly.

regards, tom lane


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

[HACKERS] recovery_target_time ignored ?

[Venkata Balaji N]

Hi,

Assuming that this might require a patch, i am posting this in
pgsql-hackers. Apologies, if this is not the appropriate mailing list to
start this discussion.

I performed a PITR and saw the below message in the log file is a bit
confusing.

2015-03-23 13:49:09.816 GMT-10 DB= PID=4707 LOG: * database system was
interrupted; last known up at 2015-03-23 10:30:26 GMT-10*
2015-03-23 13:49:09.817 GMT-10 DB= PID=4707 LOG: * starting point-in-time
recovery to 2015-03-23 10:00:26+10*
2015-03-23 13:49:09.827 GMT-10 DB= PID=4707 LOG:  restored log file
"0001000B0020" from archive
2015-03-23 13:49:09.888 GMT-10 DB= PID=4707 LOG:  redo starts at B/2090
2015-03-23 13:49:09.937 GMT-10 DB= PID=4707 LOG:  consistent recovery state
reached at B/20B8
2015-03-23 13:49:09.947 GMT-10 DB= PID=4707 LOG:  restored log file
"0001000B0021" from archive
2015-03-23 13:49:09.950 GMT-10 DB= PID=4707 LOG:  *recovery stopping before
commit of transaction 16267, time 2015-03-23 13:22:37.53007+10*


By mistake i gave "recovery_target_time" as 10:00 GMT which is 25/30
minutes behind the backup start/end time registered in the backup_label.

The parameter recovery_target_time is ignored and recovery proceeds further
applying all the available WAL Archive files finally ends up bringing up
the database.

I think it would make sense if the recovery does not proceed any further
and error out with a message like "recovery_target_time is behind the
backup time.. please consider using the backup taken prior to the
recovery_target_time"

recovery.conf file is as follows :

restore_command='cp /data/pgdata9400backup/pgwalarch9400backup/%f %p '
recovery_target_time='2015-03-23 10:00:26 GMT-10'
recovery_target_inclusive='true'

If this requires a patch, i would like to take it up.

Regards,
Venkata Balaji N

Re: [HACKERS] Table-level log_autovacuum_min_duration

[Fujii Masao]

On Thu, Mar 19, 2015 at 1:43 PM, Michael Paquier
 wrote:
> On Thu, Mar 19, 2015 at 12:40 PM, Michael Paquier
>  wrote:
>> On Thu, Mar 19, 2015 at 12:23 PM, Fujii Masao  wrote:
>>> Are you planning to update the patch so that it's based on the commit 
>>> 0d83138?
>>
>> Yes... Very soon.
>
> And here is the rebased patch.

Thanks for rebasing the patch! Looks good to me.

One concern about this patch is; currently log_autovacuum_min_duration can be
changed even while autovacuum worker is running. So, for example, when
the admin notices that autovacuum is taking very long time, he or she can
enable logging of autovacuum activity on the fly. But this patch completely
prevents us from doing that, because, with the patch, autovacuum worker always
picks up the latest setting value at its starting time and then keeps using it
to the end. Probably I can live with this. But does anyone has other thought?

Regards,

-- 
Fujii Masao


-- 
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] Display of multi-target-table Modify plan nodes in EXPLAIN

[Ashutosh Bapat]

On Sun, Mar 22, 2015 at 6:32 AM, Tom Lane  wrote:

> I've gotten the foreign table inheritance patch to a state where I'm
> almost ready to commit it, but there's one thing that's bothering me,
> which is what it does for EXPLAIN.  As it stands you might get something
> like
>
> regression=# explain (verbose) update pt1 set c1=c1+1;
>  QUERY PLAN
>
> 
>  Update on public.pt1  (cost=0.00..321.05 rows=3541 width=46)
>Foreign Update on public.ft1
>  Remote SQL: UPDATE public.ref1 SET c1 = $2 WHERE ctid = $1
>Foreign Update on public.ft2
>  Remote SQL: UPDATE public.ref2 SET c1 = $2 WHERE ctid = $1
>->  Seq Scan on public.pt1  (cost=0.00..0.00 rows=1 width=46)
>  Output: (pt1.c1 + 1), pt1.c2, pt1.c3, pt1.ctid
>->  Foreign Scan on public.ft1  (cost=100.00..148.03 rows=1170 width=46)
>  Output: (ft1.c1 + 1), ft1.c2, ft1.c3, ft1.ctid
>  Remote SQL: SELECT c1, c2, c3, ctid FROM public.ref1 FOR UPDATE
>->  Foreign Scan on public.ft2  (cost=100.00..148.03 rows=1170 width=46)
>  Output: (ft2.c1 + 1), ft2.c2, ft2.c3, ft2.ctid
>  Remote SQL: SELECT c1, c2, c3, ctid FROM public.ref2 FOR UPDATE
>->  Seq Scan on public.child3  (cost=0.00..25.00 rows=1200 width=46)
>  Output: (child3.c1 + 1), child3.c2, child3.c3, child3.ctid
> (15 rows)
>
> which seems fairly messy to me because you have to guess at which of
> the child plan subtrees goes with which "Remote SQL" item.
>
> In a green field we might choose to solve this by refactoring the output
> so that it's logically
>
> Multi-Table Update
> [
>   Update Target: pt1
>   Plan: (seq scan on pt1 here)
> ]
> [
>   Update Target: ft1
>   Remote SQL: UPDATE ref1 ...
>   Plan: (foreign scan on ft1 here)
> ]
> [
>   Update Target: ft2
>   Remote SQL: UPDATE ref2 ...
>   Plan: (foreign scan on ft2 here)
> ]
> [
>   Update Target: child3
>   Plan: (seq scan on child3 here)
> ]
>
> but I think that ship has sailed.  Changing the logical structure of
> EXPLAIN output like this would break clients that know what's where in
> JSON/YAML/XML formats, which is exactly what we said we wouldn't do with
> those output formats.
>
> What I'm imagining instead is that when there's more than one
> target relation, we produce output like
>
> Multi-Table Update
> Relation Name: pt1  -- this is the *nominal* target
> Target Relations:
>   [
> Relation Name: pt1  -- first actual target
> Schema: public
> Alias: pt1
>   ]
>   [
> Relation Name: ft1
> Schema: public
> Alias: ft1
> Remote SQL: UPDATE ref1 ...
>   ]
>   [
> Relation Name: ft2
> Schema: public
> Alias: ft2
> Remote SQL: UPDATE ref2 ...
>   ]
>   [
> Relation Name: child3
> Schema: public
> Alias: child3
>   ]
> Plans:
>   Plan: (seq scan on pt1 here)
>   Plan: (foreign scan on ft1 here)
>   Plan: (foreign scan on ft2 here)
>   Plan: (seq scan on child3 here)
>
> That is, there'd be a new subnode of ModifyTable (which existing clients
> would ignore), and that would fully identify *each* target table not only
> foreign ones.  The text-mode output might look like
>
>  Update on public.pt1  (cost=0.00..321.05 rows=3541 width=46)
>Update on public.pt1
>Foreign Update on public.ft1
>  Remote SQL: UPDATE public.ref1 SET c1 = $2 WHERE ctid = $1
>Foreign Update on public.ft2
>  Remote SQL: UPDATE public.ref2 SET c1 = $2 WHERE ctid = $1
>Update on public.child3
>->  Seq Scan on public.pt1  (cost=0.00..0.00 rows=1 width=46)
>  Output: (pt1.c1 + 1), pt1.c2, pt1.c3, pt1.ctid
>... etc ...
>
> where there would always now be as many target tables listed as
> there are child plan trees.
>
>
This looks better.
In the format above, you have specified both the Remote SQL for scan as
well as update but in the example you have only mentioned only Remote SQL
for update; it may be part of "... etc ...". It's better to provide both.


> Thoughts, better ideas?
>
> regards, tom lane
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http

Re: [HACKERS] Display of multi-target-table Modify plan nodes in EXPLAIN

[Tom Lane]

Ashutosh Bapat  writes:
> On Sun, Mar 22, 2015 at 6:32 AM, Tom Lane  wrote:
>> What I'm imagining instead is that when there's more than one
>> target relation, we produce output like ...

> This looks better.
> In the format above, you have specified both the Remote SQL for scan as
> well as update but in the example you have only mentioned only Remote SQL
> for update; it may be part of "... etc ...". It's better to provide both.

Hm?  We don't have scan nodes that read more than one table, so I'm
not following your point.

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] Order of enforcement of CHECK constraints?

[Ashutosh Bapat]

I might be only one objecting here but ...

On Sat, Mar 21, 2015 at 12:45 AM, Tom Lane  wrote:

> My Salesforce colleagues noticed some tests flapping as a result of table
> CHECK constraints not always being enforced in the same order; ie, if a
> tuple insertion/update violates more than one CHECK constraint, it's not
> deterministic which one is reported.  This is evidently because
> relcache.c's CheckConstraintFetch() just happily loads up the constraints
> in whatever order it happens to find them in pg_constraint.
>

Why is it important to report in deterministic manner? If it really
matters, we should probably report all the failing constraints. A
comparable example would be compiler throwing errors.


>
> There's at least one regression test case where this can happen, so we've
> been lucky so far that this hasn't caused buildfarm noise.
>
> We could fix it by, say, having CheckConstraintFetch() sort the
> constraints by name after loading them.
>

> In principle the same problem could occur for domain CHECK constraints,
> though the odds of multiple CHECKs failing are probably a lot lower.
>
> Do people think this is worth fixing?
>

Downthread, parallels are being drawn with triggers. The order in which
triggers are fired matters since the triggers can affect each other's
results but constraint don't do that. Do they? So, why should we waste some
cycles in sorting the constraints?


>
> 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
>



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

Re: [HACKERS] Table-level log_autovacuum_min_duration

[Michael Paquier]

On Mon, Mar 23, 2015 at 1:54 PM, Fujii Masao  wrote:
> On Thu, Mar 19, 2015 at 1:43 PM, Michael Paquier
>  wrote:
>> On Thu, Mar 19, 2015 at 12:40 PM, Michael Paquier
>>  wrote:
>>> On Thu, Mar 19, 2015 at 12:23 PM, Fujii Masao  wrote:
 Are you planning to update the patch so that it's based on the commit 
 0d83138?
>>>
>>> Yes... Very soon.
>>
>> And here is the rebased patch.
>
> Thanks for rebasing the patch! Looks good to me.
>
> One concern about this patch is; currently log_autovacuum_min_duration can be
> changed even while autovacuum worker is running. So, for example, when
> the admin notices that autovacuum is taking very long time, he or she can
> enable logging of autovacuum activity on the fly. But this patch completely
> prevents us from doing that, because, with the patch, autovacuum worker always
> picks up the latest setting value at its starting time and then keeps using it
> to the end. Probably I can live with this. But does anyone has other thought?

In AutoVacWorkerMain, I am reading the following:

 * Currently, we don't pay attention to postgresql.conf changes that
 * happen during a single daemon iteration, so we can ignore SIGHUP.
 */
pqsignal(SIGHUP, SIG_IGN);

So a worker does not see changes in postgresql.conf once it is run and
processes a database, no? The launcher does run ProcessConfigFile()
when SIGHUP shows up though.
Regards,
-- 
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] Display of multi-target-table Modify plan nodes in EXPLAIN

[Ashutosh Bapat]

On Mon, Mar 23, 2015 at 10:51 AM, Tom Lane  wrote:

> Ashutosh Bapat  writes:
> > On Sun, Mar 22, 2015 at 6:32 AM, Tom Lane  wrote:
> >> What I'm imagining instead is that when there's more than one
> >> target relation, we produce output like ...
>
> > This looks better.
> > In the format above, you have specified both the Remote SQL for scan as
> > well as update but in the example you have only mentioned only Remote SQL
> > for update; it may be part of "... etc ...". It's better to provide both.
>
> Hm?  We don't have scan nodes that read more than one table, so I'm
> not following your point.
>
> regards, tom lane
>

In the format you specified
  Multi-Table Update
Relation Name: pt1  -- this is the *nominal* target
Target Relations:
  [
Relation Name: pt1  -- first actual target
Schema: public
Alias: pt1
  ]
  [
Relation Name: ft1
Schema: public
Alias: ft1
Remote SQL: UPDATE ref1 ...
  ]

Plans:
  Plan: (seq scan on pt1 here)
  Plan: (foreign scan on ft1 here)
For relation ft1, there is an Update node as well as a scan node. Update
node has Update Remote SQL and Scan will have corresponding SELECT Remote
SQL.

But in the text output you gave
Update on public.pt1  (cost=0.00..321.05 rows=3541 width=46)
   Update on public.pt1
   Foreign Update on public.ft1
 Remote SQL: UPDATE public.ref1 SET c1 = $2 WHERE ctid = $1
   Foreign Update on public.ft2
 Remote SQL: UPDATE public.ref2 SET c1 = $2 WHERE ctid = $1
   Update on public.child3
   ->  Seq Scan on public.pt1  (cost=0.00..0.00 rows=1 width=46)
 Output: (pt1.c1 + 1), pt1.c2, pt1.c3, pt1.ctid
   ... etc ...

For ft1 there is only Update Remote SQL. whereas for child3 you have
specified the Seq Scan as well. I was wondering if the foreign scan on ft1
is part of "...etc..."?. Further we have to make it clear which scan goes
with which Update, either by listing the scans in the same order as updates
or by associating each scan with corresponding update.

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

Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)

[Abhijit Menon-Sen]

At 2015-02-24 11:22:41 -0500, da...@pgmasters.net wrote:
>
> Patch v3 is attached.
>
> […]
>
> +/* Log class enum used to represent bits in auditLogBitmap */
> +enum LogClass
> +{
> + LOG_NONE = 0,
> +
> + /* SELECT */
> + LOG_READ = (1 << 0),
> +
> + /* INSERT, UPDATE, DELETE, TRUNCATE */
> + LOG_WRITE = (1 << 1),
> +
> + /* DDL: CREATE/DROP/ALTER */
> + LOG_DDL = (1 << 2),
> +
> + /* Function execution */
> + LOG_FUNCTION = (1 << 4),
> +
> + /* Function execution */
> + LOG_MISC = (1 << 5),
> +
> + /* Absolutely everything */
> + LOG_ALL = ~(uint64)0
> +};

The comment above LOG_MISC should be changed.

More fundamentally, this classification makes it easy to reuse LOGSTMT_*
(and a nice job you've done of that, with just a few additional special
cases), I don't think this level is quite enough for our needs. I think
it should at least be possible to specifically log commands that affect
privileges and roles.

I'm fond of finer categorisation for DDL as well, but I could live with
all DDL being lumped together.

I'm experimenting with a few approaches to do this without reintroducing
switch statements to test every command. That will require core changes,
but I think we can find an acceptable arrangement. I'll post a proof of
concept in a few days.

> + * Takes an AuditEvent and, if it log_check(), writes it to the audit
> log.

I don't think log_check is the most useful name, because this sentence
doesn't tell me what the function may do. Similarly, I would prefer to
have log_acl_check be renamed acl_grants_audit or similar. (These are
all static functions anyway, I don't think a log_ prefix is needed.)

> + /* Free the column set */
> + bms_free(tmpSet);

(An aside, really: there are lots of comments like this, which I don't
think add anything to understanding the code, and should be removed.)

> + /*
> +  * We don't have access to the parsetree here, so we have to 
> generate
> +  * the node type, object type, and command tag by decoding
> +  * rte->requiredPerms and rte->relkind.
> +  */
> + auditEvent.logStmtLevel = LOGSTMT_MOD;

(I am also trying to find a way to avoid having to do this.)

> + /* Set object type based on relkind */
> + switch (class->relkind)
> + {
> + case RELKIND_RELATION:
> + utilityAuditEvent.objectType = 
> OBJECT_TYPE_TABLE;
> + break;

This occurs elsewhere too. But I suppose new relkinds are added less
frequently than new commands.

Again on a larger level, I'm not sure how I feel about _avoiding_ the
use of event triggers for audit logging. Regardless of whether we use
the deparse code (which I personally think is a good idea; Álvaro has
been working on it, and it looks very nice) to log extra information,
using the object access hook inevitably means we have to reimplement
the identification/classification code here.

In "old" pgaudit, I think that extra effort is justified by the need to
be backwards compatible with pre-event trigger releases. In a 9.5-only
version, I am not at all convinced that this makes sense.

Thoughts?

-- Abhijit


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

Re: [HACKERS] proposal: plpgsql - Assert statement

[Pavel Stehule]

2015-01-28 0:13 GMT+01:00 Jim Nasby :

> On 1/27/15 1:30 PM, Pavel Stehule wrote:
>
>> I don't see the separate warning as being helpful. I'd just do
>> something like
>>
>> +(err_hint != NULL) ? errhint("%s",
>> err_hint) : errhint("Message attached to failed assertion is null") ));
>>
>>
>> done
>>
>>
>> There should also be a test case for a NULL message.
>>
>>
>> is there, if I understand well
>>
>
> I see it now. Looks good.


updated version with Jim Nasby's doc and rebase against last changes in
plpgsql.

Regards

Pavel


>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>
commit 93163d078e61a603ca3d34e9a0f888f097b0ec0a
Author: Pavel Stehule 
Date:   Mon Mar 23 06:32:22 2015 +0100

fix missing typmod

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index b30c68d..9bd9f1b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6999,6 +6999,20 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
 
 
 
+ 
+  enable_user_asserts (boolean)
+  
+   enable_user_asserts configuration parameter
+  
+  
+  
+   
+If true, any user assertions are evaluated.  By default, this 
+is set to true.
+   
+  
+ 
+
  
   exit_on_error (boolean)
   
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 158d9d2..0a80ecf 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3373,6 +3373,9 @@ END LOOP  label ;
   
Errors and Messages
 
+  
+RAISE statement
+

 RAISE

@@ -3565,7 +3568,33 @@ RAISE unique_violation USING MESSAGE = 'Duplicate user ID: ' || user_id;
  the whole category.
 

+  
+
+  
+ASSERT statement
 
+   
+ASSERT
+   
+
+   
+assertions
+in PL/pgSQL
+   
+
+   
+Use the ASSERT statement to ensure the
+predicate is allways true. If the predicate is false or is null,
+then a assertion exception is raised.
+
+
+ASSERT expression , message expression ;
+
+
+The user assertions can be enabled or disabled via
+.
+   
+  
  
 
  
diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt
index 28c8c40..da12428 100644
--- a/src/backend/utils/errcodes.txt
+++ b/src/backend/utils/errcodes.txt
@@ -454,6 +454,7 @@ PEERRCODE_PLPGSQL_ERROR  plp
 P0001EERRCODE_RAISE_EXCEPTIONraise_exception
 P0002EERRCODE_NO_DATA_FOUND  no_data_found
 P0003EERRCODE_TOO_MANY_ROWS  too_many_rows
+P0004EERRCODE_ASSERT_EXCEPTION   assert_exception
 
 Section: Class XX - Internal Error
 
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 23e594e..32f4c2c 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -99,6 +99,7 @@ bool		IsBinaryUpgrade = false;
 bool		IsBackgroundWorker = false;
 
 bool		ExitOnAnyError = false;
+bool		enable_user_asserts = true;
 
 int			DateStyle = USE_ISO_DATES;
 int			DateOrder = DATEORDER_MDY;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 26275bd..5c3596b 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1058,6 +1058,15 @@ static struct config_bool ConfigureNamesBool[] =
 	},
 
 	{
+		{"enable_user_asserts", PGC_USERSET, ERROR_HANDLING_OPTIONS,
+			gettext_noop("Enable user assert checks."),
+			NULL
+		},
+		&enable_user_asserts,
+		true,
+		NULL, NULL, NULL
+	},
+	{
 		{"exit_on_error", PGC_USERSET, ERROR_HANDLING_OPTIONS,
 			gettext_noop("Terminate session on any error."),
 			NULL
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index eacfccb..b20efac 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -149,6 +149,7 @@ extern bool IsBackgroundWorker;
 extern PGDLLIMPORT bool IsBinaryUpgrade;
 
 extern bool ExitOnAnyError;
+extern bool enable_user_asserts;
 
 extern PGDLLIMPORT char *DataDir;
 
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 6a93540..b8c4ac9 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -153,6 +153,8 @@ static int exec_stmt_return_query(PLpgSQL_execstate *estate,
 	   PLpgSQL_stmt_return_query *stmt);
 static int exec_stmt_raise(PLpgSQL_execstate *estate,
 PLpgSQL_stmt_raise *stmt);
+static int exec_stmt_assert(PLpgSQL_execstate *estate,
+PLpgSQL_stmt_assert *stmt);
 static int exec_stmt_execsql(PLpgSQL_execstate *estate,
   PLpgSQL_stmt_execsql *stmt);
 static int exec_stmt_dynexecute(PLpgSQL_execstate *estate,
@@ -1027,12 +1029,14 @@ exception_matches_conditions(ErrorData *edata, PLpgSQL_condition *cond)
 		int			sqlerrstate = cond->sqlerrstate;
 
 	

Re: [HACKERS] inherit support for foreign tables

[Ashutosh Bapat]

On Mon, Mar 23, 2015 at 12:09 AM, Robert Haas  wrote:

> On Sun, Mar 22, 2015 at 1:57 PM, Tom Lane  wrote:
> > Etsuro Fujita  writes:
> >> [ fdw-inh-8.patch ]
> >
> > I've committed this with some substantial rearrangements, notably:
>
> I'm really glad this is going in!  Thanks to to Shigeru Hanada and
> Etsuro Fujita for working on this, to you (Tom) for putting in the
> time to get it committed, and of course to the reviewers Ashutosh
> Bapat and Kyotaro Horiguchi for their time and effort.
>
> In a way, I believe we can think of this as the beginnings of a
> sharding story for PostgreSQL.  A lot more work is needed, of course
> -- join and aggregate pushdown are high on my personal list -- but
> it's a start.
>
>
+1.


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



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

Re: [HACKERS] Display of multi-target-table Modify plan nodes in EXPLAIN

[Heikki Linnakangas]

On 03/22/2015 03:02 AM, Tom Lane wrote:

In a green field we might choose to solve this by refactoring the output
so that it's logically

Multi-Table Update
[
  Update Target: pt1
  Plan: (seq scan on pt1 here)
]
[
  Update Target: ft1
  Remote SQL: UPDATE ref1 ...
  Plan: (foreign scan on ft1 here)
]
[
  Update Target: ft2
  Remote SQL: UPDATE ref2 ...
  Plan: (foreign scan on ft2 here)
]
[
  Update Target: child3
  Plan: (seq scan on child3 here)
]


The "Remote SQL" nodes should go under the Foreign Scan nodes.


but I think that ship has sailed.  Changing the logical structure of
EXPLAIN output like this would break clients that know what's where in
JSON/YAML/XML formats, which is exactly what we said we wouldn't do with
those output formats.


If we have promised that, I think we should break the promise. No 
application should depend on the details of EXPLAIN output, even if it's 
in JSON/YAML/XML format. EXPLAIN is used by humans, and by tools like 
pgAdmin that display the output for humans, so let's do what makes most 
sense for humans. Admin tools will have to deal with new node types, and 
also new plan structures in every new release anyway. And if an admin 
tool doesn't recognize the new format, it surely falls back to 
displaying them in some a reasonable generic form.


- Heikki



--
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] Zero-padding and zero-masking fixes for to_char(float)

[Noah Misch]

On Mon, Mar 23, 2015 at 12:36:25AM -0400, Tom Lane wrote:
> Bruce Momjian  writes:
> > On Sun, Mar 22, 2015 at 12:46:08PM -0400, Noah Misch wrote:
> >> I recommend adding a "configure" test to use our snprintf.c replacements if
> >> sprintf("%.*f", 65536, 999.0) gives unexpected output.
> 
> > Do we really want to go to our /port snprintf just to handle 512+
> > digits?
> 
> I'd rather not go that direction (that is, to using a configure test).
> It assumes that all copies of libc on a particular platform behave the
> same, which seems like a bad bet to me.  I think we'd be better off to
> avoid asking libc to do anything that might not work everywhere.
> 
> On the other hand, this line of thought might end up having you
> reimplement in formatting.c the same logic I put into snprintf.c
> recently, which seems a bit silly.

We already assume that a positive PGAC_FUNC_SNPRINTF_LONG_LONG_INT_MODIFIER,
PGAC_FUNC_SNPRINTF_ARG_CONTROL and PGAC_FUNC_SNPRINTF_SIZE_T_SUPPORT will hold
forever.  While one can construct counterexamples, I don't recall much push to
change that.  This %1000f problem evades casual testing and will fade from
hacker memory.  We're liable to introduce similar bugs if everyone hacking
high level code like src/backend/utils/adt needs to remember it.

A potential compromise is to verify the configure-detected behavior during
startup_hacks().  The DBA will see clearly when the runtime libraries exhibit
a bug absent at build time.


-- 
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] Display of multi-target-table Modify plan nodes in EXPLAIN

[Peter Geoghegan]

On Sun, Mar 22, 2015 at 11:38 PM, Heikki Linnakangas  wrote:
> If we have promised that, I think we should break the promise. No
> application should depend on the details of EXPLAIN output, even if it's in
> JSON/YAML/XML format. EXPLAIN is used by humans, and by tools like pgAdmin
> that display the output for humans, so let's do what makes most sense for
> humans. Admin tools will have to deal with new node types, and also new plan
> structures in every new release anyway. And if an admin tool doesn't
> recognize the new format, it surely falls back to displaying them in some a
> reasonable generic form.


+1

-- 
Peter Geoghegan


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