Re: [HACKERS] Set of patch to address several Coverity issues

2015-07-09 Thread Michael Paquier
On Wed, Jul 8, 2015 at 6:16 PM, Andres Freund wrote:
 In any case, we are going to need at least (void) in front of those calls.

 We're needing nothing of the sort.

I don't really understand your reluctance here. As one example, see
c831593 where similar fixes are done and even back-patched.
-- 
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] Waits monitoring

2015-07-09 Thread Fujii Masao
On Thu, Jul 9, 2015 at 2:12 PM, Haribabu Kommi kommi.harib...@gmail.com wrote:
 On Thu, Jul 9, 2015 at 1:52 AM, Ildus Kurbangaliev
 i.kurbangal...@postgrespro.ru wrote:
 Hello.

 Currently, PostgreSQL offers many metrics for monitoring. However, detailed
 monitoring of waits is still not supported yet. Such monitoring would
 let dba know how long backend waited for particular event and therefore
 identify
 bottlenecks. This functionality is very useful, especially for highload
 databases. Metric for waits monitoring are provided by many popular
 commercial
 DBMS. We currently have requests of this feature from companies migrating to
 PostgreSQL from commercial DBMS. Thus, I think it would be nice for
 PostgreSQL
 to have it too.

 Yes, It is good have such wait monitoring views in PostgreSQL.
 you can add this patch to the open commitfest.

Robert and Amit proposed very similar idea and its patch is now being
reviewed in the current CommitFest. So I think that you should attend
that discussion rather than starting new one.

http://www.postgresql.org/message-id/ca+tgmoyd3gtz2_mjfuhf+rpe-bcy75ytjekvv9x-o+soncg...@mail.gmail.com

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] Further issues with jsonb semantics, documentation

2015-07-09 Thread Andrew Dunstan


On 07/09/2015 04:10 AM, Peter Geoghegan wrote:

On Mon, Jun 22, 2015 at 6:19 PM, Peter Geoghegan p...@heroku.com wrote:

On Wed, Jun 10, 2015 at 11:48 AM, Andrew Dunstan and...@dunslane.net wrote:

Please submit a patch to adjust the treatment of negative integers in the
old functions to be consistent with their treatment in the new functions.
i.e. in the range [-n,-1] they should refer to the corresponding element
counting from the right.

This patch is attached, along with a separate patch which adds a
release note compatibility item.

Where are we on this? This is currently a 9.5 release blocker.



I am on vacation right now, but I might have some time tomorrow to deal 
with it. If not, it will be Sunday or Monday when I get to it.


cheers

andrew


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


Re: [HACKERS] Waits monitoring

2015-07-09 Thread Ildus Kurbangaliev

 On Jul 9, 2015, at 5:18 PM, Fujii Masao masao.fu...@gmail.com wrote:
 
 On Thu, Jul 9, 2015 at 2:12 PM, Haribabu Kommi kommi.harib...@gmail.com 
 mailto:kommi.harib...@gmail.com wrote:
 On Thu, Jul 9, 2015 at 1:52 AM, Ildus Kurbangaliev
 i.kurbangal...@postgrespro.ru wrote:
 Hello.
 
 Currently, PostgreSQL offers many metrics for monitoring. However, detailed
 monitoring of waits is still not supported yet. Such monitoring would
 let dba know how long backend waited for particular event and therefore
 identify
 bottlenecks. This functionality is very useful, especially for highload
 databases. Metric for waits monitoring are provided by many popular
 commercial
 DBMS. We currently have requests of this feature from companies migrating to
 PostgreSQL from commercial DBMS. Thus, I think it would be nice for
 PostgreSQL
 to have it too.
 
 Yes, It is good have such wait monitoring views in PostgreSQL.
 you can add this patch to the open commitfest.
 
 Robert and Amit proposed very similar idea and its patch is now being
 reviewed in the current CommitFest. So I think that you should attend
 that discussion rather than starting new one.
 
 http://www.postgresql.org/message-id/ca+tgmoyd3gtz2_mjfuhf+rpe-bcy75ytjekvv9x-o+soncg...@mail.gmail.com
  
 http://www.postgresql.org/message-id/ca+tgmoyd3gtz2_mjfuhf+rpe-bcy75ytjekvv9x-o+soncg...@mail.gmail.com


This thread raising waits monitoring problem much more general than just adding 
one more column to pg_stat_activity. This patch contains profiling, history and 
much more details about current wait events. We appreciate joint work in this 
direction, but we have just started with publishing our current work and raise 
waits monitoring question in its full weight.


Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com http://www.postgrespro.com/
The Russian Postgres Company

Re: [HACKERS] Re: [HACKERS] GSoC 2015 proposal: Improve the performance of “ALTER TABLE .. SET LOGGED / UNLOGGED” statement

2015-07-09 Thread Andres Freund
On 2015-07-09 10:39:35 -0300, Fabrízio de Royes Mello wrote:
 If the wal_level=minimal we don't need to force the wal log of the
 contents. If the wal_level != minimal we need just to xlog all the pages,
 but in both cases we don't need the extra job to create a new datafiles and
 copy the contents between them. So we'll improve performance, or am I
 missing something?

Sure. It'll be a bit faster. I just don't see the peformance increase in
not that common situations being worth the price we'll pay in
development, code review, debugging and then maintaining some nontrivial
code. If this were likely to be a 15 line patch, I'd think differently.


-- 
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] Supporting TAP tests with MSVC and Windows

2015-07-09 Thread Michael Paquier
On Thu, Jun 25, 2015 at 1:40 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
 On Tue, May 26, 2015 at 3:39 PM, Michael Paquier wrote:
 Here is v6, a rebased version on HEAD (79f2b5d). There were some
 conflicts with the indentation and some other patches related to
 pg_rewind and initdb's tests.

 Attached is v7, rebased on 0b157a0.

Attached is v8, rebased on 1ea0620, meaning that it includes all the
fancy improvements in log capture for TAP tests.
-- 
Michael
From e8db0a990ec02ddb5bdf2fd95d42b297360ecd55 Mon Sep 17 00:00:00 2001
From: Michael Paquier mich...@otacoo.com
Date: Thu, 9 Jul 2015 06:46:13 -0700
Subject: [PATCH] Add support for TAP tests on Windows

Nodes initialized by the TAP tests use SSPI to securely perform the
tests, and test scripts are patched in a couple of places to support
Windows grammar. In the case of MSVC, tests can be run with this
command:
vcregress tapcheck
---
 doc/src/sgml/install-windows.sgml|  1 +
 src/Makefile.global.in   |  2 +-
 src/bin/pg_basebackup/t/010_pg_basebackup.pl | 67 +---
 src/bin/pg_ctl/t/001_start_stop.pl   | 14 --
 src/bin/pg_ctl/t/002_status.pl   | 12 -
 src/bin/pg_rewind/RewindTest.pm  | 56 ---
 src/bin/scripts/t/020_createdb.pl|  3 ++
 src/test/perl/TestLib.pm | 19 +---
 src/tools/msvc/Solution.pm   |  1 +
 src/tools/msvc/clean.bat |  8 
 src/tools/msvc/config_default.pl |  1 +
 src/tools/msvc/vcregress.pl  | 48 +++-
 12 files changed, 176 insertions(+), 56 deletions(-)

diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index d154b44..2047790 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -439,6 +439,7 @@ $ENV{CONFIG}=Debug;
 userinputvcregress modulescheck/userinput
 userinputvcregress ecpgcheck/userinput
 userinputvcregress isolationcheck/userinput
+userinputvcregress tapcheck/userinput
 userinputvcregress upgradecheck/userinput
 /screen
 
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 8d1250d..4b9c529 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -338,7 +338,7 @@ endef
 
 define prove_check
 rm -rf $(srcdir)/tmp_check/log
-cd $(srcdir)  TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl
+cd $(srcdir)  TESTDIR='$(CURDIR)' TESTREGRESS='$(top_builddir)/src/test/regress/pg_regress' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl
 endef
 
 else
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index e47c3a0..59e8cb4 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -1,8 +1,9 @@
 use strict;
 use warnings;
 use Cwd;
+use Config;
 use TestLib;
-use Test::More tests = 35;
+use Test::More tests = ($Config{osname} eq MSWin32) ? 25 : 35;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
@@ -25,10 +26,20 @@ if (open BADCHARS, $tempdir/pgdata/FOO\xe0\xe0\xe0BAR)
 	close BADCHARS;
 }
 
+# Use SSPI on Windows, node has been initialized already accordingly
+# by pg_regress --config-auth.
 open HBA, $tempdir/pgdata/pg_hba.conf;
-print HBA local replication all trust\n;
-print HBA host replication all 127.0.0.1/32 trust\n;
-print HBA host replication all ::1/128 trust\n;
+if ($Config{osname} ne MSWin32)
+{
+	print HBA local replication all trust\n;
+	print HBA host replication all 127.0.0.1/32 trust\n;
+	print HBA host replication all ::1/128 trust\n;
+}
+else
+{
+	print HBA host replication all 127.0.0.1/32 sspi include_realm=1 map=regress\n;
+	print HBA host replication all ::1/128 sspi include_realm=1 map=regress\n;
+}
 close HBA;
 system_or_bail 'pg_ctl', '-D', $tempdir/pgdata, 'reload';
 
@@ -64,6 +75,33 @@ command_fails([ 'pg_basebackup', '-D', $tempdir/tarbackup_l1, '-Ft' ],
 	'pg_basebackup tar with long name fails');
 unlink $tempdir/pgdata/$superlongname;
 
