Re: [HACKERS] PATCH: index-only scans with partial indexes

[Simon Riggs]

On 4 September 2015 at 22:03, Tomas Vondra 
wrote:


> A summary of 100 EXPLAIN timings looks like this:
>
>
> master   A  B  C  D  E  F
> -
> min0.10   0.10   0.30   0.29   0.66   0.23
> max1.07   1.00   2.13   1.98   4.52   1.59
> median 0.49   0.52   0.31   0.33   0.68   1.12
> average0.43   0.35   0.62   0.49   1.01   0.89
>

What are these? Times? in ms?


> However that was the idea of the benchmark, and I got no difference.
>

Please explain what this means and your conclusion, so its clear. That way
we can either reject the patch or commit it. Thanks

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

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

[HACKERS] jsonb_concat: make sure we always return a non-scalar value

[Oskari Saarenmaa]

There was a long thread about concatenating jsonb objects to each other, 
but that discussion didn't touch concatenating other types.  Currently 
jsonb_concat always just returns the other argument if one of arguments 
is considered empty.  This causes surprising behavior when concatenating 
scalar values to empty arrays:


os=# select '[]'::jsonb || '1'::jsonb;
1

os=# select '[]'::jsonb || '[1]'::jsonb;
 [1]

os=# select '[]'::jsonb || '1'::jsonb || '2'::jsonb;
 [1, 2]

os=# select '0'::jsonb || '1'::jsonb;
 [0, 1]

os=# select '{"x": "y"}'::jsonb || '[1]'::jsonb;
 [{"x": "y"}, 1]

os=# select '{"x": "y"}'::jsonb || '1'::jsonb;
ERROR:  invalid concatenation of jsonb objects

Attached a patch to fix and test this.  Also added a test case for
concatenating two scalar values which currently produces an array.. I'm 
not sure that behavior makes sense, but didn't want to change that in 
this patch as I guess someone could consider that feature useful.


/ Oskari
>From 299124e63bb26ab07fa8429b7d1c2035b81f15d5 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa 
Date: Sat, 5 Sep 2015 09:33:58 +0300
Subject: [PATCH] jsonb_concat: make sure we always return a non-scalar value

jsonb_concat used to always just return the other argument if one of
arguments was considered empty.  This caused surprising behavior when
concatenating scalar values to empty arrays.

Fixed this and added a test case for it.  Also added a test case for
concatenating two scalar values which currently produces an array.
---
 src/backend/utils/adt/jsonfuncs.c   |  8 +---
 src/test/regress/expected/jsonb.out | 18 ++
 src/test/regress/sql/jsonb.sql  |  4 
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 3b8d42e..57edb63 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -3359,11 +3359,13 @@ jsonb_concat(PG_FUNCTION_ARGS)
 			   *it2;
 
 	/*
-	 * If one of the jsonb is empty, just return other.
+	 * If one of the jsonb is empty, just return other if it's not
+	 * scalar.  If it's a scalar we need to perform concatenation to
+	 * make sure we return a non-scalar value.
 	 */
-	if (JB_ROOT_COUNT(jb1) == 0)
+	if (JB_ROOT_COUNT(jb1) == 0 && !JB_ROOT_IS_SCALAR(jb2))
 		PG_RETURN_JSONB(jb2);
-	else if (JB_ROOT_COUNT(jb2) == 0)
+	else if (JB_ROOT_COUNT(jb2) == 0 && !JB_ROOT_IS_SCALAR(jb1))
 		PG_RETURN_JSONB(jb1);
 
 	it1 = JsonbIteratorInit(>root);
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index 17656d4..98a318b 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -2912,6 +2912,24 @@ select '"c"' || '["a", "b"]'::jsonb;
  ["c", "a", "b"]
 (1 row)
 
+select '[]'::jsonb || '["a"]'::jsonb;
+ ?column? 
+--
+ ["a"]
+(1 row)
+
+select '[]'::jsonb || '"a"'::jsonb;
+ ?column? 
+--
+ ["a"]
+(1 row)
+
+select '"b"'::jsonb || '"a"'::jsonb;
+  ?column?  
+
+ ["b", "a"]
+(1 row)
+
 select '"a"'::jsonb || '{"a":1}';
 ERROR:  invalid concatenation of jsonb objects
 select '{"a":1}' || '"a"'::jsonb;
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 83ed4eb..5a2b4a8 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -718,6 +718,10 @@ select '["c"]' || '["a", "b"]'::jsonb;
 select '["a", "b"]'::jsonb || '"c"';
 select '"c"' || '["a", "b"]'::jsonb;
 
+select '[]'::jsonb || '["a"]'::jsonb;
+select '[]'::jsonb || '"a"'::jsonb;
+select '"b"'::jsonb || '"a"'::jsonb;
+
 select '"a"'::jsonb || '{"a":1}';
 select '{"a":1}' || '"a"'::jsonb;
 
-- 
2.5.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] checkpointer continuous flushing

[Fabien COELHO]

I would be curious whether flushing helps, though.


Yes, me too. I think we should try to reach on consensus for exact 
scenarios and configuration where this patch('es) can give benefit or we 
want to verify if there is any regression as I have access to this m/c for 
a very-very limited time.  This m/c might get formatted soon for some other 
purpose.


Yep, it would be great if you have time for a flush test before it 
disappears... I think it is advisable to disable the write cache as it may 
also hide the impact of flushing.


Still thinking... Depending on the results, it might be interesting to 
have these tests run with the write cache enabled as well, to check how 
much it interferes positively with performance.


I would guess "quite a lot".

--
Fabien.


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

[HACKERS] misc typofixes

[Oskari Saarenmaa]

Attached a patch to fix a number of typos in docs and comments I noticed 
over the past few weeks.


/ Oskari

>From dd5cb727d7fc1f1168040f502521a897d01cc127 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa 
Date: Sat, 5 Sep 2015 10:12:04 +0300
Subject: [PATCH] typofixes

---
 contrib/btree_gist/btree_ts.c| 2 +-
 contrib/btree_gist/btree_utils_var.c | 2 +-
 contrib/cube/cube.c  | 2 +-
 doc/src/sgml/ref/alter_role.sgml | 4 ++--
 doc/src/sgml/release-9.5.sgml| 4 ++--
 doc/src/sgml/sources.sgml| 2 +-
 src/backend/access/brin/brin_revmap.c| 4 ++--
 src/backend/access/common/heaptuple.c| 2 +-
 src/backend/access/gin/ginfast.c | 4 ++--
 src/backend/access/gist/gistproc.c   | 4 ++--
 src/backend/access/heap/heapam.c | 4 ++--
 src/backend/access/heap/rewriteheap.c| 4 ++--
 src/backend/access/transam/xact.c| 2 +-
 src/backend/optimizer/path/costsize.c| 2 +-
 src/backend/replication/logical/origin.c | 2 +-
 src/backend/utils/adt/regproc.c  | 2 +-
 src/include/storage/lwlock.h | 4 ++--
 17 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/contrib/btree_gist/btree_ts.c b/contrib/btree_gist/btree_ts.c
index c746c23..ab22b27 100644
--- a/contrib/btree_gist/btree_ts.c
+++ b/contrib/btree_gist/btree_ts.c
@@ -369,7 +369,7 @@ gbt_ts_penalty(PG_FUNCTION_ARGS)
 newdbl[2];
 
 	/*
-	 * We are allways using "double" timestamps here. Precision should be good
+	 * We are always using "double" timestamps here. Precision should be good
 	 * enough.
 	 */
 	orgdbl[0] = ((double) origentry->lower);
diff --git a/contrib/btree_gist/btree_utils_var.c b/contrib/btree_gist/btree_utils_var.c
index 8105a3b..70b3794 100644
--- a/contrib/btree_gist/btree_utils_var.c
+++ b/contrib/btree_gist/btree_utils_var.c
@@ -52,7 +52,7 @@ gbt_var_decompress(PG_FUNCTION_ARGS)
 	PG_RETURN_POINTER(entry);
 }
 
