Re: [HACKERS] COPY (query) TO ... doesn't allow parallelism

[Amit Kapila]

On Sat, Jun 3, 2017 at 9:34 PM, Andres Freund  wrote:
> Hi,
>
> On 2017-06-03 17:40:08 +0530, Amit Kapila wrote:
>> The standard_planner check is sufficient to not generate parallel
>> plans for such statements, but it won't prevent if such commands
>> (which shouldn't be executed by parallel workers) are present in
>> functions.  Consider a hypothetical case as below:
>>
>> 1.  Create a parallel safe function containing Copy commands.
>> create or replace function parallel_copy(a integer) returns integer
>> as $$
>> begin
>> Copy (select * from t1 where c1 < 2) to 'e:\\f1';
>> return a;
>> end;
>> $$ language plpgsql Parallel Safe;
>>
>> 2. Now use this in some command which can be executed in parallel.
>> explain analyze select * from t1 where c1 < parallel_copy(10);
>>
>> This can allow Copy command to be executed by parallel workers if we
>> don't have sufficient safeguards.
>
> Yes.  But I'm unclear what does that have to do with the change
> discussed in this thread?
>

It is not related to the change you have proposed.  It just occurred
to me while reading the code in the area where you have proposed to
change, so mentioned here.  It might have been better to report it in
a separate thread.

>  The pg_plan_query in copy.c setting
> CURSOR_OPT_PARALLEL_OK doesn't meaningfully change the risk of this
> happening in one way or the other.
>
>> We already tried to prohibit it in
>> plpgsql like in function _SPI_execute_plan(), we call
>> PreventCommandIfParallelMode.  However, inspite of that, we have
>> safeguards in lower level calls, so that if the code flow reaches such
>> commands in parallel mode, we error out.  We have a similar check in
>> Copy From code flow  ( PreventCommandIfParallelMode("COPY FROM");) as
>> well, but I think we should have it in Copy To flow as well.
>
> Why?  What is it effectively preventing?  Multiple workers copying to
> the same file?
>

Yeah.  Also, the query (in Copy To command) can be a write a statement
as well.  I thought giving an error from COPY TO flow would be
appropriate for it.

>  Any such function would have the same risk for separate
> sessions.
>

You are right, but not sure if it makes sense to allow any form of
write statement in parallel workers without doing more analysis.

-- 
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] ALTER INDEX .. SET STATISTICS ... behaviour

[Amit Kapila]

On Sun, Jun 4, 2017 at 10:11 AM, Tom Lane  wrote:
> Amit Kapila  writes:
>> In order to avoid losing track of this patch, I think it is better to
>> add it in open items list for 10.
>
> This is an entirely new feature, not a bug fix, and thus certainly not an
> open item for v10.  Please stick it in the next commitfest, instead.
>

Okay, that makes sense.  Sorry, I missed the point in the first read
of the mail.

-- 
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] ALTER INDEX .. SET STATISTICS ... behaviour

[Tom Lane]

Amit Kapila  writes:
> In order to avoid losing track of this patch, I think it is better to
> add it in open items list for 10.

This is an entirely new feature, not a bug fix, and thus certainly not an
open item for v10.  Please stick it in the next commitfest, instead.

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] ALTER INDEX .. SET STATISTICS ... behaviour

[Amit Kapila]

On Thu, Jun 1, 2017 at 6:40 PM, Alexander Korotkov
 wrote:
> On Wed, May 31, 2017 at 7:18 PM, Alexander Korotkov
>  wrote:
>>
>>>
>>> Don't like either of those particularly, but what about just targeting
>>> a column by column number, independently of whether it's an expression?
>>>
>>> ALTER INDEX myindex ALTER COLUMN 4 SET STATISTICS 1000;
>>
>>
>> I completely agree with that.
>> If no objections, I will write a patch for that.
>
>
> Please, find it in attached patch.
>
> I decided to forbid referencing columns by numbers for tables, because those
> numbers could contain gaps.  Also, I forbid altering statistics target for
> non-expression index columns, because it has no effect.
>

In order to avoid losing track of this patch, I think it is better to
add it in open items list for 10.



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

[Rafia Sabih]

On Fri, Jun 2, 2017 at 5:48 PM, Robert Haas  wrote:
> On Fri, Jun 2, 2017 at 8:09 AM, Amit Kapila  wrote:
>> I think if you have found spelling mistakes unrelated to this patch,
>> then it is better to submit those as a separate patch in a new thread.
>
> +1.

Sure, attached is the version without those changes.

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


cosmetic_range_default_partition_v2.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] PG10 transition tables, wCTEs and multiple operations on the same table

[Thomas Munro]

On Sun, Jun 4, 2017 at 10:41 AM, Kevin Grittner  wrote:
> On Fri, Jun 2, 2017 at 8:46 PM, Thomas Munro
>  wrote:
>
>> So, afterTriggers.query_stack is used to handle the reentrancy that
>> results from triggers running further statements that might fire
>> triggers.  It isn't used for dealing with extra ModifyTable nodes that
>> can appear in a plan because of wCTEs.  Could it also be used for that
>> purpose?  I think that would only work if it is the case that each
>> ModifyTable node begin and then runs to completion (ie no interleaving
>> of wCTE execution) and then its AS trigger fires, which I'm not sure
>> about.
>
> I don't think we want to commit to anything that depends on a CTE
> creating an optimization fence, although *maybe* that would be OK in
> the case of DML as a CTE.  That's a pretty special case; I'm not
> sure whether the standard discusses it.

According to our manual the standard doesn't allow DML in CTEs.  I
suppose it wouldn't make much sense without RETURNING, which is also
non-standard.

