Re: [HACKERS] INT64_MIN and _MAX
On Sun, Mar 22, 2015 at 2:26 AM, Andres Freund and...@anarazel.de 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
Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates
On 22 March 2015 at 22:22, Andres Freund and...@2ndquadrant.com 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 t...@sss.pgh.pa.us wrote: Andres Freund and...@2ndquadrant.com 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
On March 22, 2015 10:34:04 AM GMT+01:00, Michael Paquier michael.paqu...@gmail.com wrote: On Sun, Mar 22, 2015 at 6:22 PM, Andres Freund and...@2ndquadrant.com 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 t...@sss.pgh.pa.us wrote: Andres Freund and...@2ndquadrant.com 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=jacanadt=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] PITR failing to stop before DROP DATABASE
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
[HACKERS] Fix pgbench --progress report under (very) low rate
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] Using 128-bit integers for sum, avg and statistics aggregates
On 22/03/15 10:35, Andres Freund wrote: On March 22, 2015 10:34:04 AM GMT+01:00, Michael Paquier michael.paqu...@gmail.com wrote: On Sun, Mar 22, 2015 at 6:22 PM, Andres Freund and...@2ndquadrant.com 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=jacanadt=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] Order of enforcement of CHECK constraints?
On 3/20/15 3:37 PM, Tom Lane wrote: =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= fabriziome...@gmail.com writes: On Fri, Mar 20, 2015 at 4:19 PM, Peter Geoghegan p...@heroku.com wrote: On Fri, Mar 20, 2015 at 12:15 PM, Tom Lane t...@sss.pgh.pa.us 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] INT64_MIN and _MAX
On March 22, 2015 6:19:52 AM GMT+01:00, Andrew Gierth and...@tao11.riddles.org.uk wrote: Andrew == Andrew Gierth and...@tao11.riddles.org.uk writes: Petr == Petr Jelinek p...@2ndquadrant.com 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
On March 22, 2015 3:15:07 AM GMT+01:00, Bruce Momjian br...@momjian.us 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] proposal: searching in array function - array_position
2015-03-22 11:30 GMT+01:00 Dean Rasheed dean.a.rash...@gmail.com:
On 22 March 2015 at 06:11, Pavel Stehule pavel.steh...@gmail.com 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 pavel.steh...@gooddata.com
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 xref linkend=indexes-types.
/para
+ para
+ You can also search for specific values in an array using the functionarray_position/
+ and functionarray_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:
+
+programlisting
+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}
+/programlisting
+ /para
+
tip
para
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)') ...
primaryarray_lower/primary
/indexterm
indexterm
+primaryarray_position/primary
+ /indexterm
+ indexterm
+primaryarray_positions/primary
+ /indexterm
+ indexterm
primaryarray_prepend/primary
/indexterm
indexterm
@@ -11599,6 +11605,32 @@ SELECT NULLIF(value, '(none)') ...
row
entry
literal
+ functionarray_position/function(typeanyarray/type, typeanyelement/type optional, typeint/type/optional)
+ /literal
+/entry
+entrytypeint/type/entry
+entryreturns 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)/entry
+entryliteralarray_position(ARRAY['sun','mon','tue','wed','thu','fri','sat'], 'mon')/literal/entry
+entryliteral2/literal/entry
+ /row
+ row
+entry
+ literal
+ functionarray_positions/function(typeanyarray/type, typeanyelement/type)
+ /literal
+/entry
+entrytypeint[]/type/entry
+entryreturns an array of subscripts of all occurrences of the second
+argument in the array given as first argument (array must be
+one-dimensional)/entry
+entryliteralarray_positions(ARRAY['A','A','B','A'], 'A')/literal/entry
+entryliteral{1,2,4}/literal/entry
+ /row
+ row
+entry
+ literal
functionarray_prepend/function(typeanyelement/type, typeanyarray/type)
/literal
/entry
@@ -11708,6 +11740,23 @@ NULL baz/literallayout(3 rows)/entry
/table
para
+In functionarray_position/function and functionarray_positions/,
+each array element is compared to the searched value using
+literalIS NOT DISTINCT FROM/literal semantics.
+ /para
+
+ para
+In functionarray_position/function, literalNULL/literal is returned
+if the value is not found.
+ /para
+
+ para
+In functionarray_positions/function, literalNULL/literal is returned
+only if the array is literalNULL/literal; if the value is not found in
+the array, an empty array is returned instead.
+ /para
+
+ para
In functionstring_to_array/function, 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
Re: [HACKERS] proposal: searching in array function - array_position
On 22 March 2015 at 06:11, Pavel Stehule pavel.steh...@gmail.com 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] [PATCH] Add transforms feature
2015-03-22 3:55 GMT+01:00 Peter Eisentraut pete...@gmx.net: 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 pete...@gmx.net mailto:pete...@gmx.net: 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
Re: [HACKERS] New functions
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 openssl/x509.h
+ #include openssl/x509v3.h
#include openssl/asn1.h
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 */
Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates
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=jacanadt=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?
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] Using 128-bit integers for sum, avg and statistics aggregates
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 t...@sss.pgh.pa.us wrote: Andres Freund and...@2ndquadrant.com 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] Lets delete src/test/performance
On March 22, 2015 3:21:34 AM GMT+01:00, Bruce Momjian br...@momjian.us wrote: On Thu, Mar 19, 2015 at 09:57:05PM -0400, Bruce Momjian wrote: commit cf76759f34a172d424301cfa3723baee37f4a7ce Author: Vadim B. Mikheev vadi...@yahoo.com 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
On Sun, Mar 22, 2015 at 6:34 PM, David Rowley dgrowle...@gmail.com wrote: On 22 March 2015 at 22:22, Andres Freund and...@2ndquadrant.com 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 t...@sss.pgh.pa.us wrote: Andres Freund and...@2ndquadrant.com 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] printing table in asciidoc with psql
On Sun, Mar 22, 2015 at 10:09 AM, Bruce Momjian br...@momjian.us 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,l,l,l,frame=none] | ^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
On March 22, 2015 12:17:57 PM GMT+01:00, Christoph Berg c...@df7cb.de 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] PATCH: numeric timestamp in log_line_prefix
On 22 March 2015 at 12:47, Tomas Vondra tomas.von...@2ndquadrant.com 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 %paddingchar 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] Zero-padding and zero-masking fixes for to_char(float)
On Sun, Mar 22, 2015 at 01:42:56AM -0400, Tom Lane wrote:
David Rowley dgrowle...@gmail.com 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 br...@momjian.ushttp://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?
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] Zero-padding and zero-masking fixes for to_char(float)
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] INT64_MIN and _MAX
Andres == Andres Freund and...@anarazel.de 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 limits.h
+
#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 sys/resource.h /* 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 limits.h
+
#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/timestamp.c
Re: [HACKERS] Order of enforcement of CHECK constraints?
On Sunday, March 22, 2015, David Steele da...@pgmasters.net wrote: On 3/20/15 3:37 PM, Tom Lane wrote: =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= fabriziome...@gmail.com javascript:; writes: On Fri, Mar 20, 2015 at 4:19 PM, Peter Geoghegan p...@heroku.com javascript:; wrote: On Fri, Mar 20, 2015 at 12:15 PM, Tom Lane t...@sss.pgh.pa.us javascript:; 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 javascript:; 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] PATCH: numeric timestamp in log_line_prefix
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] Remove fsync ON/OFF as a visible option?
* 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
Etsuro Fujita fujita.ets...@lab.ntt.co.jp 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] 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] PATCH: numeric timestamp in log_line_prefix
Fabien COELHO coe...@cri.ensmp.fr 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
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] Zero-padding and zero-masking fixes for to_char(float)
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
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=jacanadt=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] Patch: Add launchd Support
On Mar 20, 2015, at 4:11 PM, David E. Wheeler da...@justatheory.com 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] PATCH: numeric timestamp in log_line_prefix
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
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] PATCH: numeric timestamp in log_line_prefix
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
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] PATCH: numeric timestamp in log_line_prefix
On Sat, Mar 21, 2015 at 4:47 PM, Tomas Vondra tomas.von...@2ndquadrant.com 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] Remove fsync ON/OFF as a visible option?
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] Using 128-bit integers for sum, avg and statistics aggregates
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=jacanadt=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
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] Abbreviated keys for Numeric
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. In both cases
+ * the
Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates
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=jacanadt=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=jacanadt=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] inherit support for foreign tables
On Sun, Mar 22, 2015 at 1:57 PM, Tom Lane t...@sss.pgh.pa.us wrote: Etsuro Fujita fujita.ets...@lab.ntt.co.jp 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] Remove fsync ON/OFF as a visible option?
On Sunday, March 22, 2015, Florian Weimer f...@deneb.enyo.de 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] Using 128-bit integers for sum, avg and statistics aggregates
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] [PATCH] Add transforms feature
2015-03-22 5:45 GMT+01:00 Pavel Stehule pavel.steh...@gmail.com: 2015-03-22 3:55 GMT+01:00 Peter Eisentraut pete...@gmx.net: 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 pete...@gmx.net mailto:pete...@gmx.net: 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
On Sun, Mar 22, 2015 at 6:22 PM, Andres Freund and...@2ndquadrant.com 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 t...@sss.pgh.pa.us wrote: Andres Freund and...@2ndquadrant.com 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=jacanadt=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] Remove fsync ON/OFF as a visible option?
On 22/03/15 08:14, Jaime Casanova wrote: El mar 21, 2015 2:00 AM, Mark Kirkwood mark.kirkw...@catalyst.net.nz 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] proposal: searching in array function - array_position
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
Regards
Pavel
2015-03-20 18:29 GMT+01:00 Alvaro Herrera alvhe...@2ndquadrant.com:
Pavel Stehule wrote:
2015-03-20 17:49 GMT+01:00 Dean Rasheed dean.a.rash...@gmail.com:
There's an issue when the array's lower bound isn't 1:
select array_offset('[2:4]={1,2,3}'::int[], 1);
array_offset
--
1
(1 row)
whereas I would expect this to return 2. Similarly for
array_offsets(), so the offsets can be used as indexes into the
original array.
I am thinking, so it is ok - it returns a offset, not position.
So you can't use it as a subscript? That sounds unfriendly. Almost
every function using this will be subtly broken.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
diff --git a/doc/src/sgml/array.sgml b/doc/src/sgml/array.sgml
new file mode 100644
index 9ea1068..85dc647
*** a/doc/src/sgml/array.sgml
--- b/doc/src/sgml/array.sgml
*** SELECT * FROM sal_emp WHERE pay_by_quart
*** 600,605
--- 600,624
index, as described in xref linkend=indexes-types.
/para
+ para
+ You can also search for specific values in an array using the functionarray_position/
+ and functionarray_positions/ functions. The former returns the position of
+ the first occurrence of a value in an array; the latter returns an array with the
+ positions of all occurrences of the value in the array. For example:
+
+ programlisting
+ 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}
+ /programlisting
+ /para
+
tip
para
Arrays are not sets; searching for specific array elements
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
new file mode 100644
index c89f343..c865f30
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
*** SELECT NULLIF(value, '(none)') ...
*** 11480,11485
--- 11480,11491
primaryarray_lower/primary
/indexterm
indexterm
+ primaryarray_position/primary
+ /indexterm
+ indexterm
+ primaryarray_positions/primary
+ /indexterm
+ indexterm
primaryarray_prepend/primary
/indexterm
indexterm
*** SELECT NULLIF(value, '(none)') ...
*** 11599,11604
--- 11605,11636
row
entry
literal
+ functionarray_position/function(typeanyarray/type, typeanyelement/type optional, typeint/type/optional)
+ /literal
+ /entry
+ entrytypeint/type/entry
+ entryreturns the position 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)/entry
+ entryliteralarray_position(ARRAY['sun','mon','tue','wed','thu','fri','sat'], 'mon')/literal/entry
+ entryliteral2/literal/entry
+/row
+row
+ entry
+ literal
+ functionarray_positions/function(typeanyarray/type, typeanyelement/type)
+ /literal
+ /entry
+ entrytypeint[]/type/entry
+ entryreturns an array of positions of all occurrences of the second
+ argument in the array given as first argument (array must be
+ one-dimensional)/entry
+ entryliteralarray_positions(ARRAY['A','A','B','A'], 'A')/literal/entry
+ entryliteral{1,2,4}/literal/entry
+/row
+row
+ entry
+ literal
functionarray_prepend/function(typeanyelement/type, typeanyarray/type)
/literal
/entry
*** NULL baz/literallayout(3 rows)/entry
*** 11708,11713
--- 11740,11762
/table
para
+ In functionarray_position/function and functionarray_positions/,
+ each array element is compared to the searched value using
+ literalIS NOT DISTINCT FROM/literal semantics.
+/para
+
+para
+ In functionarray_position/function, literalNULL/literal is returned
+ if the value is not found.
+/para
+
+para
+ In functionarray_positions/function, literalNULL/literal is returned
+ only if the array is literalNULL/literal; if the value is not found in
+ the array, an empty array is returned instead.
+/para
+
+para
In functionstring_to_array/function, 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
Re: [HACKERS] Performance improvement for joins where outer side is unique
On 20 March 2015 at 21:11, David Rowley dgrowle...@gmail.com wrote: I can continue working on your patch if you like? Or are you planning to go further with it? I've been working on this more over the weekend and I've re-factored things to allow LEFT JOINs to be properly marked as unique. I've also made changes to re-add support for detecting the uniqueness of sub-queries. Also, I've added modified the costing for hash and nested loop joins to reduce the cost for unique inner joins to cost the join the same as it does for SEMI joins. This has tipped the scales on a few plans in the regression tests. Also, please see attached unijoin_analysis.patch. This just adds some code which spouts out notices when join nodes are initialised which states if the join is unique or not. Running the regression tests with this patch in places gives: Unique Inner: Yes == 753 hits Unique Inner: No == 1430 hits So it seems we can increase the speed of about 1 third of joins by about 10%. A quick scan of the Nos seems to show quite a few cases which do not look that real world like. e.g cartesian join. It would be great if someone could run some PostgreSQL application with these 2 patches applied, and then grep the logs for the Unique Inner results... Just to get a better idea of how many joins in a real world case will benefit from this patch. Regards David Rowley unijoin_2015-03-22_87bc41e.patch Description: Binary data unijoin_analysis.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] barnacle (running CLOBBER_CACHE_RECURSIVELY) seems stuck since November
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
Re: [HACKERS] Abbreviated keys for Numeric
On Sat, Mar 21, 2015 at 3:33 PM, Andrew Gierth and...@tao11.riddles.org.uk 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: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)
On Wed, Mar 18, 2015 at 9:33 AM, Kouhei Kaigai kai...@ak.jp.nec.com wrote: On Wed, Mar 18, 2015 at 2:34 AM, Kouhei Kaigai kai...@ak.jp.nec.com 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, t4) and t2. The earlier one is also joinrel, so it expects FDW driver will make the
Re: [HACKERS] debug_sortsupport GUC?
On Sat, Mar 21, 2015 at 8:28 PM, Andrew Gierth and...@tao11.riddles.org.uk 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] Abbreviated keys for Numeric
On Fri, Mar 20, 2015 at 11:58 PM, Andrew Gierth and...@tao11.riddles.org.uk 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: [HACKERS] Parallel Seq Scan
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
Peter == Peter Geoghegan p...@heroku.com 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 reflection, I am inclined to
Re: [HACKERS] Abbreviated keys for Numeric
Peter == Peter Geoghegan p...@heroku.com 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?
Peter == Peter Geoghegan p...@heroku.com 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
On Sun, Mar 22, 2015 at 6:48 PM, Andrew Gierth and...@tao11.riddles.org.uk 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?
On Sun, Mar 22, 2015 at 6:59 PM, Andrew Gierth and...@tao11.riddles.org.uk 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)
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 br...@momjian.ushttp://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)
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 br...@momjian.ushttp://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
On 17 March 2015 at 19:12, Robert Haas robertmh...@gmail.com wrote: On Tue, Mar 17, 2015 at 1:33 AM, Amit Khandekar amitdkhan...@gmail.com 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)
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 br...@momjian.ushttp://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
On Mon, Mar 23, 2015 at 1:48 AM, Воронин Дмитрий carriingfat...@yandex.ru 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] Table-level log_autovacuum_min_duration
On Thu, Mar 19, 2015 at 1:43 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Thu, Mar 19, 2015 at 12:40 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Thu, Mar 19, 2015 at 12:23 PM, Fujii Masao masao.fu...@gmail.com 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] Zero-padding and zero-masking fixes for to_char(float)
Bruce Momjian br...@momjian.us 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 ?
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] Display of multi-target-table Modify plan nodes in EXPLAIN
On Sun, Mar 22, 2015 at 6:32 AM, Tom Lane t...@sss.pgh.pa.us 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://www.postgresql.org/mailpref/pgsql-hackers -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The
Re: [HACKERS] Display of multi-target-table Modify plan nodes in EXPLAIN
Ashutosh Bapat ashutosh.ba...@enterprisedb.com writes: On Sun, Mar 22, 2015 at 6:32 AM, Tom Lane t...@sss.pgh.pa.us 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?
I might be only one objecting here but ... On Sat, Mar 21, 2015 at 12:45 AM, Tom Lane t...@sss.pgh.pa.us 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
On Mon, Mar 23, 2015 at 1:54 PM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Mar 19, 2015 at 1:43 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Thu, Mar 19, 2015 at 12:40 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Thu, Mar 19, 2015 at 12:23 PM, Fujii Masao masao.fu...@gmail.com 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
On Mon, Mar 23, 2015 at 10:51 AM, Tom Lane t...@sss.pgh.pa.us wrote: Ashutosh Bapat ashutosh.ba...@enterprisedb.com writes: On Sun, Mar 22, 2015 at 6:32 AM, Tom Lane t...@sss.pgh.pa.us 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)
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
2015-01-28 0:13 GMT+01:00 Jim Nasby jim.na...@bluetreble.com:
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 pavel.steh...@gooddata.com
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'
variablelist
+ varlistentry id=guc-enable-user-asserts xreflabel=enable_user_asserts
+ termvarnameenable_user_asserts/varname (typeboolean/type)
+ indexterm
+ primaryvarnameenable_user_asserts/ configuration parameter/primary
+ /indexterm
+ /term
+ listitem
+ para
+If true, any user assertions are evaluated. By default, this
+is set to true.
+ /para
+ /listitem
+ /varlistentry
+
varlistentry id=guc-exit-on-error xreflabel=exit_on_error
termvarnameexit_on_error/varname (typeboolean/type)
indexterm
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 optional replaceablelabel/replaceable /optional;
sect1 id=plpgsql-errors-and-messages
titleErrors and Messages/title
+ sect2 id=plpgsql-statements-raise
+titleRAISE statement/title
+
indexterm
primaryRAISE/primary
/indexterm
@@ -3565,7 +3568,33 @@ RAISE unique_violation USING MESSAGE = 'Duplicate user ID: ' || user_id;
the whole category.
/para
/note
+ /sect2
+
+ sect2 id=plpgsql-statements-assert
+titleASSERT statement/title
+ indexterm
+primaryASSERT/primary
+ /indexterm
+
+ indexterm
+primaryassertions/primary
+secondaryin PL/pgSQL/secondary
+ /indexterm
+
+ para
+Use the commandASSERT/command statement to ensure the
+predicate is allways true. If the predicate is false or is null,
+then a assertion exception is raised.
+
+synopsis
+ASSERT replaceable class=parameterexpression/replaceable optional, replaceable class=parametermessage expression/replaceable /optional;
+/synopsis
+
+The user assertions can be enabled or disabled via
+xref linkend=guc-enable-user-asserts.
+ /para
+ /sect2
/sect1
sect1 id=plpgsql-trigger
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
