Обсуждение: Removal of unnecessary CommandCounterIncrement() when doing ONCOMMIT DELETE ROWS

Hi all,

When doing a set of ON COMMIT DELETE ROWS actions for relations, there
is a CCI happening after each truncation:
@@ -13334,10 +13334,8 @@ PreCommit_on_commit_actions(void)
    * exists at truncation time.
    */
    if (oids_to_truncate != NIL)
-   {
        heap_truncate(oids_to_truncate);
-       CommandCounterIncrement();  /* XXX needed? */
-   }

This has been visibly introduced by f9b5b41 back in 2002 which reworked
how ON COMMIT works.

Alvaro has mentioned that it would not be needed here:
20181106125337.o23kjdv546bu2tei@alvherre.pgsql
And I think that I agree with that, because visibly this applies to
index rebuilds but in those cases CCIs are happening locally.  So I
think that we can get rid of that, and I suggest to remove it only on
HEAD only of course.

Any objections?
--
Michael

On 2018-Nov-09, Michael Paquier wrote:

> Hi all,
> 
> When doing a set of ON COMMIT DELETE ROWS actions for relations, there
> is a CCI happening after each truncation:

Right, but note that this is not a loop so it's only one CCI, not one
per relation.  (This doesn't seem a big deal either way, but I see no
reason to have that line there.)

> And I think that I agree with that, because visibly this applies to
> index rebuilds but in those cases CCIs are happening locally.  So I
> think that we can get rid of that, and I suggest to remove it only on
> HEAD only of course.

+1

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

On Mon, Nov 12, 2018 at 01:19:31PM -0300, Alvaro Herrera wrote:
> On 2018-Nov-09, Michael Paquier wrote:
>> When doing a set of ON COMMIT DELETE ROWS actions for relations, there
>> is a CCI happening after each truncation:
>
> Right, but note that this is not a loop so it's only one CCI, not one
> per relation.  (This doesn't seem a big deal either way, but I see no
> reason to have that line there.)

This looks like a lack of caffeine from my side.

>> And I think that I agree with that, because visibly this applies to
>> index rebuilds but in those cases CCIs are happening locally.  So I
>> think that we can get rid of that, and I suggest to remove it only on
>> HEAD only of course.
>
> +1

Thanks Álvaro for the input, I have committed the change.
--
Michael