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