Re: [HACKERS] Partitioning vs ON CONFLICT

[Rukh Meski]

On Fri, Mar 31, 2017 at 11:44 PM, Robert Haas  wrote:
> On Fri, Mar 31, 2017 at 5:33 PM, Peter Geoghegan  wrote:
>> In my opinion, for the very limited ON CONFLICT DO NOTHING + no
>> inference specification case, the implementation should not care about
>> the presence or absence of unique indexes within or across partitions.
>
> Hmm.  That's an interesting point.  The documentation says:
>
> ON CONFLICT can be used to specify an alternative action to raising a
> unique constraint or exclusion constraint violation error.
>
> And, indeed, you could get an unique constraint or exclusion error
> because of an index on the child even though it's not global to the
> partitioning hierarchy.  So maybe we can support this after all, but
> having messed it up once, I'm inclined to think we should postpone
> this to v11, think it over some more, and try to make sure that our
> second try doesn't crash...

Naturally this means that the partitioning work will be reverted as
well, since we have a consensus that new features shouldn't make
preexisting ones worse. It's a shame, since I was really hoping to see
it in 10.0.

♜


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

Re: [HACKERS] pgbench throttling latency limit

[Rukh Meski]

Hi Fabien,

On Sun, Aug 24, 2014 at 9:16 AM, Fabien COELHO coe...@cri.ensmp.fr wrote:
 Find attached a new version:
  - fix dropped percent computation in the final report
  - simplify progress report code

I have reviewed this patch.

Is the patch in a patch format which has context?
  Yes.
Does it apply cleanly to the current git master?
  Yes.
Does it include reasonable tests, necessary doc patches, etc?
  Yes.

Does the patch actually implement that?
  Yes.
Do we want that?
  I think we do, yes.
Do we already have it?
  No.
Are there dangers?
  None that I can see.

Does the feature work as advertised?
  Almost, see below.
Are there corner cases the author has failed to consider?
  None that I can see.
Are there any assertion failures or crashes?
  No.

I can't make the -L option work at all.  If I do this:
  ./pgbench -R 100 -L 1
I get:
  pgbench: invalid option -- L
Which appears to be caused by the fact that the call to getopt_long()
has not been updated to reflect the new parameter.

Also this part:
+-L, --limit=NUM  under throttling (--rate), skip
transactions that\n
+ far behind schedule in ms
(default: do not skip)\n
I would suggest rewording this to something like skip transactions
that are more than NUM milliseconds behind schedule (default: do not
skip).

Marking Waiting for Author until these small issues have been fixed.


Thanks,

♜


-- 
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] pgbench throttling latency limit

[Rukh Meski]

Hi Fabien,

On Tue, Aug 26, 2014 at 04:07 AM, Fabien COELHO coe...@cri.ensmp.fr wrote:

 Please find attached a new version which fixes these two points.

Indeed it does.  Marking the patch ready for a committer.


Thanks,

♜


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

[HACKERS] LIMIT for UPDATE and DELETE

[Rukh Meski]

Greetings,

Based on the feedback on my previous patch, I've separated only the
LIMIT part into its own feature.  This version plays nicely with
inheritance.  The intended use is splitting up big UPDATEs and DELETEs
into batches more easily and efficiently.


♜
*** a/doc/src/sgml/ref/delete.sgml
--- b/doc/src/sgml/ref/delete.sgml
***
*** 25,30  PostgreSQL documentation
--- 25,31 
  DELETE FROM [ ONLY ] replaceable class=PARAMETERtable_name/replaceable 
[ * ] [ [ AS ] replaceable class=parameteralias/replaceable ]
  [ USING replaceable class=PARAMETERusing_list/replaceable ]
  [ WHERE replaceable class=PARAMETERcondition/replaceable | WHERE 
CURRENT OF replaceable class=PARAMETERcursor_name/replaceable ]
+ [ LIMIT { replaceable class=parametercount/replaceable | ALL } ]
  [ RETURNING * | replaceable 
class=parameteroutput_expression/replaceable [ [ AS ] replaceable 
class=parameteroutput_name/replaceable ] [, ...] ]
  /synopsis
   /refsynopsisdiv
