Re: setLastTid() and currtid()

Hi Andres,
Sorry for the late reply.

On 2019/03/26 9:44, Andres Freund wrote:
> Hi,
>
> For the tableam work I'd like to remove heapam.h from
> nodeModifyTable.c. The only remaining impediment to that is a call to
> setLastTid(), which is defined in tid.c but declared in heapam.h.
>
> That doesn't seem like a particularly accurate location, it doesn't
> really have that much to do with heap. It seems more like a general
> executor facility or something. Does anybody have a good idea where to
> put the declaration?
>
>
> Looking at how this function is used, lead to some confusion on my part.
>
>
> We currently call setLastTid in ExecInsert():
>
> if (canSetTag)
> {
> (estate->es_processed)++;
> setLastTid(&slot->tts_tid);
> }
>
> And Current_last_tid, the variable setLastTid sets, is only used in
> currtid_byreloid():
>
>
> Datum
> currtid_byreloid(PG_FUNCTION_ARGS)
> {
> Oid reloid = PG_GETARG_OID(0);
> ItemPointer tid = PG_GETARG_ITEMPOINTER(1);
> ItemPointer result;
> Relation rel;
> AclResult aclresult;
> Snapshot snapshot;
>
> result = (ItemPointer) palloc(sizeof(ItemPointerData));
> if (!reloid)
> {
> *result = Current_last_tid;
> PG_RETURN_ITEMPOINTER(result);
> }
>
> I've got to say I'm a bit baffled by this interface. If somebody passes
> in a 0 reloid, we just ignore the passed in tid, and return the last tid
> inserted into any table?
>
> I then was even more baffled to find that there's no documentation of
> this function, nor this special case behaviour, to be found
> anywhere. Not in the docs (which don't mention the function, nor it's
> special case behaviour for relation 0), nor in the code.
>
>
> It's unfortunately used in psqlobdc:
>
> else if ((flag & USE_INSERTED_TID) != 0)
> printfPQExpBuffer(&selstr, "%s where ctid = (select currtid(0, '(0,0)'))", load_stmt);

The above code remains only for PG servers whose version < 8.2.
Please remove the code around setLastTid().

regards,
Hiroshi Inoue

> I gotta say, all that currtid code looks to me like it just should be
> ripped out. It really doesn't make a ton of sense to just walk the tid
> chain for a random tid - without an open snapshot, there's absolutely no
> guarantee that you get back anything meaningful. Nor am I convinced
> it's perfectly alright to just return the latest inserted tid for a
> relation the user might not have any permission for.
>
> OTOH, we probably can't just break psqlodbc, so we probably have to hold
> our noses a bit longer and just move the prototype elsewhere? But I'm
> inclined to just say that this functionality is going to get ripped out
> soon, unless somebody from the odbc community works on making it make a
> bit more sense (tests, docs at the very very least).
>
> Greetings,
>
> Andres Freund

---
このメールは、AVG によってウイルス チェックされています。
http://www.avg.com