-/* Returns a better readable representaion of variable key ( sets pointer ) */
+/* Returns a better readable representation of variable key ( sets pointer ) */
 GBT_VARKEY_R
 gbt_var_key_readable(const GBT_VARKEY *k)
 {
diff --git a/contrib/cube/cube.c b/contrib/cube/cube.c
index 113c663..a6be59e 100644
--- a/contrib/cube/cube.c
+++ b/contrib/cube/cube.c
@@ -814,7 +814,7 @@ cube_inter(PG_FUNCTION_ARGS)
 	Max(LL_COORD(b, i), UR_COORD(b, i))
 			);
 	}
-	/* continue on the higher dimemsions only present in 'a' */
+	/* continue on the higher dimensions only present in 'a' */
 	for (; i < DIM(a); i++)
 	{
 		result->x[i] = Max(0,
diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.sgml
index e97bf4c..7638817 100644
--- a/doc/src/sgml/ref/alter_role.sgml
+++ b/doc/src/sgml/ref/alter_role.sgml
@@ -138,7 +138,7 @@ ALTER ROLE { role_specification | A
   CURRENT_USER
   

-Alter the current user instead of an explicitely identified role.
+Alter the current user instead of an explicitly identified role.

   
  
@@ -147,7 +147,7 @@ ALTER ROLE { role_specification | A
   SESSION_USER
   

-Alter the current session user instead of an explicitely identified
+Alter the current session user instead of an explicitly identified
 role.

   
diff --git a/doc/src/sgml/release-9.5.sgml b/doc/src/sgml/release-9.5.sgml
index a535e22..59f10f0 100644
--- a/doc/src/sgml/release-9.5.sgml
+++ b/doc/src/sgml/release-9.5.sgml
@@ -460,7 +460,7 @@ FIXME: Add Andres
 
-Speed up CREATE INDEX by avoiding unneccessary memory copies (Robert Haas)
+Speed up CREATE INDEX by avoiding unnecessary memory copies (Robert Haas)

   
 
@@ -,7 +,7 @@ FIXME: Correct description
 -->

 Allow changing of the WAL
-logging status of a table ater creation with ALTER TABLE .. SET LOGGED /
 UNLOGGED (Fabrzio de Royes Mello)

diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml
index aa20807..d6461ec 100644
--- a/doc/src/sgml/sources.sgml
+++ b/doc/src/sgml/sources.sgml
@@ -251,7 +251,7 @@ ereport(ERROR,


 
- errdetail_log_plural(const char *fmt_singuar, const char
+ errdetail_log_plural(const char *fmt_singular, const char
  *fmt_plural, unsigned long n, ...) is like
  errdetail_log, but with support for various plural forms of
  the message.
diff --git a/src/backend/access/brin/brin_revmap.c b/src/backend/access/brin/brin_revmap.c
index 62d440f..8c55f16 100644
--- a/src/backend/access/brin/brin_revmap.c
+++ b/src/backend/access/brin/brin_revmap.c
@@ -127,7 +127,7 @@ brinRevmapExtend(BrinRevmap *revmap, BlockNumber heapBlk)
  * it's not long enough.
  *
  * The returned buffer is also recorded in the revmap struct; finishing that
- * releases the buffer, therefore the caller needn't do it explicitely.
+ * releases the buffer, therefore the caller needn't do it explicitly.
  */
 Buffer
 

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

[Jim Nasby]

On 9/4/15 12:41 PM, Tom Lane wrote:

I wrote:

After review of all the callers of RelationClearRelation, it seems like
most of them already have sufficient guards to prevent triggering of the
Assert.  The ones that lack such tests are AtEOXact_cleanup and
AtEOSubXact_cleanup.  So what I'm now thinking is that those should do
something along the lines of



Specifically, this, which can be shown to mitigate the results of the
problem cases in an otherwise-unpatched build.


And pushed.  I noticed that while the relcache.c fix mitigates the error
pretty well in 9.3 and up, in the older branches you still end up with
a PANIC due to error stack overflow.  This may indicate that there's
some patch we'd have been better off back-porting.  However, with the
Portal changes in place the test cases work in all branches, so I'm
not excited enough to pursue the point further myself.


I was about to test this and was verifying that the crash still worked 
when I noticed this in the logs (9.4.1, not HEAD). Not sure if there's 
any realion here or not...


WARNING:  relation "pg_proc" page 121 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 122 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 123 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 124 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 125 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 126 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 127 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 128 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 129 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 130 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 131 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 132 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 133 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 134 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 135 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 136 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 137 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 138 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 139 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 140 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 141 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 142 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 143 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 144 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 145 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 146 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 147 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 148 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 149 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 150 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 151 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 152 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 153 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 154 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 155 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 156 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 157 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 158 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 159 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 160 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 161 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 162 is uninitialized --- fixing
WARNING:  relation "pg_proc" page 246 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 82 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 83 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 84 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 85 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 86 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 87 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 88 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 89 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 90 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 91 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 92 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 93 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 94 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 95 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 96 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 112 is uninitialized --- fixing
WARNING:  relation 

Re: [HACKERS] track_commit_timestamp and COMMIT PREPARED

[Petr Jelinek]

On 2015-09-02 16:14, Fujii Masao wrote:

On Wed, Aug 5, 2015 at 2:16 AM, Robert Haas  wrote:

On Mon, Aug 3, 2015 at 10:31 AM, Fujii Masao  wrote:

track_commit_timestamp tracks COMMIT PREPARED as expected in standby server,
but not in master server. Is this intentional? It should track COMMIT PREPARED
even in master? Otherwise, we cannot use commit_timestamp feature to check
the replication lag properly while we use 2PC.


That sounds like it must be a bug.  I think you should add it to the
open items list.




Attached fixes this. It includes advancement of replication origin as 
well. I didn't feel like doing refactor of commit code this late in 9.5 
cycle though, so I went with the code duplication + note in xact.c.


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index d48d101..95da328 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -41,6 +41,7 @@
 #include 
 #include 
 
+#include "access/commit_ts.h"
 #include "access/htup_details.h"
 #include "access/subtrans.h"
 #include "access/transam.h"
@@ -56,6 +57,7 @@
 #include "miscadmin.h"
 #include "pg_trace.h"
 #include "pgstat.h"
+#include "replication/origin.h"
 #include "replication/walsender.h"
 #include "replication/syncrep.h"
 #include "storage/fd.h"
@@ -2087,6 +2089,7 @@ RecordTransactionCommitPrepared(TransactionId xid,
 bool initfileinval)
 {
 	XLogRecPtr	recptr;
+	TimestampTz	committs = GetCurrentTimestamp();
 
 	START_CRIT_SECTION();
 
@@ -2094,13 +2097,34 @@ RecordTransactionCommitPrepared(TransactionId xid,
 	MyPgXact->delayChkpt = true;
 
 	/* Emit the XLOG commit record */
-	recptr = XactLogCommitRecord(GetCurrentTimestamp(),
+	recptr = XactLogCommitRecord(committs,
  nchildren, children, nrels, rels,
  ninvalmsgs, invalmsgs,
  initfileinval, false,
  xid);
 
 	/*
+	 * Record plain commit ts if not replaying remote actions, or if no
+	 * timestamp is configured.
+	 */
+	if (replorigin_sesssion_origin == InvalidRepOriginId ||
+		replorigin_sesssion_origin == DoNotReplicateId ||
+		replorigin_sesssion_origin_timestamp == 0)
+		replorigin_sesssion_origin_timestamp = committs;
+	else
+		replorigin_session_advance(replorigin_sesssion_origin_lsn,
+   XactLastRecEnd);
+
+	/*
+	 * We don't need to WAL log origin or timestamp here, the commit
+	 * record contains all the necessary information and will redo the SET
+	 * action during replay.
+	 */
+	TransactionTreeSetCommitTsData(xid, nchildren, children,
+   replorigin_sesssion_origin_timestamp,
+   replorigin_sesssion_origin, false);
+
+	/*
 	 * We don't currently try to sleep before flush here ... nor is there any
 	 * support for async commit of a prepared xact (the very idea is probably
 	 * a contradiction)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index b53d95f..6ef876d 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1119,6 +1119,9 @@ AtSubStart_ResourceOwner(void)
  *
  * Returns latest XID among xact and its children, or InvalidTransactionId
  * if the xact has no XID.  (We compute that here just because it's easier.)
+ *
+ * Note: if you change this functions you should also look at
+ * RecordTransactionCommitPrepared in twophase.c.
  */
 static TransactionId
 RecordTransactionCommit(void)

-- 
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] misc typofixes

[Heikki Linnakangas]

On 09/05/2015 10:16 AM, Oskari Saarenmaa wrote:

Attached a patch to fix a number of typos in docs and comments I noticed
over the past few weeks.


Thanks, applied!

- Heikki



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

Re: [HACKERS] Is this a bug?

[Bruce Momjian]

On Fri, Sep  4, 2015 at 09:40:10AM +0900, Michael Paquier wrote:
> Robert Haas wrote:
> > On Wed, Aug 26, 2015 at 3:31 PM, Alvaro Herrera
> >  wrote:
> > > Fabrízio de Royes Mello wrote:
> > >
> > >> Why this patch was reverted one day after applied [1]? I didn't see
> any
> > >> discussion around it.
> > >
> > > Contributors whose patches are getting committed should really
> subscribe
> > > to pgsql-committers.
> >
> > I would have thought discussion of committed patches should be moved
> > to -hackers.
> 
> I agree, but it happens anyway quite frequently.  Since recently, I make
> a point of changing the CC from -committers to -hackers, but due to
> force of habit I often forget.
> 
>  
> Noted. I usually don't do that.

I am thinking we should all agree if we should redirect commit comments
to hackers, or not, so we are consistent.

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

  + Everyone has their own god. +


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

Re: [HACKERS] checkpointer continuous flushing

[Fabien COELHO]

Hello Amit,


Woops 14 GB/s and 1.2 GB/s?! Is this a *hard* disk??


Yes, there is no SSD in system. I have confirmed the same.  There are RAID
spinning drives.


Ok...

I guess that there is some kind of cache to explain these great tps 
figures, probably on the RAID controller. What does "lspci" says? Does 
"hdparm" suggests that the write cache is enabled? It would be fine if the 
I/O system has a BBU, but that could also hide some of the patch 
benefits...


A tentative explanation for the similar figures with and without sorting 
could be that depending on the controller cache size (may be 1GB or more) 
and firmware, the I/O system reorders disk writes so that they are 
basically sequential and the fact that pg sorts them beforehand has little 
or no impact. This may also be help by the fact that buffers are not 
really in random order to begin with as the warmup phase does an initial 
"select stuff from table".


There could be other possible factors such as the file system details, 
"WAFL" hacks... the tricks are endless:-)


Checking for the right explanation would involve removing the 
unconditional select warmup to use only a long and random warmup, and 
probably trying a much larger than cache database, and/or disabling the 
write cache, reading the hardware documentation in detail... But this is 
also a lot of bother and time.


Maybe the simplest approach would be to disable the write cache for the 
test. Is that possible?



Woops, 1.6 GB/s write... same questions, "rotating plates"??


One thing to notice is that if I don't remove the output file 
(output.img) the speed is much slower, see the below output. I think 
this means in our case we will get ~320 MB/s


I would say that the OS was doing something here, and 320 MB/s looks more 
like an actual HDD RAID system sequential write performance.



If these are SSD, or if there is some SSD cache on top of the HDD, I would
not expect the patch to do much, because the SSD random I/O writes are
pretty comparable to sequential I/O writes.

I would be curious whether flushing helps, though.


Yes, me too. I think we should try to reach on consensus for exact 
scenarios and configuration where this patch('es) can give benefit or we 
want to verify if there is any regression as I have access to this m/c 
for a very-very limited time.  This m/c might get formatted soon for 
some other purpose.


Yep, it would be great if you have time for a flush test before it 
disappears... I think it is advisable to disable the write cache as it may 
also hide the impact of flushing.



So whether the database fits in 8 GB shared buffer during the 2 hours of
the pgbench run is an open question.


With this kind of configuration, I have noticed that more than 80%
of updates are HOT updates, not much bloat, so I think it won't
cross 8GB limit, but still I can keep it to 32GB if you have any doubts.


The problem with performance tests is that you want to test one thing, but 
there are many factors that intervene and you may end up testing something 
else, such as lock contention or process scheduler or whatever, rather 
than what you were trying to put in evidence. So I would suggest to be on 
the safe side and use the larger value.


--
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] Fwd: Core dump with nested CREATE TEMP TABLE

[Michael Paquier]

On Sat, Sep 5, 2015 at 3:41 PM, Jim Nasby  wrote:

> I was about to test this and was verifying that the crash still worked
> when I noticed this in the logs (9.4.1, not HEAD). Not sure if there's any
> realion here or not...
>
> WARNING:  relation "pg_proc" page 121 is uninitialized --- fixing
> WARNING:  relation "pg_proc" page 122 is uninitialized --- fixing
>

[reading vacuumlazy.c...] This seems unrelated and I would not worry about
it. Those system catalogs have been extended with new pages by a couple of
backends, but a crash happened before they could actually insert tuples on
it and commit. Perhaps you were creating a bunch of objects when a crash
happened, no?

Coming to the point, did you see a new crash with test_factory? Is that
some remnant of a previous test?
-- 
Michael

Re: [HACKERS] [PATCH] SQL function to report log message

[dinesh kumar]

On Fri, Sep 4, 2015 at 2:03 PM, Pavel Stehule 
wrote:

>
>
> 2015-09-02 21:49 GMT+02:00 dinesh kumar :
>
>> On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule 
>> wrote:
>>
>>>
>>>
>>> 2015-09-01 6:59 GMT+02:00 Pavel Stehule :
>>>


 2015-08-31 20:43 GMT+02:00 dinesh kumar :

> Hi,
>
> On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule <
> pavel.steh...@gmail.com> wrote:
>
>> Hi
>>
>> I am starting to work review of this patch
>>
>> 2015-07-13 9:54 GMT+02:00 dinesh kumar :
>>
>>> Hi All,
>>>
>>> Greetings for the day.
>>>
>>> Would like to discuss on below feature here.
>>>
>>> Feature:
>>> Having an SQL function, to write messages to log destination.
>>>
>>> Justification:
>>> As of now, we don't have an SQL function to write
>>> custom/application messages to log destination. We have "RAISE" clause
>>> which is controlled by
>>> log_ parameters. If we have an SQL function which works irrespective
>>> of log settings, that would be a good for many log parsers. What i mean 
>>> is,
>>> in DBA point of view, if we route all our native OS stats to log files 
>>> in a
>>> proper format, then we can have our log reporting tools to give most
>>> effective reports. Also, Applications can log their own messages to
>>> postgres log files, which can be monitored by DBAs too.
>>>
>>> Implementation:
>>> Implemented a new function "pg_report_log" which takes one
>>> argument as text, and returns void. I took, "LOG" prefix for all the
>>> reporting messages.I wasn't sure to go with new prefix for this, since
>>> these are normal LOG messages. Let me know, if i am wrong here.
>>>
>>> Here is the attached patch.
>>>
>>
>> This patch is not complex, but the implementation doesn't cover a
>> "ereport" well.
>>
>> Although this functionality should be replaced by custom function in
>> any PL (now or near future), I am not against to have this function in
>> core. There are lot of companies with strong resistance against stored
>> procedures - and sometimes this functionality can help with SQL 
>> debugging.
>>
>> Issues:
>>
>> 1. Support only MESSAGE field in exception - I am expecting to
>> support all fields: HINT, DETAIL, ...
>>
>
> Added these functionalities too.
>
>
>> 2. Missing regress tests
>>
>
> Adding here.
>
>
>> 3. the parsing ereport level should be public function shared with
>> PLpgSQL and other PL
>>
>
> Sorry Pavel. I am not getting your point here. Would you give me an
> example.
>

 The transformation: text -> error level is common task - and PLpgSQL it
 does in pl_gram.y. My idea is to add new function to error utils named
 "parse_error_level" and use it from PLpgSQL and from your code.


>
>
>> 4. should be hidestmt mandatory parameter?
>>
>
> I changed this argument's default value as "true".
>
>
>> 5. the function declaration is strange
>>
>> postgres=# \sf pg_report_log (text, anyelement, boolean)
>> CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, anyelement,
>> boolean)
>>  RETURNS void
>>  LANGUAGE sql
>>  STABLE STRICT COST 1
>> AS $function$SELECT pg_report_log($1::pg_catalog.text,
>> $2::pg_catalog.text, $3::boolean)$function$
>>
>> Why polymorphic? It is useless on any modern release
>>
>>
> I took quote_ident(anyelement) as referral code, for implementing
> this. Could you guide me if I am doing wrong here.
>

 I was wrong - this is ok - it is emulation of force casting to text


>
>
>> postgres=# \sf pg_report_log (text, text, boolean)
>> CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, text,
>> boolean)
>>  RETURNS void
>>  LANGUAGE internal
>>  IMMUTABLE STRICT
>> AS $function$pg_report_log$function$
>>
>> Why stable, why immutable? This function should be volatile.
>>
>> Fixed these to volatile.
>
>
>> 6. using elog level enum as errcode is wrong idea - errcodes are
>> defined in table
>> http://www.postgresql.org/docs/9.4/static/errcodes-appendix.html
>>
>
> You mean, if the elevel is 'ERROR', then we need to allow errcode. Let
> me know your inputs.
>

 I was blind, but the code was not good. Yes, error and higher needs
 error code. From ANSI/SQL anything can has error code "00 is ok", "01 ..
 warnings" ...

 There is more possibilities - look to PLpgSQL implementation - it can
 be optional parameter - it default can use ERRCODE_RAISE_EXCEPTION

Re: [HACKERS] [PATCH] SQL function to report log message

[Pavel Stehule]

2015-09-05 8:35 GMT+02:00 dinesh kumar :

>
>
> On Fri, Sep 4, 2015 at 2:03 PM, Pavel Stehule 
> wrote:
>
>>
>>
>> 2015-09-02 21:49 GMT+02:00 dinesh kumar :
>>
>>> On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule >> > wrote:
>>>


 2015-09-01 6:59 GMT+02:00 Pavel Stehule :

>
>
> 2015-08-31 20:43 GMT+02:00 dinesh kumar :
>
>> Hi,
>>
>> On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule <
>> pavel.steh...@gmail.com> wrote:
>>
>>> Hi
>>>
>>> I am starting to work review of this patch
>>>
>>> 2015-07-13 9:54 GMT+02:00 dinesh kumar :
>>>
 Hi All,

 Greetings for the day.

 Would like to discuss on below feature here.

 Feature:
 Having an SQL function, to write messages to log destination.

 Justification:
 As of now, we don't have an SQL function to write
 custom/application messages to log destination. We have "RAISE" clause
 which is controlled by
 log_ parameters. If we have an SQL function which works
 irrespective of log settings, that would be a good for many log 
 parsers.
 What i mean is, in DBA point of view, if we route all our native OS 
 stats
 to log files in a proper format, then we can have our log reporting 
 tools
 to give most effective reports. Also, Applications can log their own
 messages to postgres log files, which can be monitored by DBAs too.

 Implementation:
 Implemented a new function "pg_report_log" which takes one
 argument as text, and returns void. I took, "LOG" prefix for all the
 reporting messages.I wasn't sure to go with new prefix for this, since
 these are normal LOG messages. Let me know, if i am wrong here.

 Here is the attached patch.

>>>
>>> This patch is not complex, but the implementation doesn't cover a
>>> "ereport" well.
>>>
>>> Although this functionality should be replaced by custom function in
>>> any PL (now or near future), I am not against to have this function in
>>> core. There are lot of companies with strong resistance against stored
>>> procedures - and sometimes this functionality can help with SQL 
>>> debugging.
>>>
>>> Issues:
>>>
>>> 1. Support only MESSAGE field in exception - I am expecting to
>>> support all fields: HINT, DETAIL, ...
>>>
>>
>> Added these functionalities too.
>>
>>
>>> 2. Missing regress tests
>>>
>>
>> Adding here.
>>
>>
>>> 3. the parsing ereport level should be public function shared with
>>> PLpgSQL and other PL
>>>
>>
>> Sorry Pavel. I am not getting your point here. Would you give me an
>> example.
>>
>
> The transformation: text -> error level is common task - and PLpgSQL
> it does in pl_gram.y. My idea is to add new function to error utils named
> "parse_error_level" and use it from PLpgSQL and from your code.
>
>
>>
>>
>>> 4. should be hidestmt mandatory parameter?
>>>
>>
>> I changed this argument's default value as "true".
>>
>>
>>> 5. the function declaration is strange
>>>
>>> postgres=# \sf pg_report_log (text, anyelement, boolean)
>>> CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text,
>>> anyelement, boolean)
>>>  RETURNS void
>>>  LANGUAGE sql
>>>  STABLE STRICT COST 1
>>> AS $function$SELECT pg_report_log($1::pg_catalog.text,
>>> $2::pg_catalog.text, $3::boolean)$function$
>>>
>>> Why polymorphic? It is useless on any modern release
>>>
>>>
>> I took quote_ident(anyelement) as referral code, for implementing
>> this. Could you guide me if I am doing wrong here.
>>
>
> I was wrong - this is ok - it is emulation of force casting to text
>
>
>>
>>
>>> postgres=# \sf pg_report_log (text, text, boolean)
>>> CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, text,
>>> boolean)
>>>  RETURNS void
>>>  LANGUAGE internal
>>>  IMMUTABLE STRICT
>>> AS $function$pg_report_log$function$
>>>
>>> Why stable, why immutable? This function should be volatile.
>>>
>>> Fixed these to volatile.
>>
>>
>>> 6. using elog level enum as errcode is wrong idea - errcodes are
>>> defined in table
>>> http://www.postgresql.org/docs/9.4/static/errcodes-appendix.html
>>>
>>
>> You mean, if the elevel is 'ERROR', then we need to allow errcode.
>> Let me know your inputs.
>>
>
> I was blind, but the code was not good. Yes, error and higher needs
> error code. From 

[HACKERS] [PATCH] Refactoring of LWLock tranches

[Ildus Kurbangaliev]

Hello hackers!This patch contains LWLocks changes from pg_stat_activity thread(http://www.postgresql.org/message-id/flat/ca+tgmoyd3gtz2_mjfuhf+rpe-bcy75ytjekvv9x-o+soncg...@mail.gmail.com/)and I think it deserves a separate thread.The goal is to split LWLocks from one array to logical pieces (with separatetranches) for better monitoring and debug purposes. Each type of LWLockwill have its own tranche and a associated name.I fixed problems with EXEC_BACKEND. Another parts require a some discussion so I didn't touch them yet.On Sep 2, 2015, at 9:45 PM, Robert Haas  wrote:On Tue, Sep 1, 2015 at 6:43 PM, and...@anarazel.de  wrote:Why a new tranche for each of these? And it can't be correct that eachhas the same base?I complained about the same-base problem before.  Apparently, that got ignored.The idea to create an individual tranches for individual LWLocks havecome from Heikki Linnakangas and I also think that tranche is a good place to keepLWLock name. Base of these tranches points to MainLWLockArray and T_IDmacros keeps the old behavior for them. But I don't insist on the current implementationhere.
Ildus KurbangalievPostgres Professional: http://www.postgrespro.comThe Russian Postgres Company

lwlock_tranches_refactor.patch
Description: Binary data

Re: [HACKERS] Counting lines correctly in psql help displays

[Greg Stark]

On Sat, Sep 5, 2015 at 5:55 PM, Tom Lane  wrote:
> Ordinarily I might think that was overkill, but given the number of times
> that we've failed to update those counts in the past, I think this is
> definitely a worthwhile investment in maintainability.

So to preface this, this was just a silly hack that turned out to be
more effective and simpler to code than I expected. But I suspect it's
still just a silly idea and easier to do what you suggested. Also, it
seems we often get the count wrong on SQL output and elsewhere anyways
and I'm not sure we really want to make that a strict rule. Also, as
someone who doesn't really like the whole "sometimes you get a pager
sometimes you don't" thing and turns it off whenever he sees it I'm
not in the best place to judge how much work it's worth to get the
line count right.

But that said, here's a tricksy patch that triggers an assertion
failure if the expected_lines is wrong. I intended it to trigger in
the regression tests so it only checks if the pager is actually off.
It wouldn't be hard to make it always check though.

-- 
greg
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index cab9e6e..5e2ff09 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -2725,6 +2725,12 @@ print_troff_ms_vertical(const printTableContent *cont, 
FILE *fout)
 /* Public functions*/
 //
 
+#ifdef USE_ASSERT_CHECKING
+#define MAX_PAGEOUTPUT_LINES 1000
+#define MAX_PAGEOUTPUT_BUF (1000 * 80 * 2)
+static char *PageOutputBuf;
+static int expected_lines;
+#endif
 
 /*
  * PageOutput
@@ -2770,6 +2776,21 @@ PageOutput(int lines, const printTableOpt *topt)
 #endif
}
 
+#ifdef USE_ASSERT_CHECKING
+   if (lines < MAX_PAGEOUTPUT_LINES)
+   {
+   fflush(stdout);
+   expected_lines = lines;
+   if (!PageOutputBuf)
+   {
+   PageOutputBuf = pg_malloc0(MAX_PAGEOUTPUT_BUF);
+   setvbuf(stdout, PageOutputBuf, _IOFBF, 
MAX_PAGEOUTPUT_BUF);
+   } else {
+   memset(PageOutputBuf, MAX_PAGEOUTPUT_BUF, 0);
+   }
+   }
+#endif
+
return stdout;
 }
 
@@ -2799,6 +2820,33 @@ ClosePager(FILE *pagerpipe)
pqsignal(SIGPIPE, SIG_DFL);
 #endif
}
+
+#ifdef USE_ASSERT_CHECKING
+   if (PageOutputBuf && expected_lines)
+   {
+   int n_newlines = 0;
+   int i;
+   for (i=0;i

[HACKERS] Outdated documentation for PageHeaderData

[Julien Rouhaud]

Hi,

I just noticed that part of storage.sgml was not updated when 9.3
introduced checksum (and removed pd_tli from PageHeaderData).

Here's a simple patch to fix that.

Regards.
-- 
Julien Rouhaud
http://dalibo.com - http://dalibo.org
diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml
index e5b7b4b..9934efd 100644
--- a/doc/src/sgml/storage.sgml
+++ b/doc/src/sgml/storage.sgml
@@ -759,9 +759,10 @@ data. Empty in ordinary tables.
 
   The first 24 bytes of each page consists of a page header
   (PageHeaderData). Its format is detailed in . The first two fields track the most
-  recent WAL entry related to this page. Next is a 2-byte field
-  containing flag bits. This is followed by three 2-byte integer fields
+  linkend="pageheaderdata-table">. The first field tracks the most recent WAL
+  entry related to this page. The next field stores the checksum for this page.
+  Next is a 2-byte field containing flag bits. This is followed by three 2-byte
+  integer fields
   (pd_lower, pd_upper,
   and pd_special). These contain byte offsets
   from the page start to the start

-- 
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] SQL function to report log message

[dinesh kumar]

Hi Pavel,

On Sat, Sep 5, 2015 at 12:36 AM, Pavel Stehule 
wrote:

>
>
> 2015-09-05 8:35 GMT+02:00 dinesh kumar :
>
>>
>>
>> On Fri, Sep 4, 2015 at 2:03 PM, Pavel Stehule 
>> wrote:
>>
>>>
>>>
>>> 2015-09-02 21:49 GMT+02:00 dinesh kumar :
>>>
 On Mon, Aug 31, 2015 at 10:08 PM, Pavel Stehule <
 pavel.steh...@gmail.com> wrote:

>
>
> 2015-09-01 6:59 GMT+02:00 Pavel Stehule :
>
>>
>>
>> 2015-08-31 20:43 GMT+02:00 dinesh kumar :
>>
>>> Hi,
>>>
>>> On Sat, Aug 29, 2015 at 4:22 PM, Pavel Stehule <
>>> pavel.steh...@gmail.com> wrote:
>>>
 Hi

 I am starting to work review of this patch

 2015-07-13 9:54 GMT+02:00 dinesh kumar :

> Hi All,
>
> Greetings for the day.
>
> Would like to discuss on below feature here.
>
> Feature:
> Having an SQL function, to write messages to log destination.
>
> Justification:
> As of now, we don't have an SQL function to write
> custom/application messages to log destination. We have "RAISE" clause
> which is controlled by
> log_ parameters. If we have an SQL function which works
> irrespective of log settings, that would be a good for many log 
> parsers.
> What i mean is, in DBA point of view, if we route all our native OS 
> stats
> to log files in a proper format, then we can have our log reporting 
> tools
> to give most effective reports. Also, Applications can log their own
> messages to postgres log files, which can be monitored by DBAs too.
>
> Implementation:
> Implemented a new function "pg_report_log" which takes one
> argument as text, and returns void. I took, "LOG" prefix for all the
> reporting messages.I wasn't sure to go with new prefix for this, since
> these are normal LOG messages. Let me know, if i am wrong here.
>
> Here is the attached patch.
>

 This patch is not complex, but the implementation doesn't cover a
 "ereport" well.

 Although this functionality should be replaced by custom function
 in any PL (now or near future), I am not against to have this function 
 in
 core. There are lot of companies with strong resistance against stored
 procedures - and sometimes this functionality can help with SQL 
 debugging.

 Issues:

 1. Support only MESSAGE field in exception - I am expecting to
 support all fields: HINT, DETAIL, ...

>>>
>>> Added these functionalities too.
>>>
>>>
 2. Missing regress tests

>>>
>>> Adding here.
>>>
>>>
 3. the parsing ereport level should be public function shared with
 PLpgSQL and other PL

>>>
>>> Sorry Pavel. I am not getting your point here. Would you give me an
>>> example.
>>>
>>
>> The transformation: text -> error level is common task - and PLpgSQL
>> it does in pl_gram.y. My idea is to add new function to error utils named
>> "parse_error_level" and use it from PLpgSQL and from your code.
>>
>>
>>>
>>>
 4. should be hidestmt mandatory parameter?

>>>
>>> I changed this argument's default value as "true".
>>>
>>>
 5. the function declaration is strange

 postgres=# \sf pg_report_log (text, anyelement, boolean)
 CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text,
 anyelement, boolean)
  RETURNS void
  LANGUAGE sql
  STABLE STRICT COST 1
 AS $function$SELECT pg_report_log($1::pg_catalog.text,
 $2::pg_catalog.text, $3::boolean)$function$

 Why polymorphic? It is useless on any modern release


>>> I took quote_ident(anyelement) as referral code, for implementing
>>> this. Could you guide me if I am doing wrong here.
>>>
>>
>> I was wrong - this is ok - it is emulation of force casting to text
>>
>>
>>>
>>>
 postgres=# \sf pg_report_log (text, text, boolean)
 CREATE OR REPLACE FUNCTION pg_catalog.pg_report_log(text, text,
 boolean)
  RETURNS void
  LANGUAGE internal
  IMMUTABLE STRICT
 AS $function$pg_report_log$function$

 Why stable, why immutable? This function should be volatile.

 Fixed these to volatile.
>>>
>>>
 6. using elog level enum as errcode is wrong idea - errcodes are
 defined in table
 

Re: [HACKERS] PATCH: index-only scans with partial indexes

[Tomas Vondra]

Hi,

On 09/05/2015 10:53 AM, Simon Riggs wrote:

On 4 September 2015 at 22:03, Tomas Vondra > wrote:

A summary of 100 EXPLAIN timings looks like this:


master   A  B  C  D  E  F
-
min0.10   0.10   0.30   0.29   0.66   0.23
max1.07   1.00   2.13   1.98   4.52   1.59
median 0.49   0.52   0.31   0.33   0.68   1.12
average0.43   0.35   0.62   0.49   1.01   0.89


What are these? Times? in ms?


Yes, those are planning times in milliseconds. I've been thinking about 
possible issues in the benchmark, and I ended up with two main suspects:


  (a) environment - VM running on a laptop. thus quite noisy and
  subject to various sources of overhead, power-management, etc.

  (b) time measured using \timing in psql (by running EXPLAIN), so
  probably influenced by formatting/transfer

So I reran the benchmark on a different machine (bare metal, pretty much 
no noise in the results), and measured the planning time using EXPLAIN 
ANALYZE (Planning Time). And I got this (milliseconds):


   A B C D E F
-
 min  0.04  0.04  0.11  0.10  0.37  0.12
 max  0.10  0.10  0.92  0.92  1.62  1.23
 median   0.04  0.04  0.11  0.11  0.37  0.13
 average  0.04  0.04  0.11  0.11  0.38  0.14

   A B C D E F
-
 min  0.04  0.04  0.11  0.11  0.38  0.13
 max  0.10  0.10  0.92  0.94  1.64  1.21
 median   0.04  0.04  0.11  0.11  0.39  0.13
 average  0.04  0.04  0.11  0.12  0.40  0.14

So much lower numbers (better CPU, no virtualization, etc.), but 
otherwise exactly the same conclusion - no overhead compared to master.


I think of three ways how to make the checks more expensive:

   (a) using more indexes

   The current benchmark already uses 40 indexes (and I've tried
   with 100), and we've seen no impact at all. Adding more indexes
   will eventually show some overhead, but the number of indexes
   will be very high - I doubt anyone has a table with hundreds of
   partial indexes on a it.

   (b) using more complex index predicates

   I expect the predicate_implied_by() call to get more expensive
   for more complex predicates. I however believe that's quite
   uncommon case - vast majority of index predicates that I've seen
   use just a single equality clause.

   (c) using more complex queries (more WHERE conditions)

   Having more complex WHERE clauses seems quite plausible, though,
   so I've decided to try it. Instead of the simple query used
   before:

   select a from t where b >= 100 and b <= 200;

   I've used a query with a bunch of other conditions:

   select a from t where b >= 100 and b <= 200
 and c >= 100 and c <= 200
 and d >= 100 and d <= 200
 and a >= 100 and a <= 100;

   And indeed, this made a (tiny) difference - on the master, the
   planning was 0.50 ms on average, while with the patch it was
   0.55. But 0.05 ms is just barely above noise, even on this HW.

   Of course, this only impacts the case with partial indexes, all
   the other cases were exactly the same with and without patch.


However that was the idea of the benchmark, and I got no difference.


Please explain what this means and your conclusion, so its clear. That
way we can either reject the patch or commit it. Thanks


That means I've been unable to measure any significant overhead of the 
patch. There certainly are extreme cases where this patch might make the 
planning noticeably slower, but I believe those are rather artificial, 
and certainly wouldn't expect them in databases where a tiny increase of 
planning time would be a problem.


This benchmark however only looked at the planning overhead, but we 
should weight that with respect to possible gains. And IOS is a great 
optimization - it's not uncommon to see 2-3x improvements on databases 
that fit into RAM, and order of magnitude improvements on large 
databases (thanks to eliminating the random I/O when accessing the heap).


So my opinion is that we should commit this patch.

regards

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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make 

[HACKERS] Fix small bug for build without HAVE_SYMLINK

[YUriy Zhuravlev]

Hello hackers.

During my work on CMake I stumbled upon this simple bug. I think this is 
important for mingw users.
The function is trying to return a variable that has not been announced.

Patch in attach. Probably relevant for stable releases.

Thanks! 
-- 
YUriy Zhuravlev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Companydiff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index ff0d904..eba9c6a 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -383,13 +383,15 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 
 	/* We keep the lock on pg_tablespace until commit */
 	heap_close(rel, NoLock);
+
+	return tablespaceoid;
 #else			/* !HAVE_SYMLINK */
 	ereport(ERROR,
 			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 			 errmsg("tablespaces are not supported on this platform")));
-#endif   /* HAVE_SYMLINK */
 
-	return tablespaceoid;
+	return InvalidOid;
+#endif   /* HAVE_SYMLINK */
 }
 
 /*

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

Re: [HACKERS] Multi-column distinctness.

[Bruce Momjian]

On Fri, Aug 28, 2015 at 05:33:34PM +0900, Kyotaro HORIGUCHI wrote:
> Hello, this patch enables planner to be couscious of inter-column
> correlation.
> 
> Sometimes two or more columns in a table has some correlation
> which brings underestimate, which leads to wrong join method and
> ends with slow execution.
> 
> Tomas Vondra is now working on heavily-equipped multivariate
> statistics for OLAP usage. In contrast, this is a lightly
> implemented solution which calculates only the ratio between a
> rows estimated by current method and a actual row number. I think
> this doesn't conflict with his work except the grammar part.

I am very glad multi-variate statistics are being addressed.  I think
this is our most common cause of optimizer failures.

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

  + Everyone has their own god. +


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

Re: [HACKERS] Counting lines correctly in psql help displays

[Tom Lane]

Greg Stark  writes:
> But that said, here's a tricksy patch that triggers an assertion
> failure if the expected_lines is wrong. I intended it to trigger in
> the regression tests so it only checks if the pager is actually off.
> It wouldn't be hard to make it always check though.

Hmm ... that would put a premium on the linecount always being exactly
right (for all callers, not just the help functions).  Not sure that
I want to buy into that --- it would require more complexity in the
callers than is there now, for sure.

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] Fix small bug for build without HAVE_SYMLINK

[Tom Lane]

YUriy Zhuravlev  writes:
> During my work on CMake I stumbled upon this simple bug. I think this is 
> important for mingw users.
> The function is trying to return a variable that has not been announced.

> Patch in attach. Probably relevant for stable releases.

Will commit, thanks!

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] Fwd: Core dump with nested CREATE TEMP TABLE

[Tom Lane]

Michael Paquier  writes:
> On Sat, Sep 5, 2015 at 3:41 PM, Jim Nasby  wrote:
>> I was about to test this and was verifying that the crash still worked
>> when I noticed this in the logs (9.4.1, not HEAD). Not sure if there's any
>> realion here or not...
>> 
>> WARNING:  relation "pg_proc" page 121 is uninitialized --- fixing
>> WARNING:  relation "pg_proc" page 122 is uninitialized --- fixing

> [reading vacuumlazy.c...] This seems unrelated and I would not worry about
> it. Those system catalogs have been extended with new pages by a couple of
> backends, but a crash happened before they could actually insert tuples on
> it and commit. Perhaps you were creating a bunch of objects when a crash
> happened, no?

If this is the system you were doing the previous crash testing on, I'd
say it's a direct artifact of those crashes.  You were repeatedly crashing
the system during transactions that created functions and temp tables,
which would surely make entries in pg_proc, pg_depend, etc.  The crash
came before commit and so neither the relation pages nor the WAL entries
would necessarily get to disk.  But extension of the relations to make
room would still happen.  Moreover, there's probably nothing to cause
those new pages to get added to the relations' FSM, so each new test
attempt would add another set of pages.  Eventually autovacuum would
notice, which is what you've got logged here, but it might take awhile.

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] Counting lines correctly in psql help displays

[Tom Lane]

I wrote:
> BTW, I noticed that the PageOutput line counts for psql's usage(),
> slashUsage(), and helpVariables() were all three wrong, which I'm afraid
> has been their usual state in the past too.  Since commit 07c8651dd91d5aea
> there's been a pretty easy way to check them, which I added comments
> about; but I don't hold much hope that that will fix anything.  I wonder
> whether there's some way to not need to maintain those counts manually.

I seem to recall past proposals to fix that by putting the lines into
static char * arrays, which foundered on the fact that the output's not
necessarily constant.  But it suddenly strikes me that there's an easy
fix.  We can generate the output into a PQExpBuffer, which is just as
flexible as fprintf() is today, then count the lines and finally print.

Ordinarily I might think that was overkill, but given the number of times
that we've failed to update those counts in the past, I think this is
definitely a worthwhile investment in maintainability.

Or we could just give up and replace the counts by INT_MAX, forcing use
of the pager unless you've turned it off.  All of those outputs are long
enough now that it's hard to believe there are any common screen layouts
where you don't end up invoking the pager anyway.  (usage() is 60 lines,
the others are more.)  This is probably the reason why we've seldom
noticed they're wrong --- it barely matters anymore.

One way or the other I think it's past time to get out of the business
of maintaining these counts.  I'm willing to do the work of using a
PQExpBuffer if people think it's worth the trouble to have an accurate
count, but it may not be worth the code space.

Thoughts?

regards, tom lane


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

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Tom Lane]

I wrote:
> If there are not major objections, I'll work on cleaning up and
> committing the patch.

Pushed.  I'm not too sure about the expected outputs for python other
than 2.6, nor for sepgsql, but hopefully the buildfarm will provide
feedback.

BTW, I noticed that the PageOutput line counts for psql's usage(),
slashUsage(), and helpVariables() were all three wrong, which I'm afraid
has been their usual state in the past too.  Since commit 07c8651dd91d5aea
there's been a pretty easy way to check them, which I added comments
about; but I don't hold much hope that that will fix anything.  I wonder
whether there's some way to not need to maintain those counts manually.

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] PL/pgSQL, RAISE and error context

[Joe Conway]

On 09/05/2015 09:05 AM, Tom Lane wrote:
> I wrote:
>> If there are not major objections, I'll work on cleaning up and
>> committing the patch.
> 
> Pushed.  I'm not too sure about the expected outputs for python other
> than 2.6, nor for sepgsql, but hopefully the buildfarm will provide
> feedback.

We don't have the buildfarm actually checking sepgsql yet, but I'll
check it out manually today or tomorrow.

Joe

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



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] pg_ctl/pg_rewind tests vs. slow AIX buildfarm members

[Noah Misch]

On Fri, Sep 04, 2015 at 10:54:51PM -0400, Tom Lane wrote:
> Noah Misch  writes:
> > On Thu, Sep 03, 2015 at 03:31:06PM -0400, Tom Lane wrote:
> >>> This is the first time I've seen an indication that the
> >>> start_postmaster change mentioned in the comment is actually important
> >>> for production use, rather than just being cleanup.
> 
> > I scratched my head awhile without thinking of a credible production use 
> > case
> > that would notice the difference.  Which one did you have in mind?
> 
> Well, mainly that it's making our regression tests fail, which suggests
> behavioral instability that could be important for production.

I doubt issuing two "pg_ctl start" within two seconds, with no intervening
stop, has a production use.  However, ...

> Aside from the specific case you diagnosed, it's clear from buildfarm
> results that the five-second timeout elsewhere in
> test_postmaster_connection has got funny behavior under load; there are
> way too many cases where pg_ctl gives up after exactly that long, with
> no useful information printed.  The draft patch I posted gets rid of that
> behavior ...

... fixing this is a good deal more valuable.  Besides coping with stalls in
early postmaster startup, pg_ctl would no longer take 5s to exit when the
postmaster reports a postgresql.conf defect or other early error.

On Thu, Sep 03, 2015 at 03:31:06PM -0400, Tom Lane wrote:
> If this does (or can be made to) work on Windows, I'd propose applying
> it back to 9.2, where the current coding came in.

That seems prudent.  We'll keep meeting buildfarm members with performance
variability sufficient to reach that 5s timeout.  (Incidentally, the current
coding first appeared in 9.1.)

> --- 650,671 
>   }
>   
>   /*
> !  * Reap child process if it died; if it remains in zombie state 
> then
> !  * our kill-based test will think it's still running.
>*/
> ! #ifndef WIN32
>   {
> ! int exitstatus;
>   
> ! (void) waitpid((pid_t) pm_pid, , WNOHANG);

Let's report any exit status.  When a signal kills the young postmaster, it's
a waste to omit that detail.

>   }
> + #endif
>   
>   /*
> !  * Check whether the child postmaster process is still alive.  
> This
> !  * lets us exit early if the postmaster fails during startup.
>*/
> ! if (!postmaster_is_alive((pid_t) pm_pid))
>   return PQPING_NO_RESPONSE;

kill() adds nothing when dealing with a pg_ctl child.  waitpid() is enough.

Thanks,
nm


-- 
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] Outdated documentation for PageHeaderData

[Michael Paquier]

On Sun, Sep 6, 2015 at 5:11 AM, Julien Rouhaud 
wrote:

> I just noticed that part of storage.sgml was not updated when 9.3
> introduced checksum (and removed pd_tli from PageHeaderData).
>

Yep, see that as well:
http://www.postgresql.org/message-id/cab7npqsv1kr6fqwf2rovter5cxaw8oth+dkzjwp4meji5do...@mail.gmail.com
I just fell out of committer's laser radar I guess.
-- 
Michael

Re: [HACKERS] pg_ctl/pg_rewind tests vs. slow AIX buildfarm members

[Noah Misch]

On Sat, Sep 05, 2015 at 10:22:59PM -0400, Tom Lane wrote:
> Noah Misch  writes:
> > kill() adds nothing when dealing with a pg_ctl child.  waitpid() is enough.
> 
> The kill() coding works on Windows (I believe); waitpid() not so much.

Windows has the concept under a different name.  See postmaster.c.


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

Re: [HACKERS] pg_ctl/pg_rewind tests vs. slow AIX buildfarm members

[Tom Lane]

Noah Misch  writes:
> On Sat, Sep 05, 2015 at 10:22:59PM -0400, Tom Lane wrote:
>> The kill() coding works on Windows (I believe); waitpid() not so much.

> Windows has the concept under a different name.  See postmaster.c.

Well, if someone else wants to code and test that, feel free.  I don't
do Windows ...

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] Failing assertions in indxpath.c, placeholder.c and brin_minmax.c

