Re: Removal of currtid()/currtid2() and some table AM cleanup

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: "Inoue, Hiroshi" <h-inoue(at)dream(dot)email(dot)ne(dot)jp>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Hiroshi Saito <hiroshi(at)winpg(dot)jp>, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>
Subject: Re: Removal of currtid()/currtid2() and some table AM cleanup
Date: 2020-06-26 04:11:55
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Jun 25, 2020 at 10:14:00PM +0900, Inoue, Hiroshi wrote:
> I seem to have invalidated KEYSET-DRIVEN cursors used in positioned-update
> test. It was introduced by the commit 4a272fd but was invalidated by the
> commit 2be35a6.
> I don't object to the removal of currtid(2) because keyset-driven cursors in
> psqlodbc are changed into static cursors in many cases and I've hardly ever
> heard a complaint about it.

Hmm. I am not sure that this completely answers my original concern
though. In short, don't we still have corner cases where
keyset-driven cursors are not changed into static cursors, meaning
that currtid2() could get used? The removal of the in-core functions
would hurt applications using that, meaning that we should at least
provide an equivalent of currtid2() in the worse case as a contrib
module, no? If the code paths of currtid2() are reachable, shouldn't
we also make sure that they are still reached in the regression tests
of the driver, meaning that the driver code needs more coverage? I
have been looking at the tests and tried to tweak them using
SQLSetPos() so as the code paths involving currtid2() get reached, but
I am not really able to do so. It does not mean that that currtid2()
never gets reached, it just means that I am not able to be sure that
this part can be safely removed from the Postgres backend code :(

From what I can see on this thread, we could just remove currtid() per
the arguments of the RETURNING ctid clause supported since PG 8.2, but
it would make more sense to me to just remove both currtid/currtid2()
at once.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amul Sul 2020-06-26 04:41:41 Re: [Patch] ALTER SYSTEM READ ONLY
Previous Message Bruce Momjian 2020-06-26 04:02:10 Re: Default setting for enable_hashagg_disk