***
*** 56,61  DELETE FROM [ ONLY ] replaceable 
class=PARAMETERtable_name/replaceable [ *
--- 57,70 
/para
  
para
+If the literalLIMIT/ (or literalFETCH FIRST/) clause
+is present, processing will stop after the system has deleted the
+specified amount of rows.  Unlike in literalSELECT/, the
+literalOFFSET/literal clause is not available in
+literalDELETE/.
+   /para
+ 
+   para
 The optional literalRETURNING/ clause causes commandDELETE/
 to compute and return value(s) based on each row actually deleted.
 Any expression using the table's columns, and/or columns of other
*** a/doc/src/sgml/ref/update.sgml
--- b/doc/src/sgml/ref/update.sgml
***
*** 29,34  UPDATE [ ONLY ] replaceable 
class=PARAMETERtable_name/replaceable [ * ] [
--- 29,35 
  } [, ...]
  [ FROM replaceable class=PARAMETERfrom_list/replaceable ]
  [ WHERE replaceable class=PARAMETERcondition/replaceable | WHERE 
CURRENT OF replaceable class=PARAMETERcursor_name/replaceable ]
+ [ LIMIT { replaceable class=parametercount/replaceable | ALL } ]
  [ RETURNING * | replaceable 
class=parameteroutput_expression/replaceable [ [ AS ] replaceable 
class=parameteroutput_name/replaceable ] [, ...] ]
  /synopsis
   /refsynopsisdiv
***
*** 51,56  UPDATE [ ONLY ] replaceable 
class=PARAMETERtable_name/replaceable [ * ] [
--- 52,66 
 circumstances.
/para
  
+ 
+   para
+If the literalLIMIT/ (or literalFETCH FIRST/) clause
+is present, processing will stop after the system has updated
+the specified amount of rows.  Unlike in literalSELECT/, the
+literalOFFSET/literal clause is not available in
+literalUPDATE/.
+   /para
+ 
para
 The optional literalRETURNING/ clause causes commandUPDATE/
 to compute and return value(s) based on each row actually updated.
*** a/src/backend/executor/nodeModifyTable.c
--- b/src/backend/executor/nodeModifyTable.c
***
*** 323,329  ExecDelete(ItemPointer tupleid,
   TupleTableSlot *planSlot,
   EPQState *epqstate,
   EState *estate,
!  bool canSetTag)
  {
ResultRelInfo *resultRelInfo;
RelationresultRelationDesc;
--- 323,329 
   TupleTableSlot *planSlot,
   EPQState *epqstate,
   EState *estate,
!  int64_t *processed)
  {
ResultRelInfo *resultRelInfo;
RelationresultRelationDesc;
***
*** 480,487  ldelete:;
 */
}
  
!   if (canSetTag)
!   (estate-es_processed)++;
  
/* AFTER ROW DELETE Triggers */
ExecARDeleteTriggers(estate, resultRelInfo, tupleid, oldtuple);
--- 480,486 
 */
}
  
!   (*processed)++;
  
/* AFTER ROW DELETE Triggers */
ExecARDeleteTriggers(estate, resultRelInfo, tupleid, oldtuple);
***
*** 572,578  ExecUpdate(ItemPointer tupleid,
   TupleTableSlot *planSlot,
   EPQState *epqstate,
   EState *estate,
!  bool canSetTag)
  {
HeapTuple   tuple;
ResultRelInfo *resultRelInfo;
--- 571,577 
   TupleTableSlot *planSlot,
   EPQState *epqstate,
   EState *estate,
!  int64_t *processed)
  {
HeapTuple   tuple;
ResultRelInfo *resultRelInfo;
***
*** 771,778  lreplace:;

   estate);
}
  
!   if (canSetTag)
!   (estate-es_processed)++;
  
