Re: [HACKERS] Small comment fix in partition.c

2017-06-30 Thread Masahiko Sawada
On Fri, Jun 30, 2017 at 11:02 PM, Robert Haas  wrote:
> On Wed, Jun 28, 2017 at 5:11 AM, Masahiko Sawada  
> wrote:
>> Attached patch for $subject.
>> A period is missing at the end of sentence.
>
> Seems reasonable.  Committed.
>

Thank you!

-- 
Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


-- 
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] Incorrect mentions to pg_xlog in walmethods.c/h

2017-06-30 Thread Michael Paquier
On Sat, Jul 1, 2017 at 3:41 AM, Peter Eisentraut
 wrote:
> On 6/27/17 01:26, Michael Paquier wrote:
>> I have noticed $subject. A patch is attached. Those comments are not
>> completely wrong either as pg_basebackup can generate pg_xlog as well,
>> still I would recommend to just mention "pg_wal".
>
> committed

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] Race conditions with WAL sender PID lookups

2017-06-30 Thread Alvaro Herrera
Michael Paquier wrote:
> On Thu, Jun 29, 2017 at 7:52 AM, Alvaro Herrera
>  wrote:

> > I think I'm done with the walsender half of this patch; I still need to
> > review the walreceiver part.  I will report back tomorrow 19:00 CLT.
> 
> Thanks!
> 
> > Currently, I'm considering not to backpatch any of this.
> 
> Considering how crazy the conditions to make the information fetched
> by users inconsistent are met, I agree with that.

Pushed.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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


Re: [HACKERS] Apparent walsender bug triggered by logical replication

2017-06-30 Thread Tom Lane
I wrote:
> I've been poking into the src/test/subscription TAP tests, thinking
> that they seem a lot slower than they ought to be.  The first thing
> I came across was this bit in WaitForReplicationWorkerAttach():

> /*
>  * We need timeout because we generally don't get notified via latch
>  * about the worker attach.
>  */
> rc = WaitLatch(MyLatch,
>WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
>1000L, WAIT_EVENT_BGWORKER_STARTUP);

> Tracing showed that the worker is generally done attaching within two
> or three milliseconds of the request, making the 1000ms delay in this
> loop a rather serious killer of startup performance.  I think the right
> way to fix that is to arrange to have a signal sent back from the worker;
> but just to confirm that this *is* a bottleneck, I tried cutting the
> timeout way back.

I found that logicalrep_worker_stop() has pretty much the identical issue:
it needs to wait for a worker to attach or detach, and it isn't going
to get a latch event for that, and it's using 1-second wait quanta for
events that can be expected to occur within milliseconds.

I still think that the "right" fix would involve adding a way to get
a latch signal for attach/detach, but after some investigation it seemed
like a less-than-trivial change.  Since we could do without risky changes
post-beta, I propose that we just reduce these wait parameters to 10ms,
as in the first attached patch.  It may not ever be worth working harder
than that, since these waits only occur when starting/stopping/changing
logical replication.

I found another source of one-second-ish delays, which is that when
activity on the master stops, LogicalRepApplyLoop generally goes to sleep
without having ack'd the last bit of activity it replayed, since that
won't have been flushed by the walwriter yet.  It will send a feedback
message if it times out --- but that takes a whole second.  Maybe longer,
if unrelated wakeups prevent it from seeing WL_TIMEOUT for a few loop
iterations.  In reality, we can expect the last WAL to have been flushed
by the walwriter in, more or less, WalWriterDelay msec.  Therefore, the
second attached patch tweaks the LogicalRepApplyLoop loop to wait at most
WalWriterDelay if it's got unflushed transactions yet to ack to the
sender, and ensures that we will call send_feedback in the loop even if
no new data arrived.

These two changes cut about 15% off the runtime of the subscription tests
for me (compared to where things stand after 1f201a818).  They're probably
less interesting than that for production purposes, but I think it's worth
doing anyway.

regards, tom lane

diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index 961110c..d165d51 100644
*** a/src/backend/replication/logical/launcher.c
--- b/src/backend/replication/logical/launcher.c
*** WaitForReplicationWorkerAttach(LogicalRe
*** 201,211 
  
  		/*
  		 * We need timeout because we generally don't get notified via latch
! 		 * about the worker attach.
  		 */
  		rc = WaitLatch(MyLatch,
  	   WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
! 	   1000L, WAIT_EVENT_BGWORKER_STARTUP);
  
  		/* emergency bailout if postmaster has died */
  		if (rc & WL_POSTMASTER_DEATH)
--- 201,211 
  
  		/*
  		 * We need timeout because we generally don't get notified via latch
! 		 * about the worker attach.  But we don't expect to have to wait long.
  		 */
  		rc = WaitLatch(MyLatch,
  	   WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
! 	   10L, WAIT_EVENT_BGWORKER_STARTUP);
  
  		/* emergency bailout if postmaster has died */
  		if (rc & WL_POSTMASTER_DEATH)
*** retry:
*** 408,415 
  }
  
  /*
!  * Stop the logical replication worker and wait until it detaches from the
!  * slot.
   */
  void
  logicalrep_worker_stop(Oid subid, Oid relid)
--- 408,415 
  }
  
  /*
!  * Stop the logical replication worker for subid/relid, if any, and wait until
!  * it detaches from the slot.
   */
  void
  logicalrep_worker_stop(Oid subid, Oid relid)