[Greg Stark]

On Sun, Jul 26, 2015 at 11:15 PM, Peter Geoghegan  wrote:
> The SQLite people have been using a tool like this for some time.
> They've also had luck finding bugs with a generic fuzz-testing tool
> called "american fuzzy lop" (yes, seriously, that's what it's called),
> which apparently is the state of the art.
>
> I myself ran that tool against Postgres. I didn't spend enough time to
> tweak it in a way that might have been effective. I also didn't figure
> out a way to make iterations fast enough for the tool to be effective,
> because I was invoking Postgres in single-user mode. I might pick it
> up again in the future, but probably for a more targeted case.

I've been poking at this. Like you I ran AFL against Postgres in
single-user mode. It was slowed down by the Postgres startup time and
worse, it was very limiting being in single-user-mode.

What I think holds more promise is Libfuzzer from LLVM. It's not as
clever about generating test cases and it doesn't have a pretty curses
interface, but it's much more amenable to integrating into a
client/server architecture.

In fact this is the interface I have now:

stark=> SELECT fuzz(100, 'select $1::timestamptz')

Pretty slick, eh? :)

It's also blindingly fast. Even with a subtransaction around each
fuzzing call it's still getting up to 5-8k/s executions for simpler
functions. The main disadvantage is that it's more fragile because
it's not isolated from the program it's testing. So a bug that causes
infinite recursion or consumes lots of memory can cause it to crash
and it can take a while to do so.

