Horton

Re: Introduce new multi insert Table AM and improve performance of various SQL commands with it for Heap AM

Jeff Davis <pgsql@j-davis.com>

View thread
From: Jeff Davis <pgsql@j-davis.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>, Dilip Kumar <dilipbalaut@gmail.com>, Luc Vlaming <luc@swarm64.com>, Justin Pryzby <pryzby@telsasoft.com>, Michael Paquier <michael@paquier.xyz>, Matthias van de Meent <boekewurm+postgres@gmail.com>, Alexander Korotkov <aekorotkov@gmail.com>
Date: 2024-08-29T19:29:43Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. libpq: Fix some issues in TAP tests for service files

  2. Multiple revisions to the GROUP BY reordering tests

  3. Explore alternative orderings of group-by pathkeys during optimization.

On Thu, 2024-08-29 at 12:55 +0530, Bharath Rupireddy wrote:
> IMO, every caller branching out in the code like if (rel->rd_tableam-
> >
> tuple_modify_buffer_insert != NULL) then multi insert; else single
> insert; doesn't look good. IMO, the default implementation approach
> keeps things simple which eventually can be removed in *near* future.
> Thoughts?

I believe we need the branching in the caller anyway:

1. If there is a BEFORE row trigger with a volatile function, the
visibility rules[1] mean that the function should see changes from all
the rows inserted so far this command, which won't work if they are
still in the buffer.

2. Similarly, for an INSTEAD OF row trigger, the visibility rules say
that the function should see all previous rows inserted.

3. If there are volatile functions in the target list or WHERE clause,
the same visibility semantics apply.

4. If there's a "RETURNING ctid" clause, we need to either come up with
a way to return the tuples after flushing, or we need to use the
single-tuple path. (Similarly in the future when we support UPDATE ...
RETURNING, as Matthias pointed out.)

If we need two paths in each caller anyway, it seems cleaner to just
wrap the check for tuple_modify_buffer_insert in
table_modify_buffer_enabled().

We could perhaps use a one path and then force a batch size of one or
something, which is an alternative, but we have to be careful not to
introduce a regression (and it still requires a solution for #4).

Regards,
	Jeff Davis

[1] 
https://www.postgresql.org/docs/devel/trigger-datachanges.html