*** logicalrep_worker_stop(Oid subid, Oid re
*** 435,442 
  	generation = worker->generation;
  
  	/*
! 	 * If we found worker but it does not have proc set it is starting up,
! 	 * wait for it to finish and then kill it.
  	 */
  	while (worker->in_use && !worker->proc)
  	{
--- 435,442 
  	generation = worker->generation;
  
  	/*
! 	 * If we found a worker but it does not have proc set then it is still
! 	 * starting up; wait for it to finish starting and then kill it.
  	 */
  	while (worker->in_use && !worker->proc)
  	{
*** logicalrep_worker_stop(Oid subid, Oid re
*** 444,453 
  
  		LWLockRelease(LogicalRepWorkerLock);
  
! 		/* Wait for signal. */
  		rc = WaitLatch(MyLatch,
  	   WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
! 	   1000L, 

Re: [HACKERS] UPDATE of partition key

2017-06-30 Thread Thomas Munro
On Fri, Jun 30, 2017 at 12:01 AM, Amit Khandekar  wrote:
> On 29 June 2017 at 07:42, Amit Langote  wrote:
>> Hi Amit,
>>
>> On 2017/06/28 20:43, Amit Khandekar wrote:
>>> In attached patch v12
>>
>> The patch no longer applies and fails to compile after the following
>> commit was made yesterday:
>>
>> commit 501ed02cf6f4f60c3357775eb07578aebc912d3a
>> Author: Andrew Gierth 
>> Date:   Wed Jun 28 18:55:03 2017 +0100
>>
>> Fix transition tables for partition/inheritance.
>
> Thanks for informing Amit.
>
> As Thomas mentioned upthread, the above commit already uses a tuple
> conversion mapping from leaf partition to root partitioned table
> (mt_transition_tupconv_maps), which serves the same purpose as that of
> the mapping used in the update-partition-key patch during update tuple
> routing (mt_resultrel_maps).
>
> We need to try to merge these two into a general-purpose mapping array
> such as mt_leaf_root_maps. I haven't done that in the rebased patch
> (attached), so currently it has both these mapping fields.
>
> For transition tables, this map is per-leaf-partition in case of
> inserts, whereas it is per-subplan result rel for updates. For
> update-tuple routing, the mapping is required to be per-subplan. Now,
> for update-row-movement in presence of transition tables, we would
> require both per-subplan mapping as well as per-leaf-partition
> mapping, which can't be done if we have a single mapping field, unless
> we have some way to identify which of the per-leaf partition mapping
> elements belong to per-subplan rels.
>
> So, it's not immediately possible to merge them.

Would make sense to have a set of functions with names like
GetConvertor{From,To}{Subplan,Leaf}(mtstate, index) which build arrays
m_convertors_{from,to}_by_{subplan,leaf} the first time they need
them?

-- 
Thomas Munro
http://www.enterprisedb.com


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


Re: [HACKERS] UPDATE of partition key

2017-06-30 Thread Robert Haas
On Thu, Jun 29, 2017 at 3:52 PM, Amit Khandekar  wrote:
> So to conclude, I think, we can do this :
>
> Scenario 1 :
> Only one partitioned table : the root; rest all are leaf partitions.
> In this case, it is definitely efficient to just check the root
> partition key, which will be sufficient.
>
> Scenario 2 :
> There are few non-leaf partitioned tables (3-4) :
> Open those tables, and follow 2nd approach above: If we don't find any
> updated partition-keys in any of them, well and good. If we do find,
> failover to approach 3 : For each of the update resultrels, use the
> new rd_partcheckattrs bitmap to know if it uses any of the updated
> columns. This would be faster than pulling up attrs from the quals
> like how it was done in the patch.

I think we should just have the planner figure out a list of which
columns are partitioning columns either for the named relation or some
descendent, and set a flag if that set of columns overlaps the set of
columns updated.  At execution time, update tuple routing is needed if
either that flag is set or if some partition included in the plan has
a BR UPDATE trigger.  Attached is a draft patch implementing that
approach.

This could be made more more accurate.  Suppose table foo is
partitioned by a and some but not all of the partitions partitioned by
b.  If it so happens that, in a query which only updates b, constraint
exclusion eliminates all of the partitions that are subpartitioned by
b, it would be unnecessary to enable update tuple routing (unless BR
UPDATE triggers are present) but this patch will not figure that out.
I don't think that optimization is critical for the first version of
this feature; there will be a limited number of users with
asymmetrical subpartitioning setups, and if one of them has an idea
how to improve this without hurting anything else, they are free to
contribute a patch.  Other optimizations are possible too, but I don't
really see any of them as critical either.

I don't think the approach of building a hash table to figure out
which result rels have already been created is a good one.  That too
feels like something that the planner should be figuring out and the
executor should just be implementing what the planner decided.  I
haven't figured out exactly how that should work yet, but it seems
like it ought to be doable.

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


decide-whether-we-need-update-tuple-routing.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] Fix a typo in aclchk.c

2017-06-30 Thread Peter Eisentraut
On 6/30/17 03:58, Masahiko Sawada wrote:
> Attached patch for $subject.
> 
> s/entires/entries/

fixed

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


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


[HACKERS] Re: pg_ctl wait exit code (was Re: [COMMITTERS] pgsql: Additional tests for subtransactions in recovery)

2017-06-30 Thread Peter Eisentraut
On 5/1/17 12:19, Peter Eisentraut wrote:
> On 4/27/17 08:41, Michael Paquier wrote:
>> +$node_slave->promote;
>> +$node_slave->poll_query_until('postgres',
>> +   "SELECT NOT pg_is_in_recovery()")
>> +  or die "Timed out while waiting for promotion of standby";
>>
>> This reminds me that we should really switch PostgresNode::promote to
>> use the wait mode of pg_ctl promote, and remove all those polling
>> queries...
> 
> I was going to say: This should all be obsolete already, because pg_ctl
> promote waits by default.
> 
> However: Failure to complete promotion within the waiting time does not
> lead to an error exit, so you will not get a failure if the promotion
> does not finish.  This is probably a mistake.  Looking around pg_ctl, I
> found that this was handled seemingly inconsistently in do_start(), but
> do_stop() errors when it does not complete.
> 
> Possible patches for this attached.
> 
> Perhaps we need a separate exit code in pg_ctl to distinguish general
> errors from did not finish within timeout?

I was going to hold this back for PG11, but since we're now doing some
other tweaks in pg_ctl, it might be useful to add this too.  Thoughts?

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 67707d541a2d9e088109385c8fa1eced8af83d54 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Mon, 1 May 2017 12:10:17 -0400
Subject: [PATCH v2 1/2] pg_ctl: Make failure to complete operation a nonzero
 exit

If an operation being waited for does not complete within the timeout,
then exit with a nonzero exit status.  This was previously handled
inconsistently.
---
 doc/src/sgml/ref/pg_ctl-ref.sgml | 7 +++
 src/bin/pg_ctl/pg_ctl.c  | 8 ++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/ref/pg_ctl-ref.sgml b/doc/src/sgml/ref/pg_ctl-ref.sgml
index 71e52c4c35..12fa011c4e 100644
--- a/doc/src/sgml/ref/pg_ctl-ref.sgml
+++ b/doc/src/sgml/ref/pg_ctl-ref.sgml
@@ -412,6 +412,13 @@ Options
 pg_ctl returns an exit code based on the
 success of the startup or shutdown.

+
+   
+If the operation does not complete within the timeout (see
+option -t), then pg_ctl exits with
+a nonzero exit status.  But note that the operation might continue in
+the background and eventually succeed.
+   
   
  
 
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 0c65196bda..4e02c4cea1 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -840,7 +840,9 @@ do_start(void)
break;
case POSTMASTER_STILL_STARTING:
print_msg(_(" stopped waiting\n"));
-   print_msg(_("server is still starting up\n"));
+   write_stderr(_("%s: server did not start in 
time\n"),
+progname);
+   exit(1);
break;
case POSTMASTER_FAILED:
print_msg(_(" stopped waiting\n"));
@@ -1166,7 +1168,9 @@ do_promote(void)
else
{
print_msg(_(" stopped waiting\n"));
-   print_msg(_("server is still promoting\n"));
+   write_stderr(_("%s: server did not promote in time\n"),
+progname);
+   exit(1);
}
}
else
-- 
2.13.1

From b30b7d96161a2e27d80cc96073b44c5266c2b751 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Mon, 1 May 2017 12:11:25 -0400
Subject: [PATCH v2 2/2] Remove unnecessary pg_is_in_recovery calls in tests

Since pg_ctl promote already waits for recovery to end, these calls are
obsolete.
---
 src/test/modules/commit_ts/t/003_standby_2.pl | 1 -
 src/test/recovery/t/008_fsm_truncation.pl | 2 --
 src/test/recovery/t/009_twophase.pl   | 6 --
 src/test/recovery/t/010_logical_decoding_timelines.pl | 3 ---
 src/test/recovery/t/012_subtransactions.pl| 6 --
 5 files changed, 18 deletions(-)

diff --git a/src/test/modules/commit_ts/t/003_standby_2.pl 
b/src/test/modules/commit_ts/t/003_standby_2.pl
index 2fd561115c..c3000f5b4c 100644
--- a/src/test/modules/commit_ts/t/003_standby_2.pl
+++ b/src/test/modules/commit_ts/t/003_standby_2.pl
@@ -55,7 +55,6 @@
 $master->restart;
 
 system_or_bail('pg_ctl', '-D', $standby->data_dir, 'promote');
-$standby->poll_query_until('postgres', "SELECT pg_is_in_recovery() <> true");
 
 $standby->safe_psql('postgres', "create table t11()");
 my $standby_ts = $standby->safe_psql('postgres',
diff --git a/src/test/recovery/t/008_fsm_truncation.pl 
b/src/test/recovery/t/008_fsm_truncation.pl
index 56eecf722c..ddab464a97 

Re: [HACKERS] [PATCH] doc: Fix typo

2017-06-30 Thread Robert Haas
On Fri, Jun 30, 2017 at 2:50 PM, Peter Eisentraut
 wrote:
> On 6/28/17 07:54, Zero King wrote:
>> ---
>>  doc/src/sgml/installation.sgml | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
>> index becf868..809cacb 100644
>> --- a/doc/src/sgml/installation.sgml
>> +++ b/doc/src/sgml/installation.sgml
>> @@ -120,7 +120,7 @@ su - postgres
>>edit previous commands.  This is very helpful and is strongly
>>recommended.  If you don't want to use it then you must specify
>>the --without-readline option to
>> -  configure. As an alternative, you can often use the
>> +  configure. As an alternative, you can also use the
>>BSD-licensed libedit library, originally
>>developed on NetBSD. The
>>libedit library is
>>
>
> I don't think this was a typo.

I agree.

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


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


Re: [HACKERS] CREATE COLLATION definitional questions for ICU

2017-06-30 Thread Tom Lane
Peter Eisentraut  writes:
> On 6/25/17 11:45, Tom Lane wrote:
>> * Now that it's possible for user-created collations to have encoding -1,
>> I do not think that the "shadowing" tests in CollationCreate and
>> IsThereCollationInNamespace are sufficient.  They don't prevent a new
>> collation with encoding -1 from shadowing an existing encoding-specific
>> collation that doesn't happen to match the current DB's encoding.
>> Now, you'd have to work at it for that to cause problems --- say,
>> create such a situation in template0 and then copy template0 specifying
>> that other encoding.  But none of that is forbidden.

> I see.  Do you have an idea how to fix it without too much overhead?  We
> couldn't use the existing syscache for it.

I think you could use SearchSysCacheList to acquire a list of all
collations with the given name, and then run through it looking for
entries matching the target namespace.  This doesn't seem too inefficient,
since the list would typically have length at most one.

regards, tom lane


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


Re: [HACKERS] pg_stop_backup(wait_for_archive := true) on standby server

2017-06-30 Thread Stephen Frost
Peter, all,

* Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote:
> On 6/30/17 04:08, Masahiko Sawada wrote:
> >> I'm not sure. I think this can be considered a bug in the implementation 
> >> for
> >> 10, and as such is "open for fixing". However, it's not a very critical bug
> >> so I doubt it should be a release blocker, but if someone wants to work on 
> >> a
> >> fix I think we should commit it.
> > 
> > I agree with you. I'd like to hear opinions from other hackers as well.
> 
> It's preferable to make it work.  If it's not easily possible, then we
> should prohibit it.
> 
> Comments from Stephen (original committer)?

I agree that it'd be preferable to make it work, but I'm not sure I can
commit to having it done in short order.  I'm happy to work to prohibit
it, but if someone has a few spare cycles to make it actually work,
that'd be great.

In short, I agree with Magnus and feel like I'm more-or-less in the same
boat as he is (though slightly jealous as that's not actually physically
the case, for I hear he has a rather nice boat...).

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Typo in comment in postgres_fdw.c

2017-06-30 Thread Peter Eisentraut
On 6/28/17 09:53, Albe Laurenz wrote:
> Attached is a fix for a small typo I found.

committed

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


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


Re: [HACKERS] [PATCH] doc: Fix typo

2017-06-30 Thread Peter Eisentraut
On 6/28/17 07:54, Zero King wrote:
> ---
>  doc/src/sgml/installation.sgml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
> index becf868..809cacb 100644
> --- a/doc/src/sgml/installation.sgml
> +++ b/doc/src/sgml/installation.sgml
> @@ -120,7 +120,7 @@ su - postgres
>edit previous commands.  This is very helpful and is strongly
>recommended.  If you don't want to use it then you must specify
>the --without-readline option to
> -  configure. As an alternative, you can often use the
> +  configure. As an alternative, you can also use the
>BSD-licensed libedit library, originally
>developed on NetBSD. The
>libedit library is
> 

I don't think this was a typo.

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


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


Re: [HACKERS] Typo in comment in xlog.c: ReadRecord

2017-06-30 Thread Peter Eisentraut
On 6/27/17 20:54, Amit Langote wrote:
> Attached fixes $SUBJECT.
> 
> s/fetch_ckpt/fetching_ckpt/g

committed

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


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


Re: [HACKERS] Misleading comment in slru.h

2017-06-30 Thread Peter Eisentraut
On 6/27/17 01:43, Thomas Munro wrote:
> As mentioned in another thread[1], slru.h says:
> 
>   * Note: slru.c currently assumes that segment file names will be four hex
>   * digits.  This sets a lower bound on the segment size (64K transactions
>   * for 32-bit TransactionIds).
> 
> That comment is out of date: commit 638cf09e extended SLRUs to support
> 5 character names to support pg_multixact and commit 73c986ad extended
> support to 6 character SLRU file names for pg_commit_ts.
> 
> Should we just remove that comment?

done

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


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


Re: [HACKERS] Incorrect mentions to pg_xlog in walmethods.c/h

2017-06-30 Thread Peter Eisentraut
On 6/27/17 01:26, Michael Paquier wrote:
> I have noticed $subject. A patch is attached. Those comments are not
> completely wrong either as pg_basebackup can generate pg_xlog as well,
> still I would recommend to just mention "pg_wal".

committed

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


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


Re: [HACKERS] Out of date comment in predicate.c

2017-06-30 Thread Peter Eisentraut
On 6/27/17 01:21, Thomas Munro wrote:
> Commit ea9df812d8502fff74e7bc37d61bdc7d66d77a7f got rid of
> FirstPredicateLockMgrLock, but it's still referred to in a comment in
> predicate.c where the locking protocol is documented.  I think it's
> probably best to use the name of the macro that's usually used to
> access the lock array in the code.  Please see attached.

Does this apply equally to PredicateLockHashPartitionLock() and
PredicateLockHashPartitionLockByIndex()?  Should the comment mention or
imply both?

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


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


Re: [HACKERS] Get stuck when dropping a subscription during synchronizing table

2017-06-30 Thread Peter Eisentraut
On 6/29/17 23:39, Noah Misch wrote:
> IMMEDIATE ATTENTION REQUIRED.  This PostgreSQL 10 open item is long past due
> for your status update.  Please reacquaint yourself with the policy on open
> item ownership[1] and then reply immediately.  If I do not hear from you by
> 2017-07-01 04:00 UTC, I will transfer this item to release management team
> ownership without further notice.

I will work on this over the weekend and report back on Monday.

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


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


Re: [HACKERS] pg_stop_backup(wait_for_archive := true) on standby server

2017-06-30 Thread Peter Eisentraut
On 6/30/17 04:08, Masahiko Sawada wrote:
>> I'm not sure. I think this can be considered a bug in the implementation for
>> 10, and as such is "open for fixing". However, it's not a very critical bug
>> so I doubt it should be a release blocker, but if someone wants to work on a
>> fix I think we should commit it.
> 
> I agree with you. I'd like to hear opinions from other hackers as well.

It's preferable to make it work.  If it's not easily possible, then we
should prohibit it.

Comments from Stephen (original committer)?

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


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


Re: [HACKERS] "SELECT *" vs hidden columns and logical column order

2017-06-30 Thread Peter Eisentraut
On 6/28/17 23:52, Thomas Munro wrote:
> 2.  SQL:2011 temporal tables track system time and/or valid time with
> columns that users create and then declare to be temporal control
> columns.  I don't think they show up unless you name them directly (I
> didn't check the standard but I noticed that it's that way in another
> product), so I guess that's basically the same as (1).

In my reading of the standard, those start/end time columns would show
up normally in SELECT *.

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


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


Re: [HACKERS] CREATE COLLATION definitional questions for ICU

2017-06-30 Thread Peter Eisentraut
On 6/27/17 11:17, Tom Lane wrote:
> Moreover, if you insist on defining it this way, it's going to limit
> our freedom of action in future.  It's possible that, either in some
> future version of ICU or for some other provider, there could be more
> than one version of a collation simultaneously available.

Good point, but I think this is highly theoretical and would probably
work differently and separately from what we have now.  If someone
invented a feature that allows linking against multiple versions of ICU
at once (thus allowing old versions to be continued to be used), then
you'd still want the safety check of the current style that the version
currently in use is the one previously used.  If ICU offered a way to
use multiple collation versions from the same library version, then I'd
imagine that you would select the version with an attribute in the
locale name, not with a separate field.  And arguably, considering how
the ICU locale name resolution works, having a safety check that records
the actual version would still be of use.

So "the version I want" and "the version I got" would continue to be
separate attributes.  I would not mind a gentle renaming of the current
functionality if it could make that clearer.

> Lastly, I'm not seeing the use-case for having CREATE COLLATION magically
> make a working collation from a broken one.  Wouldn't you normally
> expect to need to do ALTER COLLATION REFRESH on an obsolete collation
> before doing anything with it?

My question would be, what's the use case for doing it the other way around?

Thinking of an analogy, if there is a table with an index that is
somehow affected by pg_upgrade and needs reindexing after pg_upgrade,
and I run CREATE TABLE LIKE to make a new table like that old table,
would it create the new table in a state that it would also require
reindexing before it can be used?  That doesn't seem very useful.

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


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


Re: [HACKERS] CREATE COLLATION definitional questions for ICU

2017-06-30 Thread Peter Eisentraut
On 6/25/17 11:45, Tom Lane wrote:
> * Now that it's possible for user-created collations to have encoding -1,
> I do not think that the "shadowing" tests in CollationCreate and
> IsThereCollationInNamespace are sufficient.  They don't prevent a new
> collation with encoding -1 from shadowing an existing encoding-specific
> collation that doesn't happen to match the current DB's encoding.
> Now, you'd have to work at it for that to cause problems --- say,
> create such a situation in template0 and then copy template0 specifying
> that other encoding.  But none of that is forbidden.

I see.  Do you have an idea how to fix it without too much overhead?  We
couldn't use the existing syscache for it.

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


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


Re: [HACKERS] CREATE COLLATION definitional questions for ICU

2017-06-30 Thread Peter Eisentraut
On 6/25/17 11:45, Tom Lane wrote:
> * For an ICU collation, should we not insist that collcollate and
> collctype be equal?  If not, what does it mean for them to be different?

I have fixed that for now by enforcing them to be the same.

In the longer term, I'm thinking about converting these two columns into
a more flexible array of properties.

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


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


Re: [HACKERS] Fix doc of DROP SUBSCRIPTION

2017-06-30 Thread Petr Jelinek
On 30/06/17 15:17, Yugo Nagata wrote:
> On Fri, 30 Jun 2017 20:17:39 +0900
> Masahiko Sawada  wrote:
> 
>> On Fri, Jun 30, 2017 at 7:01 PM, Yugo Nagata  wrote:
>>> Hi,
>>>
>>> The documentation says that a subscription that has a replication slot
>>> cannot be dropped  in a transaction block, but it is not allowed even
>>> outside of a transaction block.
>>
>> Hmm, I think we can drop a subscription outside of a transaction block
>> even if the subscription associates with a replication.
> 
> Sorry, I was wrong and missing something... I confirmaed it.
> The documentation is right. Sorry for the noise.
> 

It will only fail if the remote site is not reachable during the drop
(but then it should hint about the ALTER), maybe that's what happened to
you?

-- 
  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] Small comment fix in partition.c

2017-06-30 Thread Robert Haas
On Wed, Jun 28, 2017 at 5:11 AM, Masahiko Sawada  wrote:
> Attached patch for $subject.
> A period is missing at the end of sentence.

Seems reasonable.  Committed.

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


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


Re: [HACKERS] "SELECT *" vs hidden columns and logical column order

2017-06-30 Thread Robert Haas
On Wed, Jun 28, 2017 at 11:52 PM, Thomas Munro
 wrote:
> I am aware of at three potential projects that would change the
> meaning of "SELECT *":
>
> 1.  Incremental MATERIALIZED VIEW maintenance probably needs to be
> able to use a hidden counter column which you can ask for by name but
> will otherwise not show up to users:
>
> https://www.postgresql.org/message-id/1371480075.55528.yahoomail...@web162901.mail.bf1.yahoo.com
>
> 2.  SQL:2011 temporal tables track system time and/or valid time with
> columns that users create and then declare to be temporal control
> columns.  I don't think they show up unless you name them directly (I
> didn't check the standard but I noticed that it's that way in another
> product), so I guess that's basically the same as (1).
>
> 3.  Logical column order aka ALTER COLUMN POSITION, a recurring topic
> on pgsql-hackers for which patches have been written but nothing has
> so far managed to stick:
>
> https://www.postgresql.org/message-id/flat/20141209174146.GP1768%40alvh.no-ip.org
>
> Suppose someone wanted to chip away at a small piece of incremental
> matviews by inventing a way to declare 'hidden' columns: is there
> really a dependency here as implied in the 2013 email above?  Is
> anyone planning to revive logical column order?

If someone were planning to revive logical column order, adding say a
column attdisplaypos which controlled how an asterisk in the target
list is expanded, then it would seem to make sense to use
attdisplaypos = 0, say, to mean "don't *-expand this column at all".

Of course, somebody could also propose to add attisdisplayed as a
Boolean column and refactor all the code that currently uses attnum
for that purpose to rely on attisdisplayed instead.  That code would
then presumably get refactored again if attdisplaypos ever showed up,
but probably the second refactoring would be pretty easy.  The really
hard part here is probably finding all of the places that are relying
on attnum < 0 as a proxy for whether the column should be displayed.

BTW, if we get either of these things, can I vote for, as a follow-on
patch, changing OID columns to be displayed by default, at least in
system catalogs?  I don't think that having the primary keys of our
system catalogs as a hidden column is particularly user-friendly.

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


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


Re: [HACKERS] Parallel COPY FROM execution

2017-06-30 Thread Pavel Stehule
2017-06-30 15:45 GMT+02:00 Pavel Stehule :

>
>
> 2017-06-30 15:42 GMT+02:00 Alex K :
>
>> On Fri, Jun 30, 2017 at 3:35 PM, Pavel Stehule 
>> wrote:
>> >
>> >
>> > 2017-06-30 14:23 GMT+02:00 Alex K :
>> >>
>> >> Thus, it results in a ~60% performance boost per each x2
>> multiplication of
>> >> parallel processes, which is consistent with the initial estimation.
>> >>
>> >
>> > the important use case is big table with lot of indexes. Did you test
>> > similar case?
>>
>> Not yet, I will try it, thank you for a suggestion. But how much is it
>> 'big table' and 'lot of indexes' in numbers approximately?
>>
>
> the size is about 1/3 RAM size, 60 columns, 30 indexes
>

maybe some variants can be interesting .. 1/30 RAM, 1/20 RAM, 1/10 RAM, 1/3
RAM

and a) when bottleneck is IO, b) when bottleneck is CPU