I hope to package it up as a contrib module but it does require the
Libfuzzer library which is built as part of LLVM but only when you
build LLVM itself with the coverage sanitizer so it doesn't end up in
any llvm binary packages. And of course building LLVM takes forever
and a day... Maybe I'll include a copy of the llvm source files in the
module -- the license looks to be compatible.

Anyways, first non-security trophy it found:

stark=# SELECT 'doy'::timestamptz;
ERROR:  unexpected dtype 33 while parsing timestamptz "doy"
LINE 1: SELECT 'doy'::timestamptz;
   ^
stark=# SELECT 'dow'::timestamptz;
ERROR:  unexpected dtype 32 while parsing timestamptz "dow"
LINE 1: SELECT 'dow'::timestamptz;
   ^

This looks to be a quite an old bug. It dates to 2000 and the code
hasn't been touched since Tom wrote it -- not that Tom but Tom
Lockhart. I haven't quite grokked the ParseDateTime code but it looks
to me like the "dow" and "doy" keywords are miscategorized and should
be type UNITS not type RESERV. The corresponding code in extract_date
would then have to move to the other branch leaving "epoch" as the
only reserved word which serves double duty as a unit in extract date.

-- 
greg


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

Re: [HACKERS] Pg_upgrade remote copy

[AI Rumman]