+command_fails(
+	[ 'pg_basebackup', '-D', $tempdir/backup_foo, '-Fp', -T=/foo ],
+	'-T with empty old directory fails');
+command_fails(
+	[ 'pg_basebackup', '-D', $tempdir/backup_foo, '-Fp', -T/foo= ],
+	'-T with empty new directory fails');
+command_fails(
+	[   'pg_basebackup', '-D', $tempdir/backup_foo, '-Fp',
+		-T/foo=/bar=/baz ],
+	'-T with multiple = fails');
+command_fails(
+	[ 'pg_basebackup', '-D', $tempdir/backup_foo, '-Fp', -Tfoo=/bar ],
+	'-T with old directory not absolute fails');
+command_fails(
+	[ 'pg_basebackup', '-D', $tempdir/backup_foo, '-Fp', -T/foo=bar ],
+	'-T with new directory not absolute fails');
+command_fails(
+	[ 'pg_basebackup', '-D', $tempdir/backup_foo, '-Fp', -Tfoo ],
+	'-T with invalid format fails');
+
+# Windows does not 

[HACKERS] Re: [HACKERS] Re: [HACKERS] GSoC 2015 proposal: Improve the performance of “ALTER TABLE .. SET LOGGED / UNLOGGED” statement

2015-07-09 Thread Fabrízio de Royes Mello
On Thu, Jul 9, 2015 at 10:56 AM, Andres Freund and...@anarazel.de wrote:

 On 2015-07-09 10:39:35 -0300, Fabrízio de Royes Mello wrote:
  If the wal_level=minimal we don't need to force the wal log of the
  contents. If the wal_level != minimal we need just to xlog all the
pages,
  but in both cases we don't need the extra job to create a new datafiles
and
  copy the contents between them. So we'll improve performance, or am I
  missing something?

 Sure. It'll be a bit faster. I just don't see the peformance increase in
 not that common situations being worth the price we'll pay in
 development, code review, debugging and then maintaining some nontrivial
 code. If this were likely to be a 15 line patch, I'd think differently.

Well, this will not be a 15 line patch, but I'll try to simplify as much as
possible.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
 Timbira: http://www.timbira.com.br
 Blog: http://fabriziomello.github.io
 Linkedin: http://br.linkedin.com/in/fabriziomello
 Twitter: http://twitter.com/fabriziomello
 Github: http://github.com/fabriziomello


Re: [HACKERS] Re: [HACKERS] GSoC 2015 proposal: Improve the performance of “ALTER TABLE .. SET LOGGED / UNLOGGED” statement

2015-07-09 Thread Tom Lane
Andres Freund and...@anarazel.de writes:
 On 2015-07-09 10:39:35 -0300, Fabrízio de Royes Mello wrote:
 If the wal_level=minimal we don't need to force the wal log of the
 contents. If the wal_level != minimal we need just to xlog all the pages,
 but in both cases we don't need the extra job to create a new datafiles and
 copy the contents between them. So we'll improve performance, or am I
 missing something?

 Sure. It'll be a bit faster. I just don't see the peformance increase in
 not that common situations being worth the price we'll pay in
 development, code review, debugging and then maintaining some nontrivial
 code. If this were likely to be a 15 line patch, I'd think differently.

I'm even more worried about the possible reliability problems (ie
introduction of bugs, which are likely to be of the data-eating variety)
that such changes are likely to incur.  So I tend to agree with Andres
that this is probably not a good idea.

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] creating extension including dependencies

2015-07-09 Thread Petr Jelinek

On 2015-07-07 15:41, Andres Freund wrote:

On 2015-07-07 22:36:29 +0900, Fujii Masao wrote:

On Mon, Jun 15, 2015 at 7:50 AM, Petr Jelinek p...@2ndquadrant.com wrote:

Hi,

I am getting tired installing manually required extensions manually. I was
wondering if we might want to add option to CREATE SEQUENCE that would allow
automatic creation of the extensions required by the extension that is being
installed by the user.


I'm wondering how much helpful this feature is. Because, even if we can save
some steps for CREATE EXTENSION by using the feature, we still need to
manually find out, download and install all the extensions that the target
extension depends on. So isn't it better to implement the tool like yum, i.e.,
which performs all those steps almost automatically, rather than the proposed
feature? Maybe it's outside PostgreSQL core.


That doesn't seem to make much sense to me. Something like yum can't
install everything in all relevant databases. Sure, yum will be used to
install dependencies between extensions on the filesystem level.

At the minimum I'd like to see that CREATE EXTENSION foo; would install
install extension 'bar' if foo dependended on 'bar' if CASCADE is
specified. Right now we always error out saying that the dependency on
'bar' is not fullfilled - not particularly helpful.



That's what the proposed patch does (with slightly different syntax but 
syntax is something that can be changed easily).


--
 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] Solaris testers wanted for strxfrm() behavior

2015-07-09 Thread Peter Geoghegan
On Wed, Jul 8, 2015 at 10:18 PM, Noah Misch n...@leadboat.com wrote:
 One function had a comment explaining its workaround for an OS bug, while
 another function ignored the same bug.  That is always a defect in the
 comments at least; our code shall tell a uniform story about its API
 assumptions.  I started this thread estimating that it would end with me
 merely deleting the comment.  Thomas Munro and Tom Lane located evidence I
 hadn't found, evidence that changed the conclusion.

That seems very reasonable. I noticed that you removed the glibc
strxfrm() comment (or at least the questioning of its behavior), which
was a good decision.

 When you have to worry about a standard library function
 blithely writing past the end of a buffer, when its C89 era interface
 must be passed the size of said buffer, where does it end?

 Don't worry about the possibility of such basic bugs until someone reports
 one.  Once you have such a report, though, assume the interface behaves as
 last reported until you receive new evidence.  We decide whether to work
 around such bugs based on factors like prevalence of affected systems,
 simplicity of the workaround, and ease of field diagnosis in the absence of
 the workaround.

I must admit that I was rather surprised that more or less the same
blitheness about writing past the end of a buffer occurred a second
time in an apparently independent standard library implementation. I
think that illustrates your point well.

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

2015-07-09 Thread Pavel Stehule
2015-07-09 15:17 GMT+02:00 Merlin Moncure mmonc...@gmail.com:

 On Wed, Jul 8, 2015 at 11:28 PM, Pavel Stehule pavel.steh...@gmail.com
 wrote:
  Hi
 
  second version of this patch
 
  make check-world passed

 quickly scanning the patch, the implementation is trivial (minus
 regression test adjustments), and is, IMSNSHO, the right solution.


yes, it is right way - the behave of RAISE statement will be much more
cleaner


 Several of the source level comments need some minor wordsmithing and
 the GUCs are missing documentation.  If we've got consensus on the
 approach, I'll pitch in on that.


thank you

Pavel



 merlin



Re: [HACKERS] WAL logging problem in 9.4.3?

2015-07-09 Thread Tom Lane
Fujii Masao masao.fu...@gmail.com writes:
 On Tue, Jul 7, 2015 at 12:49 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 One idea I had was to allow the COPY optimization only if the heap file is
 physically zero-length at the time the COPY starts.

 This seems not helpful for the case where TRUNCATE is executed
 before COPY. No?

Huh?  The heap file would be zero length in that case.

 So, if COPY is executed multiple times at the same transaction,
 only first COPY can be optimized?

This is true, and I don't think we should care, especially not if we're
going to take risks of incorrect behavior in order to optimize that
third-order case.  The fact that we're dealing with this bug at all should
remind us that this stuff is harder than it looks.  I want a simple,
reliable, back-patchable fix, and I do not believe that what you are
suggesting would be any of those.

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

2015-07-09 Thread Merlin Moncure
On Thu, Jul 9, 2015 at 10:48 AM, Pavel Stehule pavel.steh...@gmail.com wrote:

 2015-07-09 15:17 GMT+02:00 Merlin Moncure mmonc...@gmail.com:

 On Wed, Jul 8, 2015 at 11:28 PM, Pavel Stehule pavel.steh...@gmail.com
 wrote:
  Hi
 
  second version of this patch
 
  make check-world passed

 quickly scanning the patch, the implementation is trivial (minus
 regression test adjustments), and is, IMSNSHO, the right solution.


 yes, it is right way - the behave of RAISE statement will be much more
 cleaner


 Several of the source level comments need some minor wordsmithing and
 the GUCs are missing documentation.  If we've got consensus on the
 approach, I'll pitch in on that.

 thank you

revised patch attached. added GUC docs and cleaned up pg_settings
language.  Also tested patch and it works beautifully.

Note, Pavel's patch does adjust default behavior to what we think is
the right settings.

merlin
diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out
new file mode 100644
index a49b562..a268fc7
*** a/contrib/dblink/expected/dblink.out
--- b/contrib/dblink/expected/dblink.out
***
*** 1,3 
--- 1,4 
+ set client_min_context TO notice;
  CREATE EXTENSION dblink;
  CREATE TABLE foo(f1 int, f2 text, f3 text[], primary key (f1,f2));
  INSERT INTO foo VALUES (0,'a','{a0,b0,c0}');
diff --git a/contrib/dblink/sql/dblink.sql b/contrib/dblink/sql/dblink.sql
new file mode 100644
index ea78cc2..cf7e57e
*** a/contrib/dblink/sql/dblink.sql
--- b/contrib/dblink/sql/dblink.sql
***
*** 1,3 
--- 1,5 
+ set client_min_context TO notice;
+ 
  CREATE EXTENSION dblink;
  
  CREATE TABLE foo(f1 int, f2 text, f3 text[], primary key (f1,f2));
diff --git a/contrib/hstore_plperl/expected/hstore_plperlu.out b/contrib/hstore_plperl/expected/hstore_plperlu.out
new file mode 100644
index 8c689ad..c97fd3f
*** a/contrib/hstore_plperl/expected/hstore_plperlu.out
--- b/contrib/hstore_plperl/expected/hstore_plperlu.out
*** INFO:  $VAR1 = {
*** 29,35 
'cc' = undef
  };
  