Pavel


>
> Regards
>
> Pavel
>
>
>>
>> Also, index updates and constraint checks performance are what I cannot
>> control during COPY execution, so probably I have not to care too much
>> about that. But of course, it is interesting, how does COPY perform in
>> that case.
>>
>>
>> Alexey
>>
>
>


Re: [HACKERS] Parallel COPY FROM execution

2017-06-30 Thread Pavel Stehule
2017-06-30 15:42 GMT+02:00 Alex K :

> On Fri, Jun 30, 2017 at 3:35 PM, Pavel Stehule 
> wrote:
> >
> >
> > 2017-06-30 14:23 GMT+02:00 Alex K :
> >>
> >> Thus, it results in a ~60% performance boost per each x2 multiplication
> of
> >> parallel processes, which is consistent with the initial estimation.
> >>
> >
> > the important use case is big table with lot of indexes. Did you test
> > similar case?
>
> Not yet, I will try it, thank you for a suggestion. But how much is it
> 'big table' and 'lot of indexes' in numbers approximately?
>

the size is about 1/3 RAM size, 60 columns, 30 indexes

Regards

Pavel


>
> Also, index updates and constraint checks performance are what I cannot
> control during COPY execution, so probably I have not to care too much
> about that. But of course, it is interesting, how does COPY perform in
> that case.
>
>
> Alexey
>