Thanks for the solution.

On Wed, Sep 2, 2015 at 4:15 PM, Bruce Momjian  wrote:

> On Wed, Sep  2, 2015 at 04:50:32PM -0300, Alvaro Herrera wrote:
> > Bruce Momjian wrote:
> > > On Fri, Aug 28, 2015 at 10:34:38PM -0700, AI Rumman wrote:
> >
> > > > In pg_upgrade, how about adding a feature to copy data directory
> over network.
> > > > That is, we can run pg_upgrade from our new host, where old host
> will be a
> > > > remote machine.
> >
> > > I think it is much simpler to just copy the old clsuter to the remote
> > > server and run pg_upgrade in --link mode on the remote server.
> >
> > Couldn't it run pg_basebackup as a first step?
>
> Yes.  I was thinking it would be simpler to just shut down the server
> and copy it, but pg_basebackup would allow the server to be up during
> the copy.
>
> --
>   Bruce Momjian  http://momjian.us
>   EnterpriseDB http://enterprisedb.com
>
>   + Everyone has their own god. +
>

Re: [HACKERS] pg_ctl/pg_rewind tests vs. slow AIX buildfarm members

[Tom Lane]

Noah Misch  writes:
> kill() adds nothing when dealing with a pg_ctl child.  waitpid() is enough.