- CONTEXT:  PL/Perl function test1
   test1 
  ---
   2
--- 29,34 
*** $$;
*** 46,52 
  SELECT test1none('aa=bb, cc=NULL'::hstore);
  INFO:  $VAR1 = 'aa=bb, cc=NULL';
  
- CONTEXT:  PL/Perl function test1none
   test1none 
  ---
   0
--- 45,50 
*** INFO:  $VAR1 = {
*** 67,73 
'cc' = undef
  };
  
- CONTEXT:  PL/Perl function test1list
   test1list 
  ---
   2
--- 65,70 
*** $VAR2 = {
*** 92,98 
'dd' = 'ee'
  };
  
- CONTEXT:  PL/Perl function test1arr
   test1arr 
  --
  2
--- 89,94 
*** INFO:  $VAR1 = {
*** 120,129 
'cc' = undef
  };
  
- CONTEXT:  PL/Perl function test3
  INFO:  $VAR1 = 'a=1, b=boo, c=NULL';
  
- CONTEXT:  PL/Perl function test3
   test3 
  ---
   
--- 116,123 
*** INFO:  $VAR1 = {
*** 161,167 
 }
  };
  
- CONTEXT:  PL/Perl function test4
  SELECT * FROM test1;
   a |b
  ---+-
--- 155,160 
diff --git a/contrib/hstore_plpython/expected/hstore_plpython.out b/contrib/hstore_plpython/expected/hstore_plpython.out
new file mode 100644
index b7a6a92..23091d3
*** a/contrib/hstore_plpython/expected/hstore_plpython.out
--- b/contrib/hstore_plpython/expected/hstore_plpython.out
*** return len(val)
*** 13,19 
  $$;
  SELECT test1('aa=bb, cc=NULL'::hstore);
  INFO:  [('aa', 'bb'), ('cc', None)]
- CONTEXT:  PL/Python function test1
   test1 
  ---
   2
--- 13,18 
*** return len(val)
*** 32,38 
  $$;
  SELECT test1n('aa=bb, cc=NULL'::hstore);
  INFO:  [('aa', 'bb'), ('cc', None)]
- CONTEXT:  PL/Python function test1n
   test1n 
  
2
--- 31,36 
diff --git a/contrib/ltree_plpython/expected/ltree_plpython.out b/contrib/ltree_plpython/expected/ltree_plpython.out
new file mode 100644
index 934529e..c6e8a7c
*** a/contrib/ltree_plpython/expected/ltree_plpython.out
--- b/contrib/ltree_plpython/expected/ltree_plpython.out
*** return len(val)
*** 9,15 
  $$;
  SELECT test1('aa.bb.cc'::ltree);
  INFO:  ['aa', 'bb', 'cc']
- CONTEXT:  PL/Python function test1
   test1 
  ---
   3
--- 9,14 
*** return len(val)
*** 24,30 
  $$;
  SELECT test1n('aa.bb.cc'::ltree);
  INFO:  ['aa', 'bb', 'cc']
- CONTEXT:  PL/Python function test1n
   test1n 
  
3
--- 23,28 
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
new file mode 100644
index b91d6c7..38ae0ad
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*** local0.*/var/log/postgresql
*** 4144,4149 
--- 4144,4171 
  
   variablelist
  
+  varlistentry id=guc-client-min-context xreflabel=client_min_context
+   

Re: [HACKERS] 9.5 release notes

2015-07-09 Thread Peter Geoghegan
On Fri, Jun 26, 2015 at 3:39 PM, Peter Geoghegan p...@heroku.com wrote:
 I attach a compatibility note that is clearly needed; adding this is
 an open item of mine for 9.5. This concerns foreign data wrappers and
 UPSERT.

Are you going to review this, Bruce? It is an open item for 9.5.

I would also like you to look into the other items I've highlighted.

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

2015-07-09 Thread Sawada Masahiko
On Tue, Jul 7, 2015 at 8:49 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Thu, Jul 2, 2015 at 9:00 PM, Sawada Masahiko sawada.m...@gmail.com
 wrote:


 Thank you for bug report, and comments.

 Fixed version is attached, and source code comment is also updated.
 Please review it.


 I am looking into this patch and would like to share my findings with
 you:

Thank you for comment.
I appreciate you taking time to review this patch.


 1.
 @@ -2131,8 +2133,9 @@ heap_insert(Relation relation, HeapTuple tup,
 CommandId cid,

 CheckForSerializableConflictIn(relation, NULL, InvalidBuffer);

   /*
 - * Find buffer to insert this
 tuple into.  If the page is all visible,
 - * this will also pin the requisite visibility map page.
 +
  * Find buffer to insert this tuple into.  If the page is all visible
 + * of all frozen, this will also pin
 the requisite visibility map and
 + * frozen map page.
   */

 typo in comments.

 /of all frozen/or all frozen

Fixed.

 2.
 visibilitymap.c
 + * The visibility map is a bitmap with two bits (all-visible and all-frozen
 + * per heap page.

 /and all-frozen/and all-frozen)
 closing round bracket is missing.

Fixed.

 3.
 visibilitymap.c
 -/*#define TRACE_VISIBILITYMAP */
 +#define TRACE_VISIBILITYMAP

 why is this hash define opened?

Fixed.

 4.
 -visibilitymap_count(Relation rel)
 +visibilitymap_count(Relation rel, bool for_visible)

 This API needs to count set bits for either visibility info, frozen info
 or both (if required), it seems better to have second parameter as
 uint8 flags rather than bool. Also, if it is required to be called at most
 places for both visibility and frozen bits count, why not get them
 in one call?

Fixed.

 5.
 Clearing visibility and frozen bit separately for the dml
 operations would lead locking/unlocking the corresponding buffer
 twice, can we do it as a one operation.  I think this is suggested
 by Simon as well.

Latest patch clears bits in one operation, and set all-frozen with
all-visible in one operation.
We can judge the page is all-frozen in two places: first scanning the
page(lazy_scan_heap), and after cleaning garbage(lazy_vacuum_page).

 6.
 - * Before locking the buffer, pin the visibility map page if it appears to
 - * be necessary.
 Since we haven't got the lock yet, someone else might be
 + * Before locking the buffer, pin the
 visibility map if it appears to be
 + * necessary.  Since we haven't got the lock yet, someone else might
 be

 Why you have deleted 'page' in above comment?

Fixed.

 7.
 @@ -3490,21 +3532,23 @@ l2:
   UnlockTupleTuplock(relation, (oldtup.t_self), *lockmode);

 if (vmbuffer != InvalidBuffer)
   ReleaseBuffer(vmbuffer);
 +
   bms_free
 (hot_attrs);

 Seems unnecessary change.

Fixed.

 8.
 @@ -1919,11 +1919,18 @@ index_update_stats(Relation rel,
   {
   BlockNumber relpages =
 RelationGetNumberOfBlocks(rel);
   BlockNumber relallvisible;
 + BlockNumber
 relallfrozen;

   if (rd_rel-relkind != RELKIND_INDEX)
 - relallvisible =
 visibilitymap_count(rel);
 + {
 + relallvisible = visibilitymap_count(rel,
 true);
 + relallfrozen = visibilitymap_count(rel, false);
 + }
   else
 /* don't bother for indexes */
 + {
   relallvisible = 0;
 +
 relallfrozen = 0;
 + }

 I think in this function, you have forgotten to update the
 relallfrozen value in pg_class.

Fixed.

 9.
 vacuumlazy.c

 @@ -253,14 +258,16 @@ lazy_vacuum_rel(Relation onerel, int options,
 VacuumParams *params,
   * NB: We
 need to check this before truncating the relation, because that
   * will change -rel_pages.
   */
 -
 if (vacrelstats-scanned_pages  vacrelstats-rel_pages)
 + if ((vacrelstats-scanned_pages +
 vacrelstats-vmskipped_pages)
 +  vacrelstats-rel_pages)
   {
 - Assert(!scan_all);

 Why you have removed this Assert, won't the count of
 vacrelstats-scanned_pages + vacrelstats-vmskipped_pages be
 equal to vacrelstats-rel_pages when scall_all = true.

Fixed.

 10.
 vacuumlazy.c
 lazy_vacuum_rel()
 ..
 + scanned_all |= scan_all;
 +

 Why this new assignment is added, please add a comment to
 explain it.

It's not necessary, removed.

 11.
 lazy_scan_heap()
 ..
 + * Also, skipping even a single page accorind to all-visible bit of
 + * visibility map means that we can't update relfrozenxid, so we only want
 + * to do it if we can skip a goodly number. On the other hand, we count
 + * both how many pages we skipped according to all-frozen bit of visibility
 + * map and how many pages we freeze page, so we can update relfrozenxid if
 + * the sum of their is as many as tuples per page.

 a.
 typo
 /accorind/according

Fixed.

 b.
 is the second part of comment (starting from On the other hand)
 right?  I mean you are comparing sum of pages skipped due to
 all_frozen bit and number of pages freezed with tuples per page.
 I don't understand how are they related?


It's wrong, I wanted to say at last sentence that, so we can update
relfrozenxid if the sum of them is as many as pages of table.

 12.
 @@ -918,8 +954,13 @@ 

Re: [HACKERS] WAL logging problem in 9.4.3?

2015-07-09 Thread Andres Freund
On 2015-07-06 11:49:54 -0400, Tom Lane wrote:
 One idea I had was to allow the COPY optimization only if the heap file is
 physically zero-length at the time the COPY starts.  That would still be
 able to optimize in all the cases we care about making COPY fast for.
 Rather than reverting cab9a0656c36739f, which would re-introduce a
 different performance problem, perhaps we could have COPY create a new
 relfilenode when it does this.  That should be safe if the table was
 previously empty.

I'm not convinced that cab9a0656c36739f needs to survive in that
form. To me only allowing one COPY to benefit from the wal_level =
minimal optimization has a significantly higher cost than
cab9a0656c36739f.

My tentative guess is that the best course is to

a) Make heap_truncate_one_rel() create a new relfeilnode. That fixes the
   truncation replay issue.

b) Force new pages to be used when using the heap_sync mode in
   COPY. That avoids the INIT danger you found. It seems rather
   reasonable to avoid using pages that have already been the target of
   WAL logging here in general.

Andres


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


Re: [HACKERS] [PATCH] Generalized JSON output functions

2015-07-09 Thread Pavel Stehule
2015-07-03 12:27 GMT+02:00 Heikki Linnakangas hlinn...@iki.fi:

 On 05/27/2015 09:51 PM, Andrew Dunstan wrote:


 On 05/27/2015 02:37 PM, Robert Haas wrote:

 On Tue, May 26, 2015 at 2:50 AM, Shulgin, Oleksandr
 oleksandr.shul...@zalando.de wrote:

 Is it reasonable to add this patch to CommitFest now?

 It's always reasonable to add a patch to the CommitFest if you would
 like for it to be reviewed and avoid having it get forgotten about.
 There seems to be some disagreement about whether we want this, but
 don't let that stop you from adding it to the next CommitFest.


 I'm not dead set against it either. When I have time I will take a
 closer look.


 Andrew, will you have the time to review this? Please add yourself as
 reviewer in the commitfest app if you do.

 My 2 cents is that I agree with your initial reaction: This is a lot of
 infrastructure and generalizing things, for little benefit. Let's change
 the current code where we generate JSON to be consistent with whitespace,
 and call it a day.


I am  thinking so it is not bad idea. This code can enforce uniform format,
and it can check if produced value is correct. It can be used in our code,
it can be used by extension's developers.

This patch is not small, but really new lines are not too much.

I'll do review today.

Regards

Pavel




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

2015-07-09 Thread Sawada Masahiko
On Fri, Jul 10, 2015 at 3:05 AM, Sawada Masahiko sawada.m...@gmail.com wrote:

 Also something for pg_upgrade is also not yet.

 TODO
 - Test case for this feature
 - pg_upgrade support.


I had forgotten to change the fork name of visibility map to vfm.
Attached latest v7 patch.
Please review it.

Regards,

--
Sawada Masahiko
diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c
index 22c5f7a..b1b6a06 100644
--- a/contrib/pgstattuple/pgstatapprox.c
+++ b/contrib/pgstattuple/pgstatapprox.c
@@ -87,7 +87,7 @@ statapprox_heap(Relation rel, output_type *stat)
 		 * If the page has only visible tuples, then we can find out the free
 		 * space from the FSM and move on.
 		 */
-		if (visibilitymap_test(rel, blkno, vmbuffer))
+		if (visibilitymap_test(rel, blkno, vmbuffer, VISIBILITYMAP_ALL_VISIBLE))
 		{
 			freespace = GetRecordedFreeSpace(rel, blkno);
 			stat-tuple_len += BLCKSZ - freespace;
diff --git a/src/backend/access/heap/Makefile b/src/backend/access/heap/Makefile
index b83d496..806ce27 100644
--- a/src/backend/access/heap/Makefile
+++ b/src/backend/access/heap/Makefile
@@ -12,6 +12,7 @@ subdir = src/backend/access/heap
 top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 
-OBJS = heapam.o hio.o pruneheap.o rewriteheap.o syncscan.o tuptoaster.o visibilitymap.o
+OBJS = heapam.o hio.o pruneheap.o rewriteheap.o syncscan.o tuptoaster.o visibilitymap.o \
+	heapfuncs.o
 
 include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 86a2e6b..ac74100 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -88,7 +88,7 @@ static HeapTuple heap_prepare_insert(Relation relation, HeapTuple tup,
 static XLogRecPtr log_heap_update(Relation reln, Buffer oldbuf,
 Buffer newbuf, HeapTuple oldtup,
 HeapTuple newtup, HeapTuple old_key_tup,
-bool all_visible_cleared, bool new_all_visible_cleared);
+			bool all_visible_cleared, bool new_all_visible_cleared);
 static void HeapSatisfiesHOTandKeyUpdate(Relation relation,
 			 Bitmapset *hot_attrs,
 			 Bitmapset *key_attrs, Bitmapset *id_attrs,
@@ -2131,8 +2131,9 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
 	CheckForSerializableConflictIn(relation, NULL, InvalidBuffer);
 
 	/*
-	 * Find buffer to insert this tuple into.  If the page is all visible,
-	 * this will also pin the requisite visibility map page.
+	 * Find buffer to insert this tuple into.  If the page is all visible
+	 * or all frozen, this will also pin the requisite visibility map and
+	 * frozen map page.
 	 */
 	buffer = RelationGetBufferForTuple(relation, heaptup-t_len,
 	   InvalidBuffer, options, bistate,
@@ -2147,10 +2148,13 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
 	if (PageIsAllVisible(BufferGetPage(buffer)))
 	{
 		all_visible_cleared = true;
+
+		/* all-frozen information is also cleared at the same time */
 		PageClearAllVisible(BufferGetPage(buffer));
+		PageClearAllFrozen(BufferGetPage(buffer));
+
 		visibilitymap_clear(relation,
-			ItemPointerGetBlockNumber((heaptup-t_self)),
-			vmbuffer);
+			ItemPointerGetBlockNumber((heaptup-t_self)), vmbuffer);
 	}
 
 	/*
@@ -2448,10 +2452,12 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
 		if (PageIsAllVisible(page))
 		{
 			all_visible_cleared = true;
+
+			/* all-frozen information is also cleared at the same time */
 			PageClearAllVisible(page);
-			visibilitymap_clear(relation,
-BufferGetBlockNumber(buffer),
-vmbuffer);
+			PageClearAllFrozen(page);
+
+			visibilitymap_clear(relation, BufferGetBlockNumber(buffer), vmbuffer);
 		}
 
 		/*
@@ -2495,7 +2501,9 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
 			/* the rest of the scratch space is used for tuple data */
 			tupledata = scratchptr;
 
-			xlrec-flags = all_visible_cleared ? XLH_INSERT_ALL_VISIBLE_CLEARED : 0;
+			if (all_visible_cleared)
+xlrec-flags = XLH_INSERT_ALL_VISIBLE_CLEARED;
+
 			xlrec-ntuples = nthispage;
 
 			/*
@@ -2731,9 +2739,9 @@ heap_delete(Relation relation, ItemPointer tid,
 
 	/*
 	 * If we didn't pin the visibility map page and the page has become all
-	 * visible while we were busy locking the buffer, we'll have to unlock and
-	 * re-lock, to avoid holding the buffer lock across an I/O.  That's a bit
-	 * unfortunate, but hopefully shouldn't happen often.
+	 * visible or all frozen while we were busy locking the buffer, we'll
+	 * have to unlock and re-lock, to avoid holding the buffer lock across an
+	 * I/O.  That's a bit unfortunate, but hopefully shouldn't happen often.
 	 */
 	if (vmbuffer == InvalidBuffer  PageIsAllVisible(page))
 	{
@@ -2925,12 +2933,16 @@ l1:
 	 */
 	PageSetPrunable(page, xid);
 
+	/* clear PD_ALL_VISIBLE and PD_ALL_FORZEN flags */
 	if (PageIsAllVisible(page))
 	{
 		all_visible_cleared = true;
+
+		/* all-frozen information is also cleared 

Re: [HACKERS] Fillfactor for GIN indexes

2015-07-09 Thread Michael Paquier
On Thu, Jul 9, 2015 at 10:33 PM, Alexander Korotkov wrote:
 [...]

+ /* Caclculate max data size on page according to fillfactor */
s/Caclculate/Calculate

When creating a simple gin index, I am seeing that GinGetMaxDataSize gets
called with ginEntryInsert:
  * frame #0: 0x00010a49d72e
postgres`GinGetMaxDataSize(index=0x000114168378, isBuild='\x01') + 14
at gindatapage.c:436
frame #1: 0x00010a49ecbe
postgres`createPostingTree(index=0x000114168378,
items=0x000114a9c038, nitems=35772, buildStats=0x7fff55787350) +
302 at gindatapage.c:1754
frame #2: 0x00010a493423
postgres`buildFreshLeafTuple(ginstate=0x7fff55784d90, attnum=1,
key=2105441, category='\0', items=0x000114a9c038, nitem=35772,
buildStats=0x7fff55787350) + 339 at gininsert.c:158
frame #3: 0x00010a492f84
postgres`ginEntryInsert(ginstate=0x7fff55784d90, attnum=1, key=2105441,
category='\0', items=0x000114a9c038, nitem=35772,
buildStats=0x7fff55787350) + 916 at gininsert.c:228

And as far as I can see GinGetMaxDataSize uses isBuild:
+ int
+ GinGetMaxDataSize(Relation index, bool isBuild)
+ {
+ int fillfactor, result;
+
+ /* Grab option value which affects only index build */
+ if (!isBuild)
However isBuild is not set in this code path, see
http://www.postgresql.org/message-id/cab7npqsc4vq9mhkqm_yvafcteho-iuy8skbxydnmgnai1xn...@mail.gmail.com
where I reported the problem. So this code is somewhat broken, no? I am
attaching to this email the patch in question that should be applied on top
of Alexander's latest version.

+ #define GIN_MIN_FILLFACTOR20
+ #define GIN_DEFAULT_FILLFACTOR90
I am still worrying about using a default fillfactor at 90, as we did a lot
of promotion about compression of GIN indexes in 9.4. Feel free to ignore
this comment if you think that 90 is a good default. The difference is
visibly in order of MBs even for large indexes still, this is changing the
default of 9.4 and 9.5.
Regards,
-- 
Michael
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index 370884e..229167b 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -191,6 +191,7 @@ ginEntryInsert(GinState *ginstate,
 		buildStats-nEntries++;
 
 	ginPrepareEntryScan(btree, attnum, key, category, ginstate);
+	btree.isBuild = (buildStats != NULL);
 
 	stack = ginFindLeafPage(btree, false);
 	page = BufferGetPage(stack-buffer);

-- 
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] Implementation of global temporary tables?

2015-07-09 Thread Zhaomo Yang
  I am not sure, if it is not useless work.

I don't understand why an implementation taking approach 2.a would be
useless. As I said, its performance will be no worse than current temp
tables and it will provide a lot of convenience to users who need to create
temp tables in every session.

Thanks,
Zhaomo

On Tue, Jul 7, 2015 at 11:53 PM, Pavel Stehule pavel.steh...@gmail.com
wrote:

 Hi


 2015-07-08 9:08 GMT+02:00 Zhaomo Yang zhy...@cs.ucsd.edu:

   more global temp tables are little bit comfortable for developers,
 I'd like to emphasize this point. This feature does much more than saving
 a developer from issuing a CREATE TEMP TABLE statement in every session.
 Here are two common use cases and I'm sure there are more.

 (1)
 Imagine in a web application scenario, a developer wants to cache some
 session information in a temp table. What's more, he also wants to specify
 some rules which reference the session information. Without this feature,
 the rules will be removed at the end of every session since they depend on
 a temporary object. Global temp tables will allow the developer to define
 the temp table and the rules once.

 (2)
 The second case is mentioned by Tom Lane back in 2010 in a thread about
 global temp tables.
 (http://www.postgresql.org/message-id/9319.1272130...@sss.pgh.pa.us)
 The context that I've seen it come up in is that people don't want to
 clutter their functions with
  create-it-if-it-doesn't-exist logic, which you have to have given the
 current behavior of temp tables.

   2.a - using on demand created temp tables - most simple solution, but
   doesn't help with catalogue bloating

 I've read the thread and people disapprove this approach because of the
 potential catalog bloat. However, I'd like to champion it. Indeed, this
 approach may have a bloat issue. But for users who needs global temp
 tables, they now have to create a new temp table in every session, which
 means they already have the bloat problem and presumably they have some
 policy to deal with it. In other words, implementing global temp tables by
 this approach gives users the same performance, plus the convenience the
 feature brings.

 The root problem here is that whether whether having the unoptimized
 feature is better than
 having no feature at all. Actually, there was a very similar discussion
 back in 2009 on global temp tables. Let me borrow Kevin Grittner's and Tom
 Lane's arguments here.

 Kevin Grittner's argument:

 http://www.postgresql.org/message-id/49f82aea.ee98.002...@wicourts.gov
 ... If you're saying we can implement the standard's global temporary
 tables in a way that performs better than current temporary tables, that's
 cool.  That would be a nice bonus in addition to the application
 programmer convenience and having another tick-mark on the standards
 compliance charts.  Do you think that's feasible?  If not, the feature
 would be useful to some with the same performance that temporary tables
 currently provide.

 Tom Lane's arguments:

 http://www.postgresql.org/message-id/24110.1241035...@sss.pgh.pa.us
 I'm all for eliminating catalog overheads, if we can find a way to do
 that.  I don't think that you get to veto implementation of the feature
 until we can find a way to optimize it better.  The question is not about
 whether having the optimization would be better than not having it --- it's
 about whether having the unoptimized feature is better than having no
 feature at all (which means people have to implement the same behavior by
 hand, and they'll *still* not get the optimization).

 There have been several threads here discussing global temp table since
 2007. Quite a few ideas aimed to avoid the bloat issue by not storing the
 metadata of the session copy in the catalog. However, it seems that none of
 them has been implemented, or even has a feasible design. So why don't we
 implement it in a unoptimized way first?


 I am not sure, if it is not useless work.

 Now, I am thinking so best implementation of global temp tables is
 enhancing unlogged tables to have local content. All local data can be
 saved in session memory. Usually it is less than 2KB with statistic, and
 you don't need to store it in catalogue. When anybody is working with any
 table, related data are copied to system cache - and there can be injected
 a implementation of global temp tables.

 regards

 Pavel Stehule



   Is there still interest about this feature?
 I'm very interested in this feature. I'm thinking about one
 implementation which is similar to Pavel's 2009 proposal (
 http://www.postgresql.org/message-id/162867790904271344s1ec96d90j6cde295fdcc78...@mail.gmail.com).
 Here are the major ideas of my design:

 (1)
 Creating the cross-session persistent schema as a regular table and
 creating session-private temp tables when a session first accesses it.

 (2)
 For DML queries, The global temp table is overloaded by its session copy
 after the relation is opened by an oid or a rangevar. For 

Re: [HACKERS] Fix to expose a GUC variable, Log_disconnections, to outside of postgres.c

2015-07-09 Thread Satoshi Nagayasu


On 2015/07/09 15:30, Heikki Linnakangas wrote:
 On 07/09/2015 07:19 AM, Satoshi Nagayasu wrote:

 On 2015/07/09 13:06, Tom Lane wrote:
 Satoshi Nagayasu sn...@uptime.jp writes:
 I just found that Log_disconnections value has not been
 exposed to outside of postgres.c.
 Despite that, Log_connections has already been exposed.

 Why would an external module need to touch either one?

 To check settings of GUC variables from a shared preload
 library.

 For example, I'd like to print WARNING  in _PG_init()
 when some GUC variable is disabled on postmaster startup.
 
 I still have a hard time seeing why an extension would be interested in
 Log_disconnections. But hey, extensions do strange things..

Definitely. :)

 You could use GetConfigOption(). I'm not necessarily opposed to exposing
 Log_disconnections, but with GetConfigOption() it will work with earlier
 versions too.

That's it! This is what I'm looking for. Thanks a lot. :)

Regards,
-- 
NAGAYASU Satoshi sn...@uptime.jp


-- 
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] optimizing vacuum truncation scans

2015-07-09 Thread Haribabu Kommi
On Thu, Jul 9, 2015 at 4:37 PM, Jeff Janes jeff.ja...@gmail.com wrote:
 On Wed, Jul 8, 2015 at 9:46 PM, Haribabu Kommi kommi.harib...@gmail.com
 wrote:

 On Mon, Jun 29, 2015 at 3:54 PM, Jeff Janes jeff.ja...@gmail.com wrote:
 
  There is still the case of pages which had their visibility bit set by a
  prior vacuum and then were not inspected by the current one.  Once the
  truncation scan runs into these pages, it falls back to the previous
  behavior of reading block by block backwards.  So there could still be
  reason to optimize that fallback using forward-reading prefetch.

 The case, I didn't understand is that, how the current vacuum misses
 the page which
 was set by the prior vacuum?


 The prior vacuum set them to all visible, but then doesn't delete them.
 Either because it got interrupted or because there were still some pages
 after them (at the time) that were not all empty.

 The current vacuum skips them entirely on the forward scan because it thinks
 it is waste of time to vacuum all visible pages.  Since it skips them, it
 doesn't know if they are empty as well as being all-visible.  There is no
 permanent indicator of the pages being all-empty, there is only the
 inference based on the current vacuum's counters and protected by the lock
 held on the table.



 The page should be counted either in skipped_pages or in
 nonempty_pages. Is it possible
 that a page doesn't comes under these two categories and it is not empty
 also?

 If the above doesn't exist, then we can directly truncate the relation
 from the highest block
 number of either nonempty_pages or skipped_pages to till the end of
 the relation.


 Right, and that is what this does (provided the vm bit is still set, so it
 does still have to loop over the vm to verify that it is still set, while it
 holds the access exclusive lock).

Thanks I got it. To re-verify the vm bit of a page after getting the
access exclusive lock,
we have to do the backward scan.

I also feel that your prefetch buffer patch needed to improve the
performance, as because
there is a need of backward scan to re-verify vm bit,

I will do some performance tests and send you the results.

 The problem is that the pages between the two counters are not known to be
 empty, and also not known to be nonempty.  Someone has to be willing to go
 check on those pages at some point, or else they will never be eligible for
 truncation.

Yes, there is no way to identify the page is empty or not without
reading the page.

Regards,
Hari Babu
Fujitsu Australia


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


Re: [HACKERS] copy.c handling for RLS is insecure

2015-07-09 Thread Andres Freund
On 2015-07-09 01:28:28 -0400, Noah Misch wrote:
  - Keep the OID check, shouldn't hurt to have it
 
 What benefit is left?

A bit of defense in depth. We execute user defined code in COPY
(e.g. BEFORE triggers). That user defined code could very well replace
the relation. Now I think right now that'd happen late enough, so the
second lookup already happened. But a bit more robust defense against
that sounds good to me.


-- 
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 to expose a GUC variable, Log_disconnections, to outside of postgres.c

2015-07-09 Thread Heikki Linnakangas
On 07/09/2015 07:19 AM, Satoshi Nagayasu wrote:
 
 On 2015/07/09 13:06, Tom Lane wrote:
 Satoshi Nagayasu sn...@uptime.jp writes:
 I just found that Log_disconnections value has not been
 exposed to outside of postgres.c.
 Despite that, Log_connections has already been exposed.

 Why would an external module need to touch either one?
 
 To check settings of GUC variables from a shared preload
 library.
 
 For example, I'd like to print WARNING  in _PG_init()
 when some GUC variable is disabled on postmaster startup.

I still have a hard time seeing why an extension would be interested in
Log_disconnections. But hey, extensions do strange things..

You could use GetConfigOption(). I'm not necessarily opposed to exposing
Log_disconnections, but with GetConfigOption() it will work with earlier
versions too.

- 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] optimizing vacuum truncation scans

2015-07-09 Thread Jeff Janes
On Wed, Jul 8, 2015 at 9:46 PM, Haribabu Kommi kommi.harib...@gmail.com
wrote:

 On Mon, Jun 29, 2015 at 3:54 PM, Jeff Janes jeff.ja...@gmail.com wrote:
 
  Attached is a patch that implements the vm scan for truncation.  It
  introduces a variable to hold the last blkno which was skipped during the
  forward portion.  Any blocks after both this blkno and after the last
  inspected nonempty page (which the code is already tracking) must have
 been
  observed to be empty by the current vacuum.  Any other process rendering
 the
  page nonempty are required to clear the vm bit, and no other process can
 set
  the bit again during the vacuum's lifetime.  So if the bit is still set,
 the
  page is still empty without needing to inspect it.

 The patch looks good and it improves the vacuum truncation speed
 significantly.

  There is still the case of pages which had their visibility bit set by a
  prior vacuum and then were not inspected by the current one.  Once the
  truncation scan runs into these pages, it falls back to the previous
  behavior of reading block by block backwards.  So there could still be
  reason to optimize that fallback using forward-reading prefetch.

 The case, I didn't understand is that, how the current vacuum misses
 the page which
 was set by the prior vacuum?


The prior vacuum set them to all visible, but then doesn't delete them.
Either because it got interrupted or because there were still some pages
after them (at the time) that were not all empty.

The current vacuum skips them entirely on the forward scan because it
thinks it is waste of time to vacuum all visible pages.  Since it skips
them, it doesn't know if they are empty as well as being all-visible.
There is no permanent indicator of the pages being all-empty, there is only
the inference based on the current vacuum's counters and protected by the
lock held on the table.



 The page should be counted either in skipped_pages or in
 nonempty_pages. Is it possible
 that a page doesn't comes under these two categories and it is not empty
 also?

 If the above doesn't exist, then we can directly truncate the relation
 from the highest block
 number of either nonempty_pages or skipped_pages to till the end of
 the relation.


Right, and that is what this does (provided the vm bit is still set, so it
does still have to loop over the vm to verify that it is still set, while
it holds the access exclusive lock).

The problem is that the pages between the two counters are not known to be
empty, and also not known to be nonempty.  Someone has to be willing to go
check on those pages at some point, or else they will never be eligible for
truncation.

Cheers,

Jeff


Re: [HACKERS] Further issues with jsonb semantics, documentation

2015-07-09 Thread Peter Geoghegan
On Mon, Jun 22, 2015 at 6:19 PM, Peter Geoghegan p...@heroku.com wrote:
 On Wed, Jun 10, 2015 at 11:48 AM, Andrew Dunstan and...@dunslane.net wrote:
 Please submit a patch to adjust the treatment of negative integers in the
 old functions to be consistent with their treatment in the new functions.
 i.e. in the range [-n,-1] they should refer to the corresponding element
 counting from the right.

 This patch is attached, along with a separate patch which adds a
 release note compatibility item.

Where are we on this? This is currently a 9.5 release blocker.

-- 
Peter Geoghegan


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


[HACKERS] [ANNOUNCE] PGDay.IT 2015 - Call for Papers

2015-07-09 Thread desmodemone
The 9th edition of the Italian PostgreSQL Day ( PGDay.it 2015 ) will be
held on Friday , 23th October 2015 in Prato, Italy.
The International Call for Papers has opened and will close on 8th August
2015.

For more information about the conference and the International Call for
Paper, please visit  :

*http://2015.pgday.it/ http://2015.pgday.it/*

Information in English for papers submission is available at :
http://2015.pgday.it/?page_id=43

For any kind of information, do not hesitate to contact the committee via
email at *pg...@itpug.org* pg...@itpug.org  .



Kind Regards

-- 
Matteo Durighetto

- - - - - - - - - - - - - - - - - - - - - - -

Italian PostgreSQL User Group http://www.itpug.org/index.it.html
Italian Community for Geographic Free/Open-Source Software
http://www.gfoss.it


Re: [HACKERS] Sharing aggregate states between different aggregate functions

2015-07-09 Thread David Rowley
On 15 June 2015 at 12:05, David Rowley david.row...@2ndquadrant.com wrote:


 This basically allows an aggregate's state to be shared between other
 aggregate functions when both aggregate's transition functions (and a few
 other things) match
 There's quite a number of aggregates in our standard set which will
 benefit from this optimisation.


After compiling the original patch with another compiler, I noticed a
couple of warnings.

The attached fixes these.

Regards

David Rowley

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


sharing_agg_states_5f0bff89_2015-07-09.patch
Description: Binary data

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


Re: [HACKERS] Improving log capture of TAP tests with IPC::Run

2015-07-09 Thread Heikki Linnakangas

On 07/09/2015 04:50 AM, Michael Paquier wrote:

On Thu, Jul 9, 2015 at 12:49 AM, Heikki Linnakangas wrote:

Looking at the manual page of Test::More, it looks like you could change
where the perl script's STDOUT and STDERR point to, because Test::More
takes
a copy of them (when? at program startup I guess..). That would be much
more
convenient than decorating every run call with  logfile.



Hm. There are two types of logs we want to capture:
1) stdout and stderr from the subprocesses kicked by IPC::Run::run
2) Status messages written in the log file by the process running the
tests.
Perhaps we could redirect the output of stdout and stderr but I think
that this is going to need an fd open from the beginning of the test
until the end, with something like that:
open my $test_logfile_fd, '', $test_logfile;
*STDOUT = $test_logfile_fd;
*STDERR = $test_logfile_fd;

While that would work on OSX and Linux for sure, I suspect that this
will not on Windows where two concurrent processes cannot write to the
same file.


Hmm, as long as you make sure all the processes use the same filehandle,
rather than open the log file separately, I think it should work. But it's
Windows, so who knows..


And actually your patch works even on Windows. Tests are running and
log can be captured in the same shape as Linux and OSX!


Great!


I came up with the attached, which does that. It also plays some tricks with
perl tie, to copy the ok - ... lines that go to the console, to the log.

I tried to test that on my Windows system, but I don't have IPC::Run
installed. How did you get that on Windows? Can you test this?


I simply downloaded them from CPAN and put them in PERL5LIB. And it
worked. For Windows, you will also need some adjustments to make the
tests able to run (see the other thread related to support TAP in MSVC
http://www.postgresql.org/message-id/cab7npqtqwphkdfzp07w7ybnbfndhw_jbamycfakare2vwg8...@mail.gmail.com)
like using SSPI for connection, adjusting listen_addresses. The patch
attached, which is a merge of your patch and my MSVC stuff, is able to
do that. This is just intended for testing, so feel free to use it if
you want to check by yourself how logs are captured. I'll repost a new
version of it on the other thread depending on the outcome here.

-system_or_bail(
-pg_basebackup -D $test_standby_datadir -p $port_master -x $log_path 21);
+system_or_bail('pg_basebackup', '-D', $test_standby_datadir,
+   '-p', $port_master, '-x';
A parenthesis is missing here.

-   my $result = run(
-   [   'pg_rewind',
-   --source-server,
-   port=$port_standby dbname=postgres,
-   --target-pgdata=$test_master_datadir ],
-   '',
-   $log_path,
-   '21');
-   ok($result, 'pg_rewind remote');
+   command_ok(['pg_rewind',
+   --source-server,
+   port=$port_standby dbname=postgres,
+   --target-pgdata=$test_master_datadir],
+  'pg_rewind remote');
As long as we are on it, I think that we should add --debug here to
make the logs more useful.

Except that this patch looks good to me. Thanks for the black magic on
stdout/stderr handling.


Thanks, fixed the parenthesis and committed. The missing --debug is a 
separate issue.


- 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] FPW compression leaks information

2015-07-09 Thread Michael Paquier
On Wed, Jul 8, 2015 at 11:33 AM, Fujii Masao wrote:
 ISTM that one our consensus is to make wal_compression SUSET
 as the first step whatever approach we adopt for the risk in question
 later. So, barring any objection, I will commit the attached patch
 and change the context to PGC_SUSET.

That's surely a step in the good direction. Thanks.
-- 
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] Improving log capture of TAP tests with IPC::Run

2015-07-09 Thread Michael Paquier
On Thu, Jul 9, 2015 at 7:29 PM, Heikki Linnakangas hlinn...@iki.fi wrote:
 Thanks, fixed the parenthesis and committed.

Thanks. I imagine that this is going to need some tuning with the
buildfarm before this becomes really useful. I will re-enable the TAP
tests of hamster once that's the case.
@Andrew: do you need a patch for the buildfarm client?

 The missing --debug is a separate issue.

I won't argue against that, but is it worth creating a new thread just
for this one-liner? The local mode of pg_rewind uses it already.
-- 
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] PL/pgSQL, RAISE and error context

2015-07-09 Thread Merlin Moncure
On Wed, Jul 8, 2015 at 11:28 PM, Pavel Stehule pavel.steh...@gmail.com wrote:
 Hi

 second version of this patch

 make check-world passed

quickly scanning the patch, the implementation is trivial (minus
regression test adjustments), and is, IMSNSHO, the right solution.

Several of the source level comments need some minor wordsmithing and
the GUCs are missing documentation.  If we've got consensus on the
approach, I'll pitch in on that.

merlin


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


Re: [HACKERS] Improving log capture of TAP tests with IPC::Run

2015-07-09 Thread Heikki Linnakangas

On 07/09/2015 04:09 PM, Michael Paquier wrote:

On Thu, Jul 9, 2015 at 7:29 PM, Heikki Linnakangas hlinn...@iki.fi wrote:

The missing --debug is a separate issue.


I won't argue against that, but is it worth creating a new thread just
for this one-liner? The local mode of pg_rewind uses it already.


Pushed, thanks.

- 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] FPW compression leaks information