>> If that is the case, perhaps AfterTriggerBeginQuery and
>> AfterTriggerEndQuery could become the responsibility of
>> nodeModifyTable.c rather than execMain.c.  I haven't tried this yet
>> and it may well be too ambitious at this stage.
>
> In a world where one statement can contain multiple DML statements
> within CTEs, that may make sense regardless of transition table
> issues; but I agree we're past the time to be considering making
> such a change for v10.

Ok.

>> Other ideas: (1) ban wCTEs that target relations with transition
>> tables in PG10, because we're out of time;
>
> Given that we haven't even discussed what to do about an UPDATE
> statement with a CTE that updates the same table when there are
> BEFORE EACH ROW UPDATE triggers on that table (which perhaps return
> NULL for some but not all rows?), I'm not sure we've yet plumbed the
> depths of this morass.
>
> [...]
>
> Can we fix things exclusive of transition tables in this release?
> If not, the only reasonable thing to do is to try to avoid further
> complicating things with CTE/transition table usage until we sort
> out the basics of triggers firing from CTE DML in the first place.

At least it's well documented that the execution order is undefined,
but yeah triggers do make things a lot more complicated and I'm not
sure I understand to what extent that is unavoidable once you decide
to allow DML in CTEs.

In the meantime, it seems like you agree that rejecting wCTEs that
affect tables with triggers with transition tables is the best
response to this bug report?  Do you think that parse analysis is the
right time to do the check?  Here's a first attempt at that.

Later, for the next cycle, I think we should investigate storing the
transition tables in ModifyTableState rather than in query_stack, and
then passing them to the Exec*Triggers() functions as arguments
(possibly inside TransitionCaptureState, if you accept my proposal for
the inheritance bug).

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


reject-wcte-with-transition-tables.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] Extra Vietnamese unaccent rules

[Michael Paquier]

On Mon, May 29, 2017 at 10:47 AM, Thomas Munro
 wrote:
>> [Quoting Michael]
>>> Actually, with the recent work that has been done with
>>> unicode_norm_table.h which has been to transpose UnicodeData.txt into
>>> user-friendly tables, shouldn't the python script of unaccent/ be
>>> replaced by something that works on this table? This does a canonical
>>> decomposition but just keeps the first characters with a class
>>> ordering of 0. So we have basic APIs able to look at UnicodeData.txt
>>> and let caller do decision making with the result returned.
>>
>> Thanks, i will learning about it.
>
> It seems like that could be useful for runtime use (I'm sure there is
> a whole world of Unicode support we could add), but here we're only
> trying to generate a mapping file to add to the source tree, so I'm
> not sure how it's relevant.

Yes, that's what I am coming at, but that would be really dictionnary
specific and that would be roughly to provide a fast-path equivalent
to the tsearch_readline* routines working on files. The addition of
new infrastructure may perhaps not be worth the performance gains.
Definitely for this fix there is no need to do anything more
complicated than tweaking the script to generate the rules.
-- 
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] Index created in BEFORE trigger not updated during INSERT

[Andres Freund]

On 2017-06-03 18:23:33 -0400, Tom Lane wrote:
> Attached is a proposed patch that closes off this problem.  I've tested
> it to the extent that it blocks Albe's example and passes check-world.

Cool.

> I'm unsure whether to back-patch or not; the main argument for not doing
> so is that if any extensions are calling DefineIndex() directly, this
> would be an API break for them.  Given what a weird case this is, I'm not
> sure it's worth that.

I slightly lean against backpatching, it has taken long to be reported,
and it's pretty easy to work around...

- 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] Index created in BEFORE trigger not updated during INSERT

[Michael Paquier]

On Sun, Jun 4, 2017 at 7:23 AM, Tom Lane  wrote:
> I'm unsure whether to back-patch or not; the main argument for not doing
> so is that if any extensions are calling DefineIndex() directly, this
> would be an API break for them.  Given what a weird case this is, I'm not
> sure it's worth that.

Knowing that it has taken 20 years to get a report for this problem,
It seems to me that one report does not win against the risk of
destabilizing extensions that would surprisingly break after a minor
update.

> A possible end-run around the API objection would be to not add an extra
> argument to DefineIndex() in the back branches, but to use !is_alter_table
> as the control condition.  That would work for the core callers, although
> we might need a special case for bootstrap mode.  On the other hand,
> thinking again about hypothetical third-party callers, it's possible that
> that's not the right answer for them, in which case they'd be really in
> trouble.  So I'm not that much in love with that answer.

Yes, that's not worth the trouble I think for back-branches.

The patch looks good to me, could you add a regression test? This
could be used later on as a basis for other DDL commands if somebody
looks at this problem for the other ones.
-- 
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] PG10 transition tables, wCTEs and multiple operations on the same table

[Kevin Grittner]

On Fri, Jun 2, 2017 at 8:46 PM, Thomas Munro
 wrote:

> So, afterTriggers.query_stack is used to handle the reentrancy that
> results from triggers running further statements that might fire
> triggers.  It isn't used for dealing with extra ModifyTable nodes that
> can appear in a plan because of wCTEs.  Could it also be used for that
> purpose?  I think that would only work if it is the case that each
> ModifyTable node begin and then runs to completion (ie no interleaving
> of wCTE execution) and then its AS trigger fires, which I'm not sure
> about.

I don't think we want to commit to anything that depends on a CTE
creating an optimization fence, although *maybe* that would be OK in
the case of DML as a CTE.  That's a pretty special case; I'm not
sure whether the standard discusses it.

> If that is the case, perhaps AfterTriggerBeginQuery and
> AfterTriggerEndQuery could become the responsibility of
> nodeModifyTable.c rather than execMain.c.  I haven't tried this yet
> and it may well be too ambitious at this stage.

In a world where one statement can contain multiple DML statements
within CTEs, that may make sense regardless of transition table
issues; but I agree we're past the time to be considering making
such a change for v10.