The kill() coding works on Windows (I believe); waitpid() not so much.

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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

> Pushed.  I'm not too sure about the expected outputs for python other
> than 2.6, nor for sepgsql, but hopefully the buildfarm will provide
> feedback.
>
>
Thank you very much

Pavel

Re: [HACKERS] Autonomous Transaction is back

[Noah Misch]

On Thu, Sep 03, 2015 at 04:21:55PM -0400, Robert Haas wrote:
> On Sat, Aug 22, 2015 at 2:23 AM, Noah Misch  wrote:
> >> > Can you get away with only looking at tuples though?  For example,
> >> > what about advisory locks?  Table locks?
> >>
> >> Well, that's an interesting question.  Can we get away with regarding
> >> those things as non-conflicting, as between the parent and child
> >> transactions?
> >
> > For system lock types, no.  While one could define advisory locks to work
> > differently, we should assume that today's advisory lockers have 
> > expectations
> > like those of system lockers.  An autonomous transaction should not bypass 
> > any
> > lock that a transaction of another backend could not bypass.
> 
> Why?
> 
> Suppose you do this:
> 
> BEGIN;
> DECLARE CURSOR foo FOR SELECT * FROM foo;
> BEGIN AUTONOMOUS TRANSACTION;
> ALTER TABLE foo ALTER bar TYPE int;
> 
> This has got to fail for safety reasons, but CheckTableNotInUse() is
> on it.  Suppose you do this:
>
> BEGIN;
> LOCK foo;
> BEGIN AUTONOMOUS TRANSACTION;
> INSERT INTO foo VALUES ('spelunk');
> 
> How will making this fail improve anything?