2015-07-09 Thread Fujii Masao
On Thu, Jul 9, 2015 at 10:05 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
 On Wed, Jul 8, 2015 at 11:33 AM, Fujii Masao wrote:
 ISTM that one our consensus is to make wal_compression SUSET
 as the first step whatever approach we adopt for the risk in question
 later. So, barring any objection, I will commit the attached patch
 and change the context to PGC_SUSET.

 That's surely a step in the good direction. Thanks.

Yep, pushed!

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] Fillfactor for GIN indexes

2015-07-09 Thread Alexander Korotkov
Hi!

On Wed, Jul 8, 2015 at 10:27 PM, Heikki Linnakangas hlinn...@iki.fi wrote:

 In dataPlaceToPageLeaf-function:

  if (append)
 {
 /*
  * Even when appending, trying to append more items than
 will fit is
  * not completely free, because we will merge the new
 items and old
  * items into an array below. In the best case, every new
 item fits in
  * a single byte, and we can use all the free space on
 the old page as
  * well as the new page. For simplicity, ignore segment
 overhead etc.
  */
 maxitems = Min(maxitems, freespace +
 GinDataPageMaxDataSize);
 }


 Hmm. So after splitting the page, there is freespace +
 GinDataPageMaxDataSize bytes available on both pages together. freespace
 has been adjusted with the fillfactor, but GinDataPageMaxDataSize is not.
 So this overshoots, because when leafRepackItems actually distributes the
 segments on the pages, it will fill both pages only up to the fillfactor.
 This is an upper bound, so that's harmless, it only leads to some
 unnecessary work in dealing with the item lists. But I think that should be:

 maxitems = Min(maxitems, freespace + leaf-maxdatasize);