Re: [HACKERS] Parallel COPY FROM execution

2017-06-30 Thread Alex K
On Fri, Jun 30, 2017 at 3:35 PM, Pavel Stehule  wrote:
>
>
> 2017-06-30 14:23 GMT+02:00 Alex K :
>>
>> Thus, it results in a ~60% performance boost per each x2 multiplication of
>> parallel processes, which is consistent with the initial estimation.
>>
>
> the important use case is big table with lot of indexes. Did you test
> similar case?

Not yet, I will try it, thank you for a suggestion. But how much is it
'big table' and 'lot of indexes' in numbers approximately?

Also, index updates and constraint checks performance are what I cannot
control during COPY execution, so probably I have not to care too much
about that. But of course, it is interesting, how does COPY perform in
that case.


Alexey


-- 
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] Request more documentation for incompatibility of parallelism and plpgsql exec_run_select

2017-06-30 Thread Mark Dilger

> On Jun 29, 2017, at 8:55 PM, Mark Dilger  wrote:
> 
> 
> Changing myfunc to create a temporary table, to execute the sql to populate
> that temporary table, and to then loop through the temporary table's rows
> fixes the problem.  For the real-world example where I hit this, that single
> change decreases the runtime from 13.5 seconds to 2.5 seconds.

Actually, this is wrong.  On further review, by the time I had changed the
function definition, the data in the test server I was querying had likely 
changed
enough for the performance to change.  That duped me into thinking I had
found a work-around.  I can no longer reproduce any performance improvement
in this manner.