Core PostgreSQL doesn't care.  This is a user interface design decision to be
made in light of current SQL expectations and future SQL author wishes.  The
LOCK reference page, our contract with users, says nothing to constrain the
choice.  LOCK is exceptional in that we never get much insight into the
caller's expectations.  I think LOCK should follow closely the built-in
commands that take the same locks.  This variation of your examples must fail
in order to avoid a paradox if the first transaction aborts:

BEGIN;
ALTER TABLE foo ALTER bar TYPE frob;
BEGIN AUTONOMOUS TRANSACTION;
INSERT INTO foo VALUES ('spelunk');

If we made that fail and made your second example succeed, that would imply
"LOCK foo" acquires a special kind of AccessExclusiveLock differing from what
ALTER TABLE acquires.  That's incompatible with my sense of the LOCK command's
role in the system.  An ability to procure an option to acquire, without
waiting, a lock and delegate that option to another transaction would have
applications.  It's a different feature calling for distinct syntax.


My comments have flowed out of a principle that autonomous transactions shall
have precisely the same semantics as using another backend via dblink.  They
should have less overhead.  They may give different error messages.  They
shall not permit sequences of commands that fail in a dblink implementation of
the same multi-transaction sequence.  I chose this principle because it fits
my intuitive notion of transaction "autonomy" and because everything I've
heard about other implementations suggests that they work in that way.  If
some RDBMS implementation does otherwise, I would find that persuasive.