Good catch! Fixed.



  else
 {
 /*
  * Calculate a conservative estimate of how many new
 items we can fit
  * on the two pages after splitting.
  *
  * We can use any remaining free space on the old page to
 store full
  * segments, as well as the new page. Each full-sized
 segment can hold
  * at least MinTuplesPerSegment items
  */
 int nnewsegments;

 nnewsegments = freespace / GinPostingListSegmentMaxSize;
 nnewsegments += GinDataPageMaxDataSize /
 GinPostingListSegmentMaxSize;
 maxitems = Min(maxitems, nnewsegments *
 MinTuplesPerSegment);
 }


 This branch makes the same mistake, but this is calculating a lower bound.
 It's important that maxitems is not set to higher value than what actually
 fits on the page, otherwise you can get an ERROR later in the function,
 when it turns out that not all the items actually fit on the page. The
 saving grace here is that this branch is never taken when building a new
 index, because index build inserts all the TIDs in order, but that seems
 pretty fragile. Should use leaf-maxdatasize instead of
 GinDataPageMaxDataSize here too.


Fixed.


 But that can lead to funny things, if fillfactor is small, and BLCKSZ is
 small too. With the minimums, BLCKSZ=1024 and fillfactor=0.2, the above
 formula will set nnewsegments to zero. That's not going to end up well. The
 problem is that maxdatasize becomes smaller than
 GinPostingListSegmentMaxSize, which is a problem. I think
 GinGetMaxDataSize() needs to make sure that the returned value is always =
 GinPostingListSegmentMaxSize.