/* AFTER ROW UPDATE Triggers */
ExecARUpdateTriggers(estate, resultRelInfo, tupleid, oldtuple, tuple,
--- 770,776 

 

Re: [HACKERS] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..

[Rukh Meski]

On Tue, Jun 24, 2014 at 04:08 AM, Heikki Linnakangas
hlinnakan...@vmware.com wrote:
 IMHO this needs to work with inheritance if we are to accept it. It would be
 a rather strange limitation for no apparent reason, other than that we
 didn't bother to implement it. It doesn't seem very difficult in theory to
 add the table OID to the plan as a junk column, and use that in the
 ModifyTable node to know which table a row came from.

I can have a go at that, but I'm somewhat afraid of the performance
implications this might have.  And it's not just users of this feature
that would pay the penalty, it would be everyone.

 Per the docs in the patch:

 +  para
 +   If the literalLIMIT/ (or literalFETCH FIRST/) clause
 +   is present, processing will stop after the system has attempted
 +   to delete the specified amount of rows.  In particular, if a row
 +   was concurrently changed not to match the given literalWHERE/
 +   clause, it will count towards the literalLIMIT/ despite it
 +   not being actually deleted.  Unlike in literalSELECT/, the
 +   literalOFFSET/literal clause is not available in
 +   literalDELETE/.
 +  /para


 That behavior with READ COMMITTED mode and concurrent changes is surprising.
 Do we really want it to behave like that, and if so, why?

Oh, oops.  Looks like I didn't submit the latest version of the patch
for the commit fest, where I had fixed the documentation.  It doesn't
work that way anymore, as we really don't want it to work that way.

 Why is OFFSET not supported? Not that I care much for that, but I'm curious.

I thought it seemed weird.  But it's supported for FOR UPDATE, so
maybe we should support it here as well.


♜


-- 
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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..

[Rukh Meski]

On Sun, May 11, 2014 at 4:47 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 The $64 question is whether we'd accept an implementation that fails
 if the target table has children (ie, is partitioned).  That seems
 to me to not be up to the project's usual quality expectations, but
 maybe if there's enough demand for a partial solution we should do so.

 It strikes me that a big part of the problem here is that the current
 support for this case assumes that the children don't all have the
 same rowtype.  Which is important if you think of child table as
 meaning subclass in the OO sense.  But for ordinary partitioning
 cases it's just useless complexity, and ModifyTable isn't the only
 thing that suffers from that useless complexity.

 If we had a notion of partitioned table that involved a restriction
 that all the child tables have the exact same rowtype, we could implement
 UPDATE/DELETE in a much saner fashion --- just one plan tree, not one
 per child table --- and it would be possible to support UPDATE/DELETE
 ORDER BY LIMIT with no more work than for the single-table case.
 So that might shift the calculation as to whether we're willing to
 accept a partial implementation.

None of the use cases I have in mind would ever (have to) use this on
a parent table; in the worst case it might make sense to do it on the
child tables individually.  Personally, I think that just refusing to
operate on tables with children is a reasonable start.  I have no
interest in working on improving partitioning, but I don't think
pushing this feature back in the hopes that someone else will would
help anyone.

But definitely only my two cents on this issue.


♜


-- 
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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..

[Rukh Meski]

On Sun, May 11, 2014 at 10:33 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On 11 May 2014 07:37, Amit Kapila amit.kapil...@gmail.com wrote:

 Tom Lane has explained these problems in a very clear manner
 in his below mail and shared his opinion about this feature as
 well.
 http://www.postgresql.org/message-id/26819.1291133...@sss.pgh.pa.us

 I don't have Tom's wonderfully articulate way of saying things, so
 I'll say it my way:

 If you want to do this, you already can already write a query that has
 the same effect. But supporting the syntax directly to execute a
 statement with an undefinable outcome is a pretty bad idea, and worse
 than that, there's a ton of useful things that we *do* want that would
 be a much higher priority for work than this. If you support Postgres,
 prioritise, please.

Yes, you can already achieve what this feature can do by other means,
but this feature makes these cases 1) more efficient in terms of how
much work has to be done 2) simpler and more elegant to write.  But
frankly I find it a bit insulting that I shouldn't work on this based
on other people's priorities.  This is a high priority item for me.

I'll look at the problem reported by Amit and come back with a
solution and my rationale for adding this feature.


♜


-- 
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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..

[Rukh Meski]

Hi,

Here's an updated patch.  I had to push the LIMIT processing into ModifyTable 
to make the behaviour sane in parallel scenarios.  As usual, please ignore if 
you're busy with 9.4.  I will work on better docs and more tests from now on 
and am preparing to make a solid case for adding this.



♜ 

update_delete_order_by_limit_v1.diff
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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..

[Rukh Meski]

Oops.  Of course shouldn't try and change how INSERT works.  Latest version 
attached.



♜

update_delete_order_by_limit_v2.diff
Description: Binary data

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

[HACKERS] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..

[Rukh Meski]

Hello hackers,

I know you're busy wrapping up the 9.4 release, so please ignore this patch.



♜

update_delete_order_by_limit_v0.diff
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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..

[Rukh Meski]

On Saturday, February 22, 2014 11:57:06 PM, Peter Geoghegan p...@heroku.com 
wrote:

I think you should describe what the patch does, why you believe the
feature is necessary, and perhaps how it compares to other, similar
things. You have documentation changes here, but that doesn't really
tell us why this is important.

Sorry, I wanted to minimize the attention my message attracts.  I mostly posted 
it to let people know I plan on working on this for 9.5 to avoid duplicated 
effort.  I will post more documentation and my reasons for wanting this feature 
in postgre later, if that's all right.



♜



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