What design principle(s) have you been using to decide how autonomous
transactions should behave?


-- 
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] Allow a per-tablespace effective_io_concurrency setting

[Merlin Moncure]

On Fri, Sep 4, 2015 at 11:21 AM, Greg Stark  wrote:
> On Thu, Sep 3, 2015 at 2:13 PM, Merlin Moncure  wrote:
>>   I find this talk of platters and spindles to be somewhat
>> baroque; for a 200$ part I have to work pretty hard to max out the
>> drive when reading and I'm still not completely sure if it's the drive
>> itself, postgres, cpu, or sata interface bottlenecking me.  This will
>> require a rethink of e_i_o configuration; in the old days there were
>> physical limitations of the drives that were in the way regardless of
>> the software stack but we are in a new era, I think.  I'm convinced
>> prefetching works and we're going to want to aggressively prefetch
>> anything and everything possible.  SSD controllers (at least the intel
>> ones) are very smart.
>
>
> Wouldn't SSDs need much *less* aggressive prefetching? There's still
> latency and there are multiple I/O channels so they will still need
> some. But spinning media gives latencies measured in milliseconds. You
> can process a lot of tuples in milliseconds. If you have a hundred
> spindles you want them all busy doing seeks because in the 5ms it
> takes them to do that you can proess all the results on a single cpu
> and the rest of time is spend waiting.
>
> When your media has latency on the order of microseconds then you only
> need to have a small handful of I/O requests in flight to keep your
> processor busy.

I'm not sure I agree with that.  100 micosecond latency is still a
pretty big deal when memory latency is measured in nanoseconds.  This
is why we have pcie storage devices.  (if anyone has a fast pice flash
device I'd be interested in seing some e_i_o tests...I bet they'd
still show some impact but it'd be less).

For spinning media, any random i/o workload on large datasets is a
freight train to 100% iowait.  As each device has to process each data
set synchronously minus what small gains can be eeked out by
reordering and combining writes.  This drives data transfer rates
under a megabyte per second.

Flash devices OTOH are essentially giant raid 0 devices of electronic
media underneath the flash controller.  Each device can functionally
do many read operations in parallel so it makes perfect sense that
they perform better when given multiple overlapping requests; unlike
spinning media they can actually make use of that information and
those assumptions are well supported by measurement.

Flash is getting better all the time and storage technologies that
remove its one weakness, random writes, appear to be right around the
corner.   The age of storage being a bottleneck for database systems
without massive high cost engineering and optimization has come to an
end.  This is good for software developers and especially good for
databases because most of the perceived mythology of databases being
'slow' are in fact storage issues.  With that problem gone focus will
return to clean data processing models which is where database systems
excel with their many decades of refinement.

merlin


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