Fixed.


 Now that we have a fillfactor, shouldn't we replace the 75% heuristic
 later in that function, when inserting to the rightmost page rather than
 building it from scratch? In B-tree, the fillfactor is applied to that case
 too.


Sounds reasonable. Now it works like btree.

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


gin_fillfactor_6.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] Re: [HACKERS] Re: [HACKERS] GSoC 2015 proposal: Improve the performance of “ALTER TABLE .. SET LOGGED / UNLOGGED” statement

2015-07-09 Thread Fabrízio de Royes Mello
On Wed, Jul 8, 2015 at 11:37 AM, Andres Freund and...@anarazel.de wrote:

 On 2015-07-08 10:58:51 -0300, Fabrízio de Royes Mello wrote:
  Think in an ETL job that can be use an unlogged table to improve the
load
  performance, but this job create a large table and to guarantee the
data
  consistency you need to transform it into a regular table, and with the
  current implementation rewrite the entire heap, toast and indexes.

 Don't buy that. The final target relation will usually already have
 content. Also everything but wal_level=minimal will force you to WAL log
 the contents anyway.

If the wal_level=minimal we don't need to force the wal log of the
contents. If the wal_level != minimal we need just to xlog all the pages,
but in both cases we don't need the extra job to create a new datafiles and
copy the contents between them. So we'll improve performance, or am I
missing something?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
 Timbira: http://www.timbira.com.br
 Blog: http://fabriziomello.github.io
 Linkedin: http://br.linkedin.com/in/fabriziomello
 Twitter: http://twitter.com/fabriziomello
 Github: http://github.com/fabriziomello


Re: [HACKERS] Cube extension kNN support

2015-07-09 Thread Alexander Korotkov
Hi!

On Sat, May 9, 2015 at 6:53 AM, Stas Kelvich stas.kelv...@gmail.com wrote:

 Patch is pretty ready, last issue was about changed extension interface,
 so there should be migration script and version bump.
 Attaching a version with all migration stuff.


I can't see cube--1.0--1.1.sql in the patch. Did forget to include it?

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


Re: [HACKERS] Improving log capture of TAP tests with IPC::Run

2015-07-09 Thread Tom Lane
Heikki Linnakangas hlinn...@iki.fi writes:
 Pushed, thanks.

Shouldn't we consider back-patching these improvements into 9.5 and 9.4?
ISTM the main point is to help debug buildfarm failures, and we won't be
getting much benefit if only one-third of such reports have decent
logging.

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

2015-07-09 Thread Pavel Stehule
2015-07-09 22:57 GMT+02:00 Merlin Moncure mmonc...@gmail.com:

 On Thu, Jul 9, 2015 at 3:31 PM, Pavel Stehule pavel.steh...@gmail.com
 wrote:
 
 
  2015-07-09 20:08 GMT+02:00 Merlin Moncure mmonc...@gmail.com:
 
  On Thu, Jul 9, 2015 at 10:48 AM, Pavel Stehule pavel.steh...@gmail.com
 
  wrote:
  
   2015-07-09 15:17 GMT+02:00 Merlin Moncure mmonc...@gmail.com:
  
   On Wed, Jul 8, 2015 at 11:28 PM, Pavel Stehule
   pavel.steh...@gmail.com
   wrote:
Hi
   
second version of this patch
   
make check-world passed
  
   quickly scanning the patch, the implementation is trivial (minus
   regression test adjustments), and is, IMSNSHO, the right solution.
  
  
   yes, it is right way - the behave of RAISE statement will be much more
   cleaner
  
  
   Several of the source level comments need some minor wordsmithing and
   the GUCs are missing documentation.  If we've got consensus on the
   approach, I'll pitch in on that.
  
   thank you
 
  revised patch attached. added GUC docs and cleaned up pg_settings
  language.  Also tested patch and it works beautifully.
 
  Note, Pavel's patch does adjust default behavior to what we think is
  the right settings.
 
 
  Thank you for documentation.
 
  There is small error - default for client_min_context is error - not
 notice.
  With this level a diff from regress tests is minimal. Default for
  log_min_context should be warning.

 whoop!  thanks.   Also, I was playing a bit with the idea of making
 client_min_context superuser only setting.  The idea being this
 could be used to prevent leakage of stored procedure code in cases
 where the admins don't want it to be exposed.  I figured it was a bad
 idea though; it would frustrate debugging in reasonable cases.


This is not designed for security usage. Probably there can be some rule in
future - the possibility to see or don't see  a error context - OFF, ON.
For this reason, the setting a some min level is not good way.

Regards

Pavel





 merlin



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

2015-07-09 Thread Merlin Moncure
On Thu, Jul 9, 2015 at 3:31 PM, Pavel Stehule pavel.steh...@gmail.com wrote:


 2015-07-09 20:08 GMT+02:00 Merlin Moncure mmonc...@gmail.com:

 On Thu, Jul 9, 2015 at 10:48 AM, Pavel Stehule pavel.steh...@gmail.com
 wrote:
 
  2015-07-09 15:17 GMT+02:00 Merlin Moncure mmonc...@gmail.com:
 
  On Wed, Jul 8, 2015 at 11:28 PM, Pavel Stehule
  pavel.steh...@gmail.com
  wrote:
   Hi
  
   second version of this patch
  
   make check-world passed
 
  quickly scanning the patch, the implementation is trivial (minus
  regression test adjustments), and is, IMSNSHO, the right solution.
 
 
  yes, it is right way - the behave of RAISE statement will be much more
  cleaner
 
 
  Several of the source level comments need some minor wordsmithing and
  the GUCs are missing documentation.  If we've got consensus on the
  approach, I'll pitch in on that.
 
  thank you

 revised patch attached. added GUC docs and cleaned up pg_settings
 language.  Also tested patch and it works beautifully.

 Note, Pavel's patch does adjust default behavior to what we think is
 the right settings.


 Thank you for documentation.

 There is small error - default for client_min_context is error - not notice.
 With this level a diff from regress tests is minimal. Default for
 log_min_context should be warning.

whoop!  thanks.   Also, I was playing a bit with the idea of making
client_min_context superuser only setting.  The idea being this
could be used to prevent leakage of stored procedure code in cases
where the admins don't want it to be exposed.  I figured it was a bad
idea though; it would frustrate debugging in reasonable cases.

merlin


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


Re: [HACKERS] WAL logging problem in 9.4.3?