> Other ideas: (1) ban wCTEs that target relations with transition
> tables in PG10, because we're out of time;

Given that we haven't even discussed what to do about an UPDATE
statement with a CTE that updates the same table when there are
BEFORE EACH ROW UPDATE triggers on that table (which perhaps return
NULL for some but not all rows?), I'm not sure we've yet plumbed the
depths of this morass.

For example, for this:

drop table if exists t1 cascade;

create table t1 (c1 int not null, c2 text not null default '');
insert into t1 (c1) values (1), (2), (3), (4), (5);

drop function if exists t1_upd_func() cascade;
create function t1_upd_func()
  returns trigger
  language plpgsql
as $$
begin
  if old.c1 between 2 and 4 then
new.c2 = old.c2 || ';' || old.c1::text || ',' || new.c1::text;
  end if;
  if old.c1 > 3 then
return null;
  end if;
  return new;
end;
$$;
create trigger t1_upd_update
  before update
  on t1
  for each row execute procedure t1_upd_func();
with x as (update t1 set c1 = c1 - 1 returning c1)
  update t1 set c1 = t1.c1 + 1 from x where x.c1 = t1.c1;
select * from t1;

... what is the correct result?

I get this:

 c1 |  c2
+--
  4 |
  5 |
  0 |
  1 | ;2,1
  2 | ;3,2
(5 rows)

It is definitely not what I expected, and seems wrong in multiple
ways from a logical perspective, looking  at those as set-based
operations.  (Tautologies about the procedural mechanism that
creates the result are not defenses of the result itself.)

Can we fix things exclusive of transition tables in this release?
If not, the only reasonable thing to do is to try to avoid further
complicating things with CTE/transition table usage until we sort
out the basics of triggers firing from CTE DML in the first place.

--
Kevin Grittner
VMware vCenter Server
https://www.vmware.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] Why does logical replication launcher set application_name?

[Michael Paquier]

On Sat, Jun 3, 2017 at 4:33 PM, Kuntal Ghosh  wrote:
> On Sat, Jun 3, 2017 at 2:14 AM, Petr Jelinek
>  wrote:
>> However, I am not sure about the bgw_name_extra. I think I would have
>> preferred keeping full bgw_name field which would be used where full
>> name is needed and bgw_type where only the worker type is used.

Yes, I don't thnk as well that this has any types of gain. With only
bgw_name, it is still possible to append the same prefix to all the
bgworkers of the same type, and do a search on pg_stat_activity using
'~' for example to fetch all the workers with the same string.

>> The concatenation just doesn't sit well with me, especially if it requires
>> the bgw_name_extra to start with space.
>
> +1.

That's not friendly.
-- 
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] Index created in BEFORE trigger not updated during INSERT

[Tom Lane]

I wrote:
> Andres Freund  writes:
>> Hm, strategically sprinkled CheckTableNotInUse() might do the trick?

> +1.  We can't reasonably make it work: the outer query already has its
> list of indexes that need to be inserted into.  Also, if you try to
> make the index via ALTER TABLE ADD CONSTRAINT in the trigger, that will
> give you "cannot ALTER TABLE "mytable" because it is being used by active
> queries in this session" because of the check in AlterTable().

Attached is a proposed patch that closes off this problem.  I've tested
it to the extent that it blocks Albe's example and passes check-world.

I'm unsure whether to back-patch or not; the main argument for not doing
so is that if any extensions are calling DefineIndex() directly, this
would be an API break for them.  Given what a weird case this is, I'm not
sure it's worth that.

A possible end-run around the API objection would be to not add an extra
argument to DefineIndex() in the back branches, but to use !is_alter_table
as the control condition.  That would work for the core callers, although
we might need a special case for bootstrap mode.  On the other hand,
thinking again about hypothetical third-party callers, it's possible that
that's not the right answer for them, in which case they'd be really in
trouble.  So I'm not that much in love with that answer.

> It doesn't seem terribly hard to fix the CREATE INDEX case to behave
> likewise, but I wonder how many other cases we've missed?

That remains an open question :-(

regards, tom lane

diff --git a/src/backend/bootstrap/bootparse.y b/src/backend/bootstrap/bootparse.y
index de3695c..2e1fef0 100644
*** a/src/backend/bootstrap/bootparse.y
--- b/src/backend/bootstrap/bootparse.y
*** Boot_DeclareIndexStmt:
*** 323,328 
--- 323,329 
  $4,
  false,
  false,
+ false,
  true, /* skip_build */
  false);
  	do_end();
*** Boot_DeclareUniqueIndexStmt:
*** 366,371 
--- 367,373 
  $5,
  false,
  false,
