Date: Mon, 21 Dec 2020 01:47:25 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>,
	Andres Freund <andres@anarazel.de>, Luc Vlaming <luc@swarm64.com>,
	Paul Guo <guopa@vmware.com>
Subject: Re: New Table Access Methods for Multi and Single Inserts
Message-ID: <20201221074725.GF30237@telsasoft.com>
References: 
 <CALj2ACVi9eTRYR=gdca5wxtj3Kk_9q9qVccxsS1hngTGOCjPwQ@mail.gmail.com>
 <20201217050522.GU30237@telsasoft.com>
 <CALj2ACVgT1iocd5nQ+rEmqt3xcCONkR037qbc8PiojdR39Ag=w@mail.gmail.com>
 <20201217204442.GX30237@telsasoft.com>
 <CALj2ACW3BC5kgdffZ2LD_CT2wQoXVc29kGB74SVWnGZ=UFqcAQ@mail.gmail.com>
 <20201218175439.GA30237@telsasoft.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201218175439.GA30237@telsasoft.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Fri, Dec 18, 2020 at 11:54:39AM -0600, Justin Pryzby wrote:
> On Fri, Dec 18, 2020 at 07:39:14AM +0530, Bharath Rupireddy wrote:
> > On Fri, Dec 18, 2020 at 2:14 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > > Are you thinking that TableInsertState would eventually have additional
> > > attributes which would apply to other tableams, but not to heap ?  Is
> > > heap_insert_begin() really specific to heap ?  It's allocating and populating a
> > > structure based on its arguments, but those same arguments would be passed to
> > > every other AM's insert_begin routine, too.  Do you need a more flexible data
> > > structure, something that would also accomodate extensions?  I'm thinking of
> > > reloptions as a loose analogy.
> > 
> > I could not think of other tableam attributes now. But +1 to have that
> > kind of flexible structure for TableInsertState. So, it can have
> > tableam type and attributes within the union for each type.
> 
> Right now you have heap_insert_begin(), and I asked if it was really
> heap-specific.  Right now, it populates a struct based on a static list of
> arguments, which are what heap uses.  
> 
> If you were to implement a burp_insert_begin(), how would it differ from
> heap's?  With the current API, they'd (have to) be the same, which means either
> that it should apply to all AMs (or have a "default" implementation that can be
> overridden by an AM), or that this API assumes that other AMs will want to do
> exactly what heap does, and fails to allow other AMs to implement optimizations
> for bulk inserts as claimed.
> 
> I don't think using a "union" solves the problem, since it can only accommodate
> core AMs, and not extensions, so I suggested something like reloptions, which
> have a "namespace" prefix (and core has toast.*, like ALTER TABLE t SET
> toast.autovacuum_enabled).

I think you'd want to handle things like:

 - a compressed AM wants to specify a threshold for a tuple's *compressed* size
   (maybe in addition to the uncompressed size);
 - a "columnar" AM wants to specify a threshold size for a column, rather
   than for each tuple;

I'm not proposing to handle those specific parameters, but rather pointing out
that your implementation doesn't allow handling AM-specific considerations,
which I think was the goal.

The TableInsertState structure would need to store those, and then the AM's
multi_insert_v2 routine would need to make use of them.

It feels a bit like we'd introduce the idea of an "AM option", except that it
wouldn't be user-facing (or maybe some of them would be?).  Maybe I've
misunderstood though, so other opinions are welcome.

-- 
Justin