2015-07-09 Thread Fujii Masao
On Tue, Jul 7, 2015 at 12:49 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Andres Freund and...@anarazel.de writes:
 On 2015-07-06 11:14:40 -0400, Tom Lane wrote:
 The COUNT() correctly says 11 rows, but after crash-and-recover,
 only the row with -1 is there.  This is because the INSERT writes
 out an INSERT+INIT WAL record, which we happily replay, clobbering
 the data added later by COPY.

 ISTM any WAL logged action that touches a relfilenode essentially needs
 to disable further optimization based on the knowledge that the relation
 is new.

 After a bit more thought, I think it's not so much any WAL logged action
 as any unconditionally-replayed action.  INSERT+INIT breaks this
 example because heap_xlog_insert will unconditionally replay the action,
 even if the page is valid and has same or newer LSN.  Similarly, TRUNCATE
 is problematic because we redo it unconditionally (and in that case it's
 hard to see an alternative).

 It'd not be impossible to add more state to the relcache entry for the
 relation. Whether it's likely that we'd find all the places that'd need
 updating that state, I'm not sure.

 Yeah, the sticking point is mainly being sure that the state is correctly
 tracked, both now and after future changes.  We'd need to identify a state
 invariant that we could be pretty confident we'd not break.

 One idea I had was to allow the COPY optimization only if the heap file is
 physically zero-length at the time the COPY starts.

This seems not helpful for the case where TRUNCATE is executed
before COPY. No?

 That would still be
 able to optimize in all the cases we care about making COPY fast for.
 Rather than reverting cab9a0656c36739f, which would re-introduce a
 different performance problem, perhaps we could have COPY create a new
 relfilenode when it does this.  That should be safe if the table was
 previously empty.

So, if COPY is executed multiple times at the same transaction,
only first COPY can be optimized?


After second thought, I'm thinking that we can safely optimize
COPY if no problematic WAL records like INSERT+INIT or TRUNCATE
are generated since current REDO location or the table was created
at the same transaction. That is, if INSERT or TRUNCATE is executed
after the table creation, but if CHECKPOINT happens subsequently,
we don't need to log COPY. The subsequent crash recovery will not
replay such problematic WAL records. So the example cases where
we can optimize COPY are:

BEGIN
CREATE TABLE
COPY
COPY-- subsequent COPY also can be optimized

BEGIN
CREATE TABLE
TRUNCATE
CHECKPOINT
COPY

BEGIN
CREATE TABLE
INSERT
CHECKPOINT
COPY

A crash recovery can start from previous REDO location (i.e., REDO
location of the last checkpoint record). So we might need to check
whether such problematic WAL records are generated since the previous
REDO location instead of current one.

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] WAL logging problem in 9.4.3?

2015-07-09 Thread Tom Lane
Andres Freund and...@anarazel.de writes:
 On 2015-07-06 11:49:54 -0400, Tom Lane wrote:
 Rather than reverting cab9a0656c36739f, which would re-introduce a
 different performance problem, perhaps we could have COPY create a new
 relfilenode when it does this.  That should be safe if the table was
 previously empty.

 I'm not convinced that cab9a0656c36739f needs to survive in that
 form. To me only allowing one COPY to benefit from the wal_level =
 minimal optimization has a significantly higher cost than
 cab9a0656c36739f.

What evidence have you got to base that value judgement on?

cab9a0656c36739f was based on an actual user complaint, so we have good
evidence that there are people out there who care about the cost of
truncating a table many times in one transaction.  On the other hand,
I know of no evidence that anyone's depending on multiple sequential
COPYs, nor intermixed COPY and INSERT, to be fast.  The original argument
for having this COPY optimization at all was to make restoring pg_dump
scripts in a single transaction fast; and that use-case doesn't care
about anything but a single COPY into a virgin table.

I think you're worrying about exactly the wrong case.

 My tentative guess is that the best course is to
 a) Make heap_truncate_one_rel() create a new relfeilnode. That fixes the
truncation replay issue.
 b) Force new pages to be used when using the heap_sync mode in
COPY. That avoids the INIT danger you found. It seems rather
reasonable to avoid using pages that have already been the target of
WAL logging here in general.

And what reason is there to think that this would fix all the problems?
We know of those two, but we've not exactly looked hard for other cases.
Again, the only known field usage for the COPY optimization is the pg_dump
scenario; were that not so, we'd have noticed the problem long since.
So I don't have any faith that this is a well-tested area.

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] obsolete comment in elog.h

2015-07-09 Thread Fujii Masao
On Thu, Jul 9, 2015 at 1:36 PM, Pavel Stehule pavel.steh...@gmail.com wrote:
 Hi

 the comment about NOTICE level in elog.h is obsolete

Committed. Thanks!

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] Improving log capture of TAP tests with IPC::Run

2015-07-09 Thread Michael Paquier
On Thu, Jul 9, 2015 at 10:43 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Heikki Linnakangas hlinn...@iki.fi writes:
 Pushed, thanks.

 Shouldn't we consider back-patching these improvements into 9.5 and 9.4?
 ISTM the main point is to help debug buildfarm failures, and we won't be
 getting much benefit if only one-third of such reports have decent
 logging.

1ea0620 can be directly cherry-picked on REL9_5_STABLE. For
REL9_4_STABLE the attached looks to work fine.

buildfarm will need to be patched btw... Here is the result of a run
including the log improvements:
http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=dangomushidt=2015-07-09%2019%3A53%3A02stg=bin-check
-- 
Michael
From dc557fdbae810f22d7fccd9dbc4da1f5a9a0b029 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas heikki.linnakan...@iki.fi
Date: Thu, 9 Jul 2015 13:19:10 +0300
Subject: [PATCH] Improve logging of TAP tests.

Create a log file for each test run. Stdout and stderr of the test script,
as well as any subprocesses run as part of the test, are redirected to
the log file. This makes it a lot easier to debug test failures. Also print
the test output (ok 12 - ... messages) to the log file, and the command
line of any external programs executed with the system_or_bail and run_log
functions. This makes it a lot easier to debug failing tests.

Modify some of the pg_ctl and other command invocations to not use 'silent'
or 'quiet' options, and don't redirect output to /dev/null, so that you get
all the information in the log instead.

In the passing, construct some command lines in a way that works if $tempdir
contains quote-characters. I haven't systematically gone through all of
them or tested that, so I don't know if this is enough to make that work.

pg_rewind tests had a custom mechanism for creating a similar log file. Use
the new generic facility instead.

Michael Paquier and me.
---
 src/Makefile.global.in |  1 +
 src/bin/pg_basebackup/t/010_pg_basebackup.pl   |  2 +-
 src/bin/pg_controldata/t/001_pg_controldata.pl |  2 +-
 src/bin/pg_ctl/t/001_start_stop.pl |  2 +-
 src/bin/pg_ctl/t/002_status.pl |  4 +-
 src/test/perl/SimpleTee.pm | 27 
 src/test/perl/TestLib.pm   | 96 +-
 7 files changed, 110 insertions(+), 24 deletions(-)
 create mode 100644 src/test/perl/SimpleTee.pm

diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index aec6187..157fc21 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -322,6 +322,7 @@ cd $(srcdir)  TESTDIR='$(CURDIR)' PATH=$(bindir):$$PATH PGPORT='6$(DEF_PGPOR
 endef
 
 define prove_check
+rm -rf $(srcdir)/tmp_check/log
 $(MKDIR_P) tmp_check/log
 $(MAKE) -C $(top_builddir) DESTDIR='$(CURDIR)'/tmp_check/install install '$(CURDIR)'/tmp_check/log/install.log 21
 cd $(srcdir)  TESTDIR='$(CURDIR)' PATH=$(CURDIR)/tmp_check/install$(bindir):$$PATH $(call add_to_path,$(ld_library_path_var),$(CURDIR)/tmp_check/install$(libdir)) top_builddir='$(CURDIR)/$(top_builddir)' PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 24a828b..538ca0a 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -30,7 +30,7 @@ print HBA local replication all trust\n;
 print HBA host replication all 127.0.0.1/32 trust\n;
 print HBA host replication all ::1/128 trust\n;
 close HBA;
-system_or_bail 'pg_ctl', '-s', '-D', $tempdir/pgdata, 'reload';
+system_or_bail 'pg_ctl', '-D', $tempdir/pgdata, 'reload';
 
 command_fails(
 	[ 'pg_basebackup', '-D', $tempdir/backup ],
diff --git a/src/bin/pg_controldata/t/001_pg_controldata.pl b/src/bin/pg_controldata/t/001_pg_controldata.pl
index a4180e7..e36fa2d 100644
--- a/src/bin/pg_controldata/t/001_pg_controldata.pl
+++ b/src/bin/pg_controldata/t/001_pg_controldata.pl
@@ -11,6 +11,6 @@ program_options_handling_ok('pg_controldata');
 command_fails(['pg_controldata'], 'pg_controldata without arguments fails');
 command_fails([ 'pg_controldata', 'nonexistent' ],
 	'pg_controldata with nonexistent directory fails');
-system_or_bail initdb -D '$tempdir'/data -A trust /dev/null;
+system_or_bail 'initdb', '-D', $tempdir/data, '-A', 'trust';
 command_like([ 'pg_controldata', $tempdir/data ],
 	qr/checkpoint/, 'pg_controldata produces output');
diff --git a/src/bin/pg_ctl/t/001_start_stop.pl b/src/bin/pg_ctl/t/001_start_stop.pl
index 0f62c14..dacdd3e 100644
--- a/src/bin/pg_ctl/t/001_start_stop.pl
+++ b/src/bin/pg_ctl/t/001_start_stop.pl
@@ -33,4 +33,4 @@ command_ok([ 'pg_ctl', 'restart', '-D', $tempdir/data, '-w', '-m', 'fast' ],
 command_ok([ 'pg_ctl', 'restart', '-D', $tempdir/data, '-w', '-m', 'fast' ],
 	'pg_ctl restart with server running');
 
-system_or_bail 'pg_ctl', '-s', 'stop', '-D', $tempdir/data, '-m', 'fast';
+system_or_bail 'pg_ctl', 'stop', '-D', $tempdir/data,