+ false,
  true, /* skip_build */
  false);
  	do_end();
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 4861799..c090279 100644
*** a/src/backend/commands/indexcmds.c
--- b/src/backend/commands/indexcmds.c
*** CheckIndexCompatible(Oid oldId,
*** 295,300 
--- 295,303 
   * 'is_alter_table': this is due to an ALTER rather than a CREATE operation.
   * 'check_rights': check for CREATE rights in namespace and tablespace.  (This
   *		should be true except when ALTER is deleting/recreating an index.)
+  * 'check_not_in_use': check for table not already in use in current session.
+  *		This should be true unless caller is holding the table open, in which
+  *		case the caller had better have checked it earlier.
   * 'skip_build': make the catalog entries but leave the index file empty;
   *		it will be filled later.
   * 'quiet': suppress the NOTICE chatter ordinarily provided for constraints.
*** DefineIndex(Oid relationId,
*** 307,312 
--- 310,316 
  			Oid indexRelationId,
  			bool is_alter_table,
  			bool check_rights,
+ 			bool check_not_in_use,
  			bool skip_build,
  			bool quiet)
  {
*** DefineIndex(Oid relationId,
*** 405,410 
--- 409,423 
   errmsg("cannot create indexes on temporary tables of other sessions")));
  
  	/*
+ 	 * Unless our caller vouches for having checked this already, insist that
+ 	 * the table not be in use by our own session, either.  Otherwise we might
+ 	 * fail to make entries in the new index (for instance, if an INSERT or
+ 	 * UPDATE is in progress and has already made its list of target indexes).
+ 	 */
+ 	if (check_not_in_use)
+ 		CheckTableNotInUse(rel, "CREATE INDEX");
+ 
+ 	/*
  	 * Verify we (still) have CREATE rights in the rel's namespace.
  	 * (Presumably we did when the rel was created, but maybe not anymore.)
  	 * Skip check if caller doesn't want it.  Also skip check if
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7959120..b61fda9 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*** ATExecAddIndex(AlteredTableInfo *tab, Re
*** 6679,6684 
--- 6679,6685 
  		  InvalidOid,	/* no predefined OID */
  		  true, /* is_alter_table */
  		  check_rights,
+ 		  false,	/* check_not_in_use - we did it already */
  		  skip_build,
  		  quiet);
  
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 1e941fb..a22fd53 100644
*** a/src/backend/tcop/utility.c
--- b/src/backend/tcop/utility.c
*** ProcessUtilitySlow(ParseState *pstate,
*** 1329,1334 
--- 1329,1335 
  	InvalidOid, /* no predefined OID */
  	false,		/* is_alter_table */
  	true,		/* check_rights */
+ 	true,		/* check_not_in_use */

Re: [HACKERS] Re: [GSOC 17] Eliminate O(N^2) scaling from rw-conflict tracking in serializable transactions

[Kevin Grittner]

On Sat, Jun 3, 2017 at 1:51 AM, Mengxing Liu
 wrote:

> I tried 30 cores. But the CPU utilization is about 45%~70%.
> How can we distinguish  where the problem is? Is disk I/O or Lock?

A simple way is to run `vmstat 1` for a bit during the test.  Can
you post a portion of the output of that here?  If you can configure
the WAL directory to a separate mount point (e.g., use the --waldir
option of initdb), a snippet of `iostat 1` output would be even
better.

I think the best thing may be if you can generate a CPU flame graph
of the worst case you can make for these lists:
http://www.brendangregg.com/flamegraphs.html  IMO, such a graph
highlights the nature of the problem better than anything else.

> Does "traversing these list" means the function "RWConflictExists"?
> And "10% waiting on the corresponding kernel mutexes" means the
> lock in the function CheckForSerializableIn/out ?

Since they seemed to be talking specifically about the conflict
list, I had read that as SerializableXactHashLock, although the
wording is a bit vague -- they may mean something more inclusive.

> If I used 30 cores for server, and 90 clients, RWConflictExists
> consumes 1.0% CPU cycles, and CheckForSerializableIn/out takes 5%
> in all.
> But the total CPU utilization of PG is about 50%. So the result
> seems to be matched.
> If we can solve this problem, we can use this benchmark for the
> future work.

If you can get a flame graph of CPU usage on this load, that would
be ideal.  At that point, we can discuss what is best to attack.
Reducing something that is 10% of the total PostgreSQL CPU load in a
particular workload sounds like it could still have significant
value, although if you see a way to attack the other 90% (or some
portion of it larger than 10%) instead, I think we could adjust the
scope based on those results.

--
Kevin Grittner
VMware vCenter Server
https://www.vmware.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] Tweaking tab completion for some backslash commands

[Tom Lane]

Masahiko Sawada  writes:
> Attached patch tweaks tab completion for some backslash commands.

Pushed, thanks!

regards, tom lane


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

Re: [HACKERS] Create subscription with `create_slot=false` and incorrect slot name

[Dmitry Dolgov]

> On 26 May 2017 at 23:05, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:
>
> > On 5/25/17 19:16, Petr Jelinek wrote:
> >> The reported error is just one of many errors that can happen when DROP
> >> SUBSCRIPTION tries to drop the slot (doens't exist, still active, no
> >> permission, etc.).  We don't want to give the hint that is effectively
> >> "just forget about the slot then" for all of them.  So we would need
> >> some way to distinguish "you can't do this right now" from "this would
> >> never work" (400 vs 500 errors).
> >>
> > This specific error returns ERRCODE_UNDEFINED_OBJECT error code so we
> > could check for it and offer hint only for this case.
>
> We would have to extend the walreceiver interface a little to pass that
> through, but it seems doable.

Just to make it clear for me. If I understand correctly, it should be more
or
less easy to extend it in that way, something like in the attached patch.
Or am I missing something?
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 86eb31d..9f7d73c 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -941,10 +941,20 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
 		res = walrcv_exec(wrconn, cmd.data, 0, NULL);
 
 		if (res->status != WALRCV_OK_COMMAND)
-			ereport(ERROR,
-			(errmsg("could not drop the replication slot \"%s\" on publisher",
-	slotname),
-			 errdetail("The error was: %s", res->err)));
+		{
+			if (res->errcode == ERRCODE_UNDEFINED_OBJECT)
+ereport(ERROR,
+(errmsg("could not drop the replication slot \"%s\" on publisher",
+		slotname),
+ errdetail("The error was: %s", res->err),
+ errhint("Use ALTER SUBSCRIPTION ... SET (slot_name = NONE) "
+		 "to disassociate the subscription from the slot.")));
+			else
+ereport(ERROR,
+(errmsg("could not drop the replication slot \"%s\" on publisher",
+		slotname),
+ errdetail("The error was: %s", res->err)));
+		}
 		else
 			ereport(NOTICE,
 	(errmsg("dropped replication slot \"%s\" on publisher",
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index ebe9c91..1caa051 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -873,6 +873,7 @@ libpqrcv_exec(WalReceiverConn *conn, const char *query,
 {
 	PGresult   *pgres = NULL;
 	WalRcvExecResult *walres = palloc0(sizeof(WalRcvExecResult));
+	const char *errorcode = NULL;
 
 	if (MyDatabaseId == InvalidOid)
 		ereport(ERROR,
@@ -915,6 +916,9 @@ libpqrcv_exec(WalReceiverConn *conn, const char *query,
 		case PGRES_FATAL_ERROR:
 		case PGRES_BAD_RESPONSE:
 			walres->status = WALRCV_ERROR;
+			errorcode = PQresultErrorField(pgres, PG_DIAG_SQLSTATE);
+			walres->errcode = MAKE_SQLSTATE(errorcode[0], errorcode[1], errorcode[2],
+			errorcode[3], errorcode[4]);
 			walres->err = pchomp(PQerrorMessage(conn->streamConn));
 			break;
 	}
diff --git a/src/include/replication/walreceiver.h b/src/include/replication/walreceiver.h
index 31d090c..b0582af 100644
--- a/src/include/replication/walreceiver.h
+++ b/src/include/replication/walreceiver.h
@@ -186,6 +186,7 @@ typedef struct WalRcvExecResult
 {
 	WalRcvExecStatus status;
 	char	   *err;
+	int			errcode;
 	Tuplestorestate *tuplestore;
 	TupleDesc	tupledesc;
 } WalRcvExecResult;

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

[Peter Eisentraut]

On 6/2/17 14:52, Peter Eisentraut wrote:
> On 5/24/17 15:14, Petr Jelinek wrote:
>> All the locking works just fine the way it is in master. The issue with
>> deadlock with apply comes from the wrong handling of the SIGTERM in the
>> apply (we didn't set InterruptPending). I changed the SIGTERM handler in
>> patch 0001 just to die which is actually the correct behavior for apply
>> workers. I also moved the connection cleanup code to the
>> before_shmem_exit callback (similar to walreceiver) and now that part
>> works correctly.
> 
> I have committed this, in two separate parts.  This should fix the
> originally reported issue.
> 
> I will continue to work through your other patches.  I notice there is
> still a bit of discussion about another patch, so please let me know if
> there is anything else I should be looking for.

I have committed the remaining two patches.  I believe this fixes the
originally reported issue.

-- 
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] COPY (query) TO ... doesn't allow parallelism

[Andres Freund]

Hi,

On 2017-06-03 17:40:08 +0530, Amit Kapila wrote:
> The standard_planner check is sufficient to not generate parallel
> plans for such statements, but it won't prevent if such commands
> (which shouldn't be executed by parallel workers) are present in
> functions.  Consider a hypothetical case as below:
> 
> 1.  Create a parallel safe function containing Copy commands.
> create or replace function parallel_copy(a integer) returns integer
> as $$
> begin
> Copy (select * from t1 where c1 < 2) to 'e:\\f1';
> return a;
> end;
> $$ language plpgsql Parallel Safe;
> 
> 2. Now use this in some command which can be executed in parallel.
> explain analyze select * from t1 where c1 < parallel_copy(10);
> 
> This can allow Copy command to be executed by parallel workers if we
> don't have sufficient safeguards.

Yes.  But I'm unclear what does that have to do with the change
discussed in this thread?  The pg_plan_query in copy.c setting
CURSOR_OPT_PARALLEL_OK doesn't meaningfully change the risk of this
happening in one way or the other.

> We already tried to prohibit it in
> plpgsql like in function _SPI_execute_plan(), we call
> PreventCommandIfParallelMode.  However, inspite of that, we have
> safeguards in lower level calls, so that if the code flow reaches such
> commands in parallel mode, we error out.  We have a similar check in
> Copy From code flow  ( PreventCommandIfParallelMode("COPY FROM");) as
> well, but I think we should have it in Copy To flow as well.

Why?  What is it effectively preventing?  Multiple workers copying to
the same file?  Any such function would have the same risk for separate
sessions.

Greetings,

Andres Freund


-- 
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] Extra Vietnamese unaccent rules

[Dang Minh Huong]

On May 30, 29 Heisei, at 00:22, Dang Minh Huong  wrote:

unaccent.patch
Description: Binary data
On May 29, 29 Heisei, at 10:47, Thomas Munro  wrote:On Sun, May 28, 2017 at 7:55 PM, Dang Minh Huong  wrote:Thanks for reporting and lecture about unicode.I attached a patch as the instruction from Thomas. Could you confirm it.-   is_plain_letter(table[codepoint.combining_ids[0]]) and \+   (is_plain_letter(table[codepoint.combining_ids[0]]) or\+    len(table[codepoint.combining_ids[0]].combining_ids) > 1) and \Shouldn't you use "or is_letter_with_marks()", instead of "or len(...)1"?  Your test might catch something that isn't based on a 'letter'(according to is_plain_letter).  Otherwise this looks pretty good tome.  Please add it to the next commitfest.Thanks for confirm, sir.I will add it to the next CF soon.Sorry for lately response. I attach the update patch.---Thanks and best regards,Dang Minh Huong

Re: [HACKERS] logical replication - still unstable after all these months

[Jeff Janes]

On Fri, Jun 2, 2017 at 4:10 PM, Petr Jelinek 
wrote:

>
> While I was testing something for different thread I noticed that I
> manage transactions incorrectly in this patch. Fixed here, I didn't test
> it much yet (it takes a while as you know :) ). Not sure if it's related
> to the issue you've seen though.
>

This conflicts again with Peter E's recent commit
3c9bc2157a4f465b3c070d1250597568d2dc285f

Thanks,

Jeff

Re: [HACKERS] [POC] hash partitioning

[Dilip Kumar]

On Thu, May 25, 2017 at 9:59 AM, amul sul  wrote:
> On Mon, May 22, 2017 at 2:23 PM, amul sul  wrote:
>>
>> Updated patch attached. Thanks a lot for review.
>>
> Minor fix in the document, PFA.

Patch need rebase

---
Function header is not consistent with other neighbouring functions
(some function contains function name in the header but others don't)
+/*
+ * Compute the hash value for given not null partition key values.
+ */

--
postgres=# create table t1 partition of t for values with (modulus 2,
remainder 1) partition by range(a);
CREATE TABLE
postgres=# create table t1_1 partition of t1 for values from (8) to (10);
CREATE TABLE
postgres=# insert into t1 values(8);
2017-06-03 18:41:46.067 IST [5433] ERROR:  new row for relation "t1_1"
violates partition constraint
2017-06-03 18:41:46.067 IST [5433] DETAIL:  Failing row contains (8).
2017-06-03 18:41:46.067 IST [5433] STATEMENT:  insert into t1 values(8);
ERROR:  new row for relation "t1_1" violates partition constraint
DETAIL:  Failing row contains (8).

The value 8 is violating the partition constraint of the t1 and we are
trying to insert to value in t1,
still, the error is coming from the leaf level table t1_1, that may be
fine but from error, it appears that
it's violating the constraint of t1_1 whereas it's actually violating
the constraint of t1.

>From Implementation, it appears that based on the key are identifying
the leaf partition and it's only failing during ExecInsert while
checking the partition constraint.

Other than that, patch looks fine to me.

-- 
Regards,
Dilip Kumar
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: retry shm attach for windows (WAS: Re: [HACKERS] OK, so culicidae is *still* broken)

[Amit Kapila]

On Fri, Jun 2, 2017 at 7:20 PM, Petr Jelinek
 wrote:
> On 02/06/17 15:37, Amit Kapila wrote:
>>
>> No, it is to avoid calling free of memory which is not reserved on
>> retry.  See the comment:
>> + * On the first try, release memory region reservation that was made by
>> + * the postmaster.
>>
>> Are you referring to the same function in sysv_shm.c, if so probably I
>> can say refer the same API in win32_shmem.c or maybe add a similar
>> comment there as well?
>>
>
> Yeah something like that would help, but my main confusion comes from
> the fact that there is counter (and even named as such) but only
> relevant difference is 0 and not 0. I'd like mention of that mainly
> since I was confused by that on the first read.
>

Okay, I have added the comment to explain the same.  I have also
modified the patch to adjust the looping as per your suggestion.

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


win_shm_retry_reattach_v3.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] COPY (query) TO ... doesn't allow parallelism

[Amit Kapila]

On Thu, Jun 1, 2017 at 10:16 PM, Andres Freund  wrote:
> On 2017-06-01 21:37:56 +0530, Amit Kapila wrote:
>> On Thu, Jun 1, 2017 at 9:34 PM, Andres Freund  wrote:
>> > On 2017-06-01 21:23:04 +0530, Amit Kapila wrote:
>> >> On a related note, I think it might be better to have an
>> >> IsInParallelMode() check in this case as we have at other places.
>> >> This is to ensure that if this command is invoked via plpgsql function
>> >> and that function runs is the parallel mode, it will act as a
>> >> safeguard.
>> >
>> > Hm? Which other places do it that way?  Isn't standard_planner()
>> > centralizing such a check?
>> >
>>
>> heap_insert->heap_prepare_insert, heap_update, heap_delete, etc.
>
> Those aren't comparable, they're not invoking the planner - and all the
> places that set PARALLEL_OK don't check for it.  The relevant check for
> planning is in standard_planner().
>

The standard_planner check is sufficient to not generate parallel
plans for such statements, but it won't prevent if such commands
(which shouldn't be executed by parallel workers) are present in
functions.  Consider a hypothetical case as below:

1.  Create a parallel safe function containing Copy commands.
create or replace function parallel_copy(a integer) returns integer
as $$
begin
Copy (select * from t1 where c1 < 2) to 'e:\\f1';
return a;
end;
$$ language plpgsql Parallel Safe;

2. Now use this in some command which can be executed in parallel.
explain analyze select * from t1 where c1 < parallel_copy(10);

This can allow Copy command to be executed by parallel workers if we
don't have sufficient safeguards.  We already tried to prohibit it in
plpgsql like in function _SPI_execute_plan(), we call
PreventCommandIfParallelMode.  However, inspite of that, we have
safeguards in lower level calls, so that if the code flow reaches such
commands in parallel mode, we error out.  We have a similar check in
Copy From code flow  ( PreventCommandIfParallelMode("COPY FROM");) as
well, but I think we should have it in Copy To flow as well.

I agree that at first place user shouldn't mark such functions as
parallel safe, but having such safeguards can prevent us from problems
where users have incorrectly marked some functions as parallel safe.

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

[Ashutosh Bapat]

Here's some detailed review of the code.

@@ -1883,6 +1883,15 @@ heap_drop_with_catalog(Oid relid)
 if (OidIsValid(parentOid))
 {
 /*
+ * Default partition constraints are constructed run-time from the
+ * constraints of its siblings(basically by negating them), so any
+ * change in the siblings needs to rebuild the constraints of the
+ * default partition. So, invalidate the sibling default partition's
+ * relcache.
+ */
+InvalidateDefaultPartitionRelcache(parentOid);
+
Do we need a lock on the default partition for doing this? A query might be
scanning the default partition directly and we will invalidate the relcache
underneath it. What if two partitions are being dropped simultaneously and
change default constraints simultaneously. Probably the lock on the parent
helps there, but need to check it. What if the default partition cache is
invalidated because partition gets added/dropped to the default partition
itself. If we need a lock on the default partition, we will need to
check the order in which we should be obtaining the locks so as to avoid
deadlocks. This also means that we have to test PREPARED statements involving
default partition. Any addition/deletion/attach/detach of other partition
should invalidate those cached statements.

+if (partition_bound_has_default(boundinfo))
+{
+overlap = true;
+with = boundinfo->default_index;
+}
You could possibly rewrite this as
overlap = partition_bound_has_default(boundinfo);
with = boundinfo->default_index;
that would save one indentation and a conditional jump.

+if (partdesc->nparts > 0 && partition_bound_has_default(boundinfo))
+check_default_allows_bound(parent, spec);
If the table has a default partition, nparts > 0, nparts > 0 check looks
redundant. The comments above should also explain that this check doesn't
trigger when a default partition is added since we don't expect an existing
default partition in such a case.

+ * Checks if there exists any row in the default partition that passes the
+ * check for constraints of new partition, if any reports an error.
grammar two conflicting ifs in the same statement. You may want to rephrase
this as "This function checks if there exists a row in the default
partition that fits in the new
partition and throws an error if it finds one."

+if (new_spec->strategy != PARTITION_STRATEGY_LIST)
+return;
This should probably be an Assert. When default range partition is supported
this function would silently return, meaning there is no row in the default
partition which fits the new partition. We don't want that behavior.

The code in check_default_allows_bound() to check whether the default partition
has any rows that would fit new partition looks quite similar to the code in
ATExecAttachPartition() checking whether all rows in the table being attached
as a partition fit the partition bounds. One thing that
check_default_allows_bound() misses is, if there's already a constraint on the
default partition refutes the partition constraint on the new partition, we can
skip the scan of the default partition since it can not have rows that would
fit the new partition. ATExecAttachPartition() has code to deal with a similar
case i.e. the table being attached has a constraint which implies the partition
constraint. There may be more cases which check_default_allows_bound() does not
handle but ATExecAttachPartition() handles. So, I am wondering whether it's
better to somehow take out the common code into a function and use it. We will
have to deal with a difference through. The first one would throw an error when
finding a row that satisfies partition constraints whereas the second one would
throw an error when it doesn't find such a row. But this difference can be
handled through a flag or by negating the constraint. This would also take care
of Amit Langote's complaint about foreign partitions. There's also another
difference that the ATExecAttachPartition() queues the table for scan and the
actual scan takes place in ATRewriteTable(), but there is not such queue while
creating a table as a partition. But we should check if we can reuse the code to
scan the heap for checking a constraint.

In case of ATTACH PARTITION, probably we should schedule scan of default
partition in the alter table's work queue like what ATExecAttachPartition() is
doing for the table being attached. That would fit in the way alter table
works.

 make_partition_op_expr(PartitionKey key, int keynum,
-   uint16 strategy, Expr *arg1, Expr *arg2)
+uint16 strategy, Expr *arg1, Expr *arg2, bool is_default)
Indentation

+if (is_default &&
+((operoid = get_negator(operoid)) == InvalidOid))
+ereport(ERROR, 

Re: [HACKERS] Why does logical replication launcher set application_name?

[Kuntal Ghosh]

On Sat, Jun 3, 2017 at 8:53 AM, Peter Eisentraut
 wrote:
> On 6/2/17 15:08, Peter Eisentraut wrote:
>> On 5/30/17 23:10, Peter Eisentraut wrote:
>>> Here is a proposed solution that splits bgw_name into bgw_type and
>>> bgw_name_extra.  bgw_type shows up in pg_stat_activity.backend_type.
>>> Uses of application_name are removed, because they are no longer
>>> necessary to identity the process type.
>>>
>>> This code appears to be buggy because I sometimes get NULL results of
>>> the backend_type lookup, implying that it couldn't find the background
>>> worker slot.  This needs another look.
>>
AFAICU, when we register a background worker using
RegisterDynamicBackgroundWorker, it's not included in
BackgroundWorkerList(which is postmaster's list of registered
background workers, in private memory). Instead, we use only
BackgroundWorkerSlots. Perhaps, this is the reason that backend_type
is NULL for parallel workers.

-- 
Thanks & Regards,
Kuntal Ghosh
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] Why does logical replication launcher set application_name?

[Kuntal Ghosh]

On Sat, Jun 3, 2017 at 2:14 AM, Petr Jelinek
 wrote:
> On 02/06/17 21:05, Peter Eisentraut wrote:
>> On 6/2/17 02:31, Masahiko Sawada wrote:
>>> I'd say current patch makes the user difficult to
>>> distinguish between apply worker and table sync worker.
>>
>> We could arguably make apply workers and sync workers have different
>> bgw_type values.  But if you are interested in that level of detail, you
>> should perhaps look at pg_stat_subscription.  pg_stat_activity only
>> contains the "common" data, and the process-specific data is in other views.
>>
>
> Agreed with this.
>
> However, I am not sure about the bgw_name_extra. I think I would have
> preferred keeping full bgw_name field which would be used where full
> name is needed and bgw_type where only the worker type is used. The
> concatenation just doesn't sit well with me, especially if it requires
> the bgw_name_extra to start with space.
>
+1.



-- 
Thanks & Regards,
Kuntal Ghosh
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] Re: [GSOC 17] Eliminate O(N^2) scaling from rw-conflict tracking in serializable transactions

[Mengxing Liu]

> -Original Messages-
> From: "Kevin Grittner" 
> Sent Time: 2017-06-03 01:44:16 (Saturday)
> To: "Alvaro Herrera" 
> Cc: "Mengxing Liu" , 
> "pgsql-hackers@postgresql.org" 
> Subject: Re: Re: Re: [HACKERS] Re: [GSOC 17] Eliminate O(N^2) scaling from 
> rw-conflict tracking in serializable transactions
> 
> > Mengxing Liu wrote:
> 
> >> The CPU utilization of CheckForSerializableConflictOut/In is
> >> 0.71%/0.69%.
> 
> How many cores were on the system used for this test?  The paper
> specifically said that they didn't see performance degradation on
> the PostgreSQL implementation until 32 concurrent connections with
> 24 or more cores.  The goal here is to fix *scaling* problems.  Be
> sure you are testing at an appropriate scale.  (The more sockets,
> cores, and clients, the better, I think.)
> 
> 

I used 15 cores for server and 50 clients. 
I tried 30 cores. But the CPU utilization is about 45%~70%. 
How can we distinguish  where the problem is? Is disk I/O or Lock ?

> On Fri, Jun 2, 2017 at 10:08 AM, Alvaro Herrera
>  wrote:
> 
> > Kevin mentioned during PGCon that there's a paper by some group in
> > Sydney that developed a benchmark on which this scalability
> > problem showed up very prominently.
> 
> https://pdfs.semanticscholar.org/6c4a/e427e6f392b7dec782b7a60516f0283af1f5.pdf
> 
> "[...] we see a much better scalability of pgSSI than the
> corresponding implementations on InnoDB. Still, the overhead of
> pgSSI versus standard SI increases significantly with higher MPL
> than one would normally expect, reaching around 50% with MPL 128."
> 

Actually, I implemented the benchmark described in the paper at first. I 
reported the result in a previous email.
The problem is that the transaction with longer conflict list is easier to be 
aborted, so the conflict list can not grow too long.
I modify the benchmark by using Update-only transaction and Read-only 
transaction to get rid of this problem. In this way, dangerous structure will 
never be generated.

> "Our profiling showed that PostgreSQL spend 2.3% of the overall
> runtime in traversing these list, plus 10% of its runtime waiting on
> the corresponding kernel mutexes."
> 

Does "traversing these list" means the function "RWConflictExists"? 
And "10% waiting on the corresponding kernel mutexes" means the lock in the 
function CheckForSerializableIn/out ?

> If you cannot duplicate their results, you might want to contact the
> authors for more details on their testing methodology.
> 

If I used 30 cores for server, and 90 clients, RWConflictExists consumes 1.0% 
CPU cycles, and CheckForSerializableIn/out takes 5% in all. 
But the total CPU utilization of PG is about 50%. So the result seems to be 
matched. 
If we can solve this problem, we can use this benchmark for the future work.
 

Sincerely

--
Mengxing Liu










-- 
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] Range Merge Join v1

[Andrew Borodin]

2017-06-02 19:42 GMT+05:00 Jeff Davis :
> On Tue, May 30, 2017 at 11:44 PM, Andrew Borodin  wrote:
>> 1. Are there any types, which could benefit from Range Merge and are
>> not covered by this patch?
>
> I thought about this for a while, and the only thing I can think of
> are range joins that don't explicitly use range types.

Let me try to write && in SQL

select * from a join b where (a.min<=b.max and a.min>=b.min) or
(a.max<=b.max and a.max>=b.min);

Quite complicated. Here user knows that min <= max, but DB don't. If
we write it precisely we get hell lot of permutations.
Here's what I think:
1. For me, this feature seems hard to implement.
2. This feature also will be hard to commit solely, since it's use
case will be rare.
3. If this could yield unexpected performance for queries like
select * from t where xc or a!=z or [other conditions]
and optimizer could think: "Aha! I'll sort it and do it fast" it'd be cool.

I do not think range joins that don't explicitly use range types are
possible right now...

>> 2. Can Range Merge handle merge of different ranges? Like int4range()
>> && int8range() ?
>
> Right now, there aren't even casts between range types. I think the
> best way to handle that at this point would be to add casts among the
> numeric ranges. There may be advantages to supporting any two ranges
> where the contained types are part of the same opfamily, but it seems
> a little early to add that complication.
Agree.


> I think there are a couple more things that could be done if we want
> to. Let me know if you think these things should be done now, or if
> they should be a separate patch later when the need arises:
>
> * Support for r1 @> r2 joins (join on "contains" rather than "overlaps").
> * Better integration with the catalog so that users could add their
> own types that support range merge join.

1. I believe this changes can be incremental. They constitute value
for the end user, then they are committable. This patch is not overly
complicated, but it's easier to do this stuff in small pieces.
2. The commitfest is 3 months away. If you will have new versions of
the patch, I'll review again. Maybe will spot some new things :)

Best regards, Andrey Borodin, Octonica.


-- 
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] walsender & parallelism

[Andres Freund]

On 2017-06-02 22:12:46 -0700, Noah Misch wrote:
> On Fri, Jun 02, 2017 at 11:27:55PM -0400, Peter Eisentraut wrote:
> > On 5/31/17 23:54, Peter Eisentraut wrote:
> > > On 5/29/17 22:01, Noah Misch wrote:
> > >> On Tue, May 23, 2017 at 01:45:59PM -0400, Andres Freund wrote:
> > >>> On May 23, 2017 1:42:41 PM EDT, Petr Jelinek 
> > >>>  wrote:
> >  Hi,
> > 
> >  so this didn't really move anywhere AFAICS, do we think the approach
> >  I've chosen is good or do we want to do something else here?
> > >>>
> > >>> Can you add it to the open items list?
> > >>
> > >> [Action required within three days.  This is a generic notification.]
> > > 
> > > I have posted an update.  The next update will be Friday.
> > 
> > Andres is now working on this.  Let's give him a bit of time. ;-)
> 
> If you intended this as your soon-due status update, it is missing a mandatory
> bit.

I'm now owning this item.  I've posted patches, await review.  If none
were to be forthcoming, I'll do another pass Monday, and commit.

- Andres


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