mark

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


Re: [HACKERS] Fix doc of DROP SUBSCRIPTION

2017-06-30 Thread Yugo Nagata
On Fri, 30 Jun 2017 20:17:39 +0900
Masahiko Sawada  wrote:

> On Fri, Jun 30, 2017 at 7:01 PM, Yugo Nagata  wrote:
> > Hi,
> >
> > The documentation says that a subscription that has a replication slot
> > cannot be dropped  in a transaction block, but it is not allowed even
> > outside of a transaction block.
> 
> Hmm, I think we can drop a subscription outside of a transaction block
> even if the subscription associates with a replication.

Sorry, I was wrong and missing something... I confirmaed it.
The documentation is right. Sorry for the noise.

> 
> =# table pg_subscription;
>  subdbid | subname  | subowner | subenabled |subconninfo
>  | subslotname | subsynccommit | subpublications
> -+--+--++---+-+---+-
>13164 | hoge_sub |   10 | t  | dbname=postgres
> port=5550 | hoge_sub| off   | {one_pub}
> (1 row)
> 
> =# drop subscription hoge_sub ;
> DROP SUBSCRIPTION
> 
> Regards,
> 
> --
> Masahiko Sawada
> NIPPON TELEGRAPH AND TELEPHONE CORPORATION
> NTT Open Source Software Center


-- 
Yugo Nagata 


-- 
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] Broken hint bits (freeze)

2017-06-30 Thread Amit Kapila
On Fri, Jun 30, 2017 at 6:26 AM, Bruce Momjian  wrote:
> On Wed, Jun 28, 2017 at 10:11:35PM -0400, Bruce Momjian wrote:
>> On Fri, Jun 23, 2017 at 06:17:47PM +0300, Sergey Burladyan wrote:
>> > PS:
>> > I successfully upgraded last night from 9.2 to 9.4 and find other issue :-)
>> >
>> > It is about hash index and promote:
>> > 1. create master
>> > 2. create standby from it
>> > 3. create unlogged table and hash index like:
>> >  create unlogged table test (id int primary key, v text);
>> >  create index on test using hash (id);
>> > 3. stop master
>> > 4. promote standby
>> >
>> > now, if you try to upgrade this new promoted master pg_upgrade will stop
>> > on this hash index:
>> > error while creating link for relation "public.test_id_idx" 
>> > ("s/9.2/base/16384/16393" to "m/9.4/base/16422/16393"): No such file or 
>> > directory
>> > Failure, exiting
>> >
>> > I touch this file (s/9.2/base/16384/16393) and rerun pg_upgrade from
>> > scratch and it complete successfully.
>>
>> Sergey, can you please test if the table "test" is not unlogged, does
>> pg_upgrade still fail on the hash index file?
>
> I was able to reproduce this failure on my server.  :-)
>
> What I found is that the problem is larger than I thought.  Sergey is
> correct that pg_upgrade fails because there is no hash file associated
> with the unlogged table, but in fact a simple access of the unlogged
> table with a hash index generates an error:
>
> test=> SELECT * FROM t_u_hash;
> ERROR:  could not open file "base/16384/16392": No such file or 
> directory
>
> What is interesting is that this is the only combination that generates
> an error.
>

Yes and that is because normally we log the creation of init fork for
unlogged relations (both heap and index, refer btbuildempty for index
and
heap_create_init_fork for heap), but for hash indexes prior to 10, we
don't log for init forks.

>  A unlogged able with a btree index or a logged table with a
> hash index are fine, e.g.:
>
>List of relations
>  Schema |   Name| Type  |  Owner
> +---+---+--
>  public | t_btree   | table | postgres
>  public | t_hash| table | postgres
>  public | t_u_btree | table | postgres
> fail-->  public | t_u_hash  | table | postgres
>
> This doesn't fail on PG 10 since we WAL-log hash indexes.
>
> I think we have two questions:
>
> 1.  do we fix this in the server

If we want to fix this in the server then we need to log (write WAL)
the init fork for hash indexes.

> 2.  if not, do we fix pg_upgrade
>

I think even if we provide a fix in pg_upgrade, it might not suffice
the need because this problem can come if the user just promotes
standby server (<=9.6) to master considering we had unlogged table and
hash index on that table.

I think we should fix the server.


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


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


Re: [HACKERS] CREATE COLLATION definitional questions for ICU

2017-06-30 Thread Peter Eisentraut
On 6/25/17 11:45, Tom Lane wrote:
> * Also (and this would be a pre-existing bug), why doesn't the FROM
> path copy the old collation's encoding?  For example, if you attempted
> to clone the "C" encoding, you wouldn't get a true clone but something
> that's specific to the current DB's encoding.

Fixed that.  This has probably no practical impact, so I don't plan to
backpatch it.

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


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


Re: [HACKERS] Parallel COPY FROM execution

2017-06-30 Thread Pavel Stehule
2017-06-30 14:23 GMT+02:00 Alex K :

> Greetings pgsql-hackers,
>
> I am a GSOC student this year, my initial proposal has been discussed
> in the following thread
> https://www.postgresql.org/message-id/flat/7179F2FD-49CE-
> 4093-AE14-1B26C5DFB0DA%40gmail.com
>
> Patch with COPY FROM errors handling seems to be quite finished, so
> I have started thinking about parallelism in COPY FROM, which is the next
> point in my proposal.
>
> In order to understand are there any expensive calls in COPY, which
> can be executed in parallel, I did a small research. First, please, find
> flame graph of the most expensive copy.c calls during the 'COPY FROM file'
> attached (copy_from.svg). It reveals, that inevitably serial operations
> like
> CopyReadLine (<15%), heap_multi_insert (~15%) take less than 50% of
> time in summary, while remaining operations like heap_form_tuple and
> multiple checks inside NextCopyFrom probably can be executed well in
> parallel.
>
> Second, I have compared an execution time of 'COPY FROM a single large
> file (~300 MB, 5000 lines)' vs. 'COPY FROM four equal parts of the
> original file executed in the four parallel processes'. Though it is a
> very rough test, it helps to obtain an overall estimation:
>
> Serial:
> real 0m56.571s
> user 0m0.005s
> sys 0m0.006s
>
> Parallel (x4):
> real 0m22.542s
> user 0m0.015s
> sys 0m0.018s
>
> Thus, it results in a ~60% performance boost per each x2 multiplication of
> parallel processes, which is consistent with the initial estimation.
>
>
the important use case is big table with lot of indexes. Did you test
similar case?

Regards

Pavel


[HACKERS] Parallel COPY FROM execution

2017-06-30 Thread Alex K
Greetings pgsql-hackers,

I am a GSOC student this year, my initial proposal has been discussed
in the following thread
https://www.postgresql.org/message-id/flat/7179F2FD-49CE-4093-AE14-1B26C5DFB0DA%40gmail.com

Patch with COPY FROM errors handling seems to be quite finished, so
I have started thinking about parallelism in COPY FROM, which is the next
point in my proposal.

In order to understand are there any expensive calls in COPY, which
can be executed in parallel, I did a small research. First, please, find
flame graph of the most expensive copy.c calls during the 'COPY FROM file'
attached (copy_from.svg). It reveals, that inevitably serial operations like
CopyReadLine (<15%), heap_multi_insert (~15%) take less than 50% of
time in summary, while remaining operations like heap_form_tuple and
multiple checks inside NextCopyFrom probably can be executed well in parallel.

Second, I have compared an execution time of 'COPY FROM a single large
file (~300 MB, 5000 lines)' vs. 'COPY FROM four equal parts of the
original file executed in the four parallel processes'. Though it is a
very rough test, it helps to obtain an overall estimation:

Serial:
real 0m56.571s
user 0m0.005s
sys 0m0.006s

Parallel (x4):
real 0m22.542s
user 0m0.015s
sys 0m0.018s

Thus, it results in a ~60% performance boost per each x2 multiplication of
parallel processes, which is consistent with the initial estimation.

After several discussions I have two possible solutions on my mind:

1) Simple solution

Let us focus only on the 'COPY FROM file', then it is relatively easy to
implement, just give the same file and offset to each worker.

++ Simple; more reliable solution; probably it will give us the most possible
 performance boost

- - Limited number of use cases. Though 'COPY FROM file' is a frequent case,
even when one use it with psql \copy, client-side file read and stdin
streaming to the backend are actually performed

2) True parallelism

Implement a pool of bg_workers and simple shared_buffer/query. While main
COPY process will read an input data and put raw lines into the query, parallel
bg_workers will take lines from there and process.

++ More general solution; support of various COPY FROM use-cases

- - Much more sophisticated solution; probably less performance boost
compared to 1)

I will be glad to any comments and criticism.


Alexey

-- 
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] Adding support for Default partition in partitioning

2017-06-30 Thread Jeevan Ladhe
Hi,

On Mon, Jun 19, 2017 at 12:34 PM, Amit Langote <
langote_amit...@lab.ntt.co.jp> wrote:

> On 2017/06/16 14:16, Ashutosh Bapat wrote:
> > On Fri, Jun 16, 2017 at 12:48 AM, Robert Haas 
> wrote:
> >> On Thu, Jun 15, 2017 at 12:54 PM, Ashutosh Bapat
> >>  wrote:
> >>> Some more comments on the latest set of patches.
> >> or looking up the OID in the
> >> relcache multiple times.
> >
> > I am not able to understand this in the context of default partition.
> > After that nobody else is going to change its partitions and their
> > bounds (since both of those require heap_open on parent which would be
> > stuck on the lock we hold.). So, we have to check only once if the
> > table has a default partition. If it doesn't, it's not going to
> > acquire one unless we release the lock on the parent i.e at the end of
> > transaction. If it has one, it's not going to get dropped till the end
> > of the transaction for the same reason. I don't see where we are
> > looking up OIDs multiple times.
>
> Without heap_opening the parent, the only way is to look up parentOid's
> children in pg_inherits and for each child looking up its pg_class tuple
> in the syscache to see if its relpartbound indicates that it's a default
> partition.  That seems like it won't be inexpensive either.
>
> It would be nice if could get that information (that is - is a given
> relation being heap_drop_with_catalog'd a partition of the parent that
> happens to have default partition) in less number of steps than that.
> Having that information in relcache is one way, but as mentioned, that
> turns out be expensive.
>
> Has anyone considered the idea of putting the default partition OID in the
> pg_partitioned_table catalog?  Looking the above information up would
> amount to one syscache lookup.  Default partition seems to be special
> enough object to receive a place in the pg_partitioned_table tuple of the
> parent.  Thoughts?
>

I liked this suggestion. Having an entry in pg_partitioned_table would avoid
both expensive methods, i.e. 1. opening the parent or 2. lookup for
each of the children first in pg_inherits and then its corresponding entry
in
pg_class.
Unless anybody has any other suggestions/comments here, I am going to
implement this suggestion.

Thanks,
Jeevan Ladhe


Re: [HACKERS] Code quality issues in ICU patch

2017-06-30 Thread Peter Eisentraut
On 6/24/17 11:51, Tom Lane wrote:
> Ah, I was about to suggest the same thing, but I was coming at it from
> the standpoint of not requiring buffers several times larger than
> necessary, which could in itself cause avoidable palloc failures.
> 
> I was going to suggest a small variant actually: run the conversion
> function an extra time only if the string is long enough to make the
> space consumption interesting, say

I had thought about something like that, too, but my concern is that we
then have double the code paths to test.  I have run some performance
tests and I couldn't detect any differences between the variants.  So
unless someone has any other insights, I think I'll go with the proposed
patch by tomorrow.

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


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


Re: [HACKERS] Postgres process invoking exit resulting in sh-QUIT core

2017-06-30 Thread Craig Ringer
On 30 June 2017 at 17:41, K S, Sandhya (Nokia - IN/Bangalore)
 wrote:

> When we checked the process listing during the time of core generation, we
> found Postgres startup process is invoking “sh -c exit 1”:
> 4518  9249  0.1  0.0 155964  2036 ?Ss   15:05   0:00 postgres:
> startup process   waiting for 000102EB

Looks like an archive_command or restore_command .

If 'sh' is dumping core, you probably have issues at a low level in
the kernel, file system, etc. Check dmesg.

-- 
 Craig Ringer   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] Fix doc of DROP SUBSCRIPTION

2017-06-30 Thread Masahiko Sawada
On Fri, Jun 30, 2017 at 7:01 PM, Yugo Nagata  wrote:
> Hi,
>
> The documentation says that a subscription that has a replication slot
> cannot be dropped  in a transaction block, but it is not allowed even
> outside of a transaction block.

Hmm, I think we can drop a subscription outside of a transaction block
even if the subscription associates with a replication.

=# table pg_subscription;
 subdbid | subname  | subowner | subenabled |subconninfo
 | subslotname | subsynccommit | subpublications
-+--+--++---+-+---+-
   13164 | hoge_sub |   10 | t  | dbname=postgres
port=5550 | hoge_sub| off   | {one_pub}
(1 row)

=# drop subscription hoge_sub ;
DROP SUBSCRIPTION

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


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


[HACKERS] Postgres process invoking exit resulting in sh-QUIT core

2017-06-30 Thread K S, Sandhya (Nokia - IN/Bangalore)
Hi,

We are using Postgres version 9.3.14 over linux based OS and we are observing 
sh-QUIT core files randomly when we are restarting the system(occurrence seen 
once in 30 times).
Backtrace is showing as below:

Loaded symbols for /lib64/ld.so.1
Core was generated by `sh -c exit 1'.
Program terminated with signal 3, Quit.
#0  0x005559ed78f0 in do_lookup_x () from /lib64/ld.so.1
(gdb) bt
#0  0x005559ed78f0 in do_lookup_x () from /lib64/ld.so.1
#1  0x005559ed7b88 in _dl_lookup_symbol_x () from /lib64/ld.so.1
#2  0x005559ed916c in _dl_relocate_object () from /lib64/ld.so.1
#3  0x005559ed0b6c in dl_main () from /lib64/ld.so.1
#4  0x005559ee5214 in _dl_sysdep_start () from /lib64/ld.so.1
#5  0x005559ece1b0 in _dl_start_final () from /lib64/ld.so.1
#6  0x005559ece3f0 in _dl_start () from /lib64/ld.so.1
#7  0x005559ecdc10 in __start () from /lib64/ld.so.1
Backtrace stopped: frame did not save the PC

When we checked the process listing during the time of core generation, we 
found Postgres startup process is invoking "sh -c exit 1":
4518  9249  0.1  0.0 155964  2036 ?Ss   15:05   0:00 postgres: 
startup process   waiting for 000102EB
4518 10288  0.0  0.0   3600   508 ?S15:11   0:00  \_ sh -c exit 
1

We tried disabling DB and running the same testcase which didn't result in core 
being generated.
Also we are using immediate shutdown mode which uses SIGQUIT.

Can you please help us in debugging the issue ?

Regards,
Sandhya





[HACKERS] Fix doc of DROP SUBSCRIPTION

2017-06-30 Thread Yugo Nagata
Hi,

The documentation says that a subscription that has a replication slot
cannot be dropped  in a transaction block, but it is not allowed even
outside of a transaction block. Attached is a patch to fix it.

Regards,

-- 
Yugo Nagata 
diff --git a/doc/src/sgml/ref/drop_subscription.sgml b/doc/src/sgml/ref/drop_subscription.sgml
index f535c00..bb48578 100644
--- a/doc/src/sgml/ref/drop_subscription.sgml
+++ b/doc/src/sgml/ref/drop_subscription.sgml
@@ -38,10 +38,9 @@ DROP SUBSCRIPTION [ IF EXISTS ] name
 
   
-   DROP SUBSCRIPTION cannot be executed inside a
-   transaction block if the subscription is associated with a replication
-   slot.  (You can use ALTER SUBSCRIPTION to unset the
-   slot.)
+   DROP SUBSCRIPTION cannot be executed if the subscription
+   is associated with a replication slot.  (You can use ALTER
+   SUBSCRIPTION to unset the slot.)
   
  
 

-- 
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] Bug in ExecModifyTable function and trigger issues for foreign tables

2017-06-30 Thread Etsuro Fujita

On 2017/06/16 21:29, Etsuro Fujita wrote:

On 2017/06/16 19:26, Ashutosh Bapat wrote:



That issue has not been addressed. The reason stated was that it would
make code complicated. But I have not had chance to look at how
complicated would be and assess myself whether that's worth the
trouble.
I have to admit that what I proposed upthread is a quick-and-dirty 
kluge.  One thing I thought to address your concern was to move 
rewriteTargetListUD entirely from the rewriter to the planner when doing 
inherited UPDATE/DELETE, but I'm not sure that's a good idea, because at 
least I think that would need a lot more changes to the rewriter.


I'll have second thought about this, so I'll mark this as waiting on author.

Best regards,
Etsuro Fujita



--
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] Apparent walsender bug triggered by logical replication

2017-06-30 Thread Petr Jelinek
On 30/06/17 04:46, Tom Lane wrote:
> Petr Jelinek  writes:
>> On 30/06/17 02:07, Tom Lane wrote:
>>> I'm also kind of wondering why the "behind the apply" path out of
>>> LogicalRepSyncTableStart exists at all; as far as I can tell we'd be much
>>> better off if we just let the sync worker exit always as soon as it's done
>>> the initial sync, letting any extra catchup happen later.  The main thing
>>> the current behavior seems to be accomplishing is to monopolize one of the
>>> scarce max_sync_workers_per_subscription slots for the benefit of a single
>>> table, for longer than necessary.  Plus it adds additional complicated
>>> interprocess signaling.
> 
>> Hmm, I don't understand what you mean here. The "letting any extra
>> catchup happen later" would never happen if the sync is behind apply as
>> apply has already skipped relevant transactions.
> 
> Once the sync worker has exited, we have to have some other way of dealing
> with that.  I'm wondering why we can't let that other way take over

We make apply wait for the sync worker to get to expected position if it
was behind and only then continue, we can't exactly do that if the apply
already skipped some changes.

> immediately.  The existing approach is inefficient, according to the
> traces I've been poring over all day, and frankly I am very far from
> convinced that it's bug-free either.
-- 
  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] Parallel Append implementation

2017-06-30 Thread Rafia Sabih
On Tue, Apr 4, 2017 at 12:37 PM, Amit Khandekar 
wrote:

> Attached is an updated patch v13 that has some comments changed as per
> your review, and also rebased on latest master.
>

This is not applicable on the latest head i.e. commit
-- 08aed6604de2e6a9f4d499818d7c641cbf5eb9f7, looks like need a rebasing.

-- 
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/


Re: [HACKERS] [PATCH] Generic type subscripting

2017-06-30 Thread Arthur Zakirov
On Wednesday, 10 May 2017 23:43:10 MSK, Dmitry Dolgov wrote:
> So, a few words about current state of the patch:
> 
> * after a lot of serious improvements general design of this feature is
> agreeable
> 
> * we introduced a lot of small changes to polish it
> 
> * I rebased the patch on the latest version of master, so you can take a
> look at it again
> 
> As always, any feedback is welcome.

Hello,

Can you rebase the patch please? It is not applyed now. I think it is because 
of pgindent.

> +
> + scratch->d.sbsref.eval_finfo = eval_finfo;
> + scratch->d.sbsref.nested_finfo = nested_finfo;
> +

Also I have noticed that assigning eval_finfo and nested_finfo after every time 
eval step is pushed is unnecessary in ExecInitSubscriptingRef() function. We 
need them only for EEOP_SBSREF_OLD, EEOP_SBSREF_ASSIGN and EEOP_SBSREF_FETCH 
steps.

-- 
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


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


Re: [HACKERS] Partition-wise join for join between (declaratively) partitioned tables

2017-06-30 Thread Rafia Sabih
On Mon, May 22, 2017 at 12:02 PM, Ashutosh Bapat <
ashutosh.ba...@enterprisedb.com> wrote:

>
> Here's set of patches rebased on latest head.
>

In an attempt to test this set of patches, I found that not all of the
patches could be applied on latest head-- commit
08aed6604de2e6a9f4d499818d7c641cbf5eb9f7
Might be in need of rebasing.

-- 
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/


Re: [HACKERS] Multi column range partition table

2017-06-30 Thread Ashutosh Bapat
On Fri, Jun 30, 2017 at 1:36 PM, Amit Langote
 wrote:
>
> Alright, I spent some time implementing a patch to allow specifying
> -infinity and +infinity in arbitrary ways.  Of course, it prevents
> nonsensical inputs with appropriate error messages.

I don't think -infinity and +infinity are the right terms. For a
string or character data type there is no -infinity and +infinity.
Similarly for enums. We need to extend UNBOUNDED somehow to indicate
the end of a given type in the given direction. I thought about
UNBOUNDED LEFT/RIGHT but then whether LEFT indicates -ve side or +side
would cause confusion. Also LEFT/RIGHT may work for a single
dimensional datatype but not for multi-dimensional spaces. How about
MINIMUM/MAXIMUM or UNBOUNDED MIN/MAX to indicate the extremities.

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


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


Re: [HACKERS] pg_stop_backup(wait_for_archive := true) on standby server

2017-06-30 Thread Masahiko Sawada
On Thu, Jun 29, 2017 at 10:30 PM, Magnus Hagander  wrote:
> On Thu, Jun 22, 2017 at 6:22 PM, Masahiko Sawada 
> wrote:
>>
>> On Thu, Jun 22, 2017 at 10:36 PM, Magnus Hagander 
>> wrote:
>> >
>> >
>> > On Thu, Jun 22, 2017 at 10:12 AM, Masahiko Sawada
>> > 
>> > wrote:
>> >>
>> >> Hi,
>> >>
>> >> Since an optional second argument wait_for_archive of pg_stop_backup
>> >> has been  introduced in PostgreSQL 10 we can choose whether wait for
>> >> archiving. But my colleagues found that we can do pg_stop_backup with
>> >> wait_for_archive = true on the standby server but it actually doesn't
>> >> wait for WAL archiving. Because this behavior is not documented and we
>> >> cannot find out it without reading source code it will confuse the
>> >> user.
>> >>
>> >> I think we can raise an error when pg_stop_backup with
>> >> wait_for_archive = true is executed on the standby. Attached patch
>> >> change it so that.
>> >
>> >
>> > Wouldn't it be better to make it *work*? If you have
>> > archive_mode=always, it
>> > makes sense to want to wait on the standby as well, does it not?
>> >
>>
>> Yes, ideally it will be better to make it wait for WAL archiving on
>> standby server when archive_mode=always. But I think it would be for
>> PG11 item, and this item is for PG10.
>>
>
> I'm not sure. I think this can be considered a bug in the implementation for
> 10, and as such is "open for fixing". However, it's not a very critical bug
> so I doubt it should be a release blocker, but if someone wants to work on a
> fix I think we should commit it.
>

I agree with you. I'd like to hear opinions from other hackers as well.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


-- 
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 range partition table

2017-06-30 Thread Amit Langote
On 2017/06/23 17:00, Dean Rasheed wrote:
> On 23 June 2017 at 08:01, Ashutosh Bapat
>  wrote:
>> The way we have designed our syntax, we don't have a way to tell that
>> p3 comes after p2 and they have no gap between those. But I don't
>> think that's your question. What you are struggling with is a way to
>> specify a lower bound (10, +infinity) so that anything with i1 > 10
>> would go to partition 3.
> 
> I think actually there is a fundamental problem here, which arises
> because UNBOUNDED has 2 different meanings depending on context, and
> thus it is not possible in general to specify the start of one range
> to be equal to the end of the previous range, as is necessary to get
> contiguous non-overlapping ranges.

Okay, I thought about this a bit more and I think I realize that this
arbitrary-sounding restriction of allowing only -infinity in FROM and
+infinity in TO limits the usefulness of the feature to specify infinite
bounds at all.

> Note that this isn't just a problem for floating point datatypes
> either, it also applies to other types such as strings. For example,
> given a partition over (text, int) types defined with the following
> values:
> 
>   FROM ('a', UNBOUNDED) TO ('b', UNBOUNDED)
> 
> which is equivalent to
> 
>   FROM ('a', -INFINITY) TO ('b', +INFINITY)
> 
> where should the next range start?
> 
> Even if you were to find a way to specify "the next string after 'b'",
> it wouldn't exactly be pretty. The problem is that the above partition
> corresponds to "all the strings starting with 'a', plus the string
> 'b', which is pretty ugly. A neater way to define the pair of ranges
> in this case would be:
> 
>   FROM ('a', -INFINITY) TO ('b', -INFINITY)
>   FROM ('b', -INFINITY) TO ('c', -INFINITY)
> 
> since then all strings starting with 'a' would fall into the first
> partition and all the strings starting with 'b' would fall into the
> second one.

I agree that a valid use case like the one above is awkward to express
currently.

> Currently, when there are 2 partition columns, the partition
> constraint is defined as
> 
>   (a is not null) and (b is not null)
>   and
>   (a > al or (a = al and b >= bl))
>   and
>   (a < au or (a = au and b < bu))
> 
> if the upper bound bu were allowed to be -INFINITY (something that
> should probably be forbidden unless the previous column's upper bound
> were finite), then this would simplify to
> 
>   (a is not null) and (b is not null)
>   and
>   (a > al or (a = al and b >= bl))
>   and
>   (a < au)
> 
> and in the example above, where al is -INFINITY, it would further simplify to
> 
>   (a is not null) and (b is not null)
>   and
>   (a >= al)
>   and
>   (a < au)
> 
> There would also be a similar simplification possible if the lower
> bound of a partition column were allowed to be +INFINITY.

Yep.

> So, I think that having UNBOUNDED represent both -INFINITY and
> +INFINITY depending on context is a design flaw, and that we need to
> allow both -INFINITY and +INFINITY as upper and lower bounds (provided
> they are preceded by a column with a finite bound). I think that, in
> general, that's the only way to allow contiguous non-overlapping
> partitions to be defined on multiple columns.

Alright, I spent some time implementing a patch to allow specifying
-infinity and +infinity in arbitrary ways.  Of course, it prevents
nonsensical inputs with appropriate error messages.

When implementing the same, I initially thought that the only grammar
modification required is to allow specifying a sign before the unbounded
keyword, but thought it sounded strange to call the actual bound values
-unbounded and +unbounded.  While the keyword "unbounded" describes the
property of being unbounded, actual values are really -infinity and
+infinity.  So, I decided to instead modify the grammar to accept
-infinity and +infinity in the FROM and TO lists.  The sign is optional
and in its absence, infinity in FROM means -infinity and vice versa.  This
decision may be seen as controversial, now that we are actually in beta,
if we decide to go with this patch at all.

Some adjustments were required in the logic in partition.c that depended
on the old assumption that all infinite values in the lower bound meant
-infinity and vice versa.  That includes get_qual_for_range() being able
to simplify the partition constraint as Dean mentioned in his email.

When testing the patch, I realized that the current code in
check_new_partition_bound() that checks for range partition overlap had a
latent bug that resulted in false positives for the new cases that the new
less restrictive syntax allowed.  I spent some time simplifying that code
while also fixing the aforementioned bug.  It's implemented in the
attached patch 0001.

0002 is the patch that implements the new syntax.

It's possible that this won't be considered a PG 10 open item but a new
feature and so PG 11 material, as Ashutosh also wondered.

Thanks,
Amit
From 

[HACKERS] Fix a typo in aclchk.c

2017-06-30 Thread Masahiko Sawada
Hi,

Attached patch for $subject.

s/entires/entries/

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


fix_typo_in_aclchk_c.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] Default Partition for Range

2017-06-30 Thread Beena Emerson
Hello Dilip,

On Wed, Jun 21, 2017 at 6:27 PM, Dilip Kumar  wrote:
> On Tue, Jun 20, 2017 at 6:57 PM, Dilip Kumar  wrote:
>> This is basically crashing in RelationBuildPartitionDesc, so I think
>> we don't have any test case for testing DEFAULT range partition where
>> partition key has more than one attribute.  So I suggest we can add
>> such test case.
>
> Some more comments.
>
> 
> - bound->datums = (Datum *) palloc0(key->partnatts * sizeof(Datum));
> - bound->content = (RangeDatumContent *) palloc0(key->partnatts *
> -   sizeof(RangeDatumContent));
> + bound->datums = datums ? (Datum *) palloc0(key->partnatts * sizeof(Datum))
> + : NULL;
> + bound->content = (RangeDatumContent *) palloc0(
> + (datums ? key->partnatts : 1) * sizeof(RangeDatumContent));
>   bound->lower = lower;
>
>   i = 0;
> +
> + /* If default, datums are NULL */
> + if (datums == NULL)
> + bound->content[i] = RANGE_DATUM_DEFAULT;
> 
>
> For the default partition we are only setting bound->content[0] to
> default, but content for others key
> attributes are not initialized.  But later in the code, if the content
> of the first key is RANGE_DATUM_DEFAULT then it should not access the
> next content,  but I see there are some exceptions.  Which can access
> uninitialized value?
>
> for example see below code
>
> 
> for (i = 0; i < key->partnatts; i++)
>   {
> + if (rb_content[i] == RANGE_DATUM_DEFAULT)   --> why it's going to
> content for next parttion attribute, we never initialized that?
> + continue;
> +
>   if (rb_content[i] != RANGE_DATUM_FINITE)
>   return rb_content[i] == RANGE_DATUM_NEG_INF ? -1 : 1;
> 
>
> Also
> In RelatiobBuildPartitionDesc
>
> 
> for (j = 0; j < key->partnatts; j++)
> {
> -- Suppose first is RANGE_DATUM_DEFAULT then we should not check next
>because that is never initialized.  I think this is the cause of
> the crash also what I have reported above.
> 
> if (rbounds[i]->content[j] == RANGE_DATUM_FINITE)
> boundinfo->datums[i][j] =
> datumCopy(rbounds[i]->datums[j],
>  key->parttypbyval[j],
>  key->parttyplen[j]);
> /* Remember, we are storing the tri-state value. */
> boundinfo->content[i][j] = rbounds[i]->content[j];
> 
>

Thank you for your review and analysis.

I have updated the patch.
- bound->content is set to RANGE_DATUM_DEFAULT for each of the keys
and not just the first one.
- Improve the way of handling DEFAULT bounds in RelationBuildPartitionDesc,

There is a test for multiple column range in alter_table.sql

-- 

Beena Emerson

EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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