| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com> |
| Cc: | Peter Eisentraut <peter(at)eisentraut(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: SQL:2011 Application Time Update & Delete |
| Date: | 2025-11-12 09:31:53 |
| Message-ID: | 7A20FF56-1A90-4221-A95E-5A605E36410A@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Nov 5, 2025, at 03:12, Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com> wrote:
>
> On Wed, Oct 29, 2025 at 11:02 PM Paul A Jungwirth
> <pj(at)illuminatedcomputing(dot)com> wrote:
>>
>> On Tue, Oct 28, 2025 at 3:49 AM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>>> On 24.10.25 19:08, Paul A Jungwirth wrote:
>>>> The first 3 doc patches all apply to features that we released in v18,
>>>> so it would be nice to get those reviewed/merged soon if possible.
>>>
>>> I have looked through the documentation patches 0001 through 0003.
>>
>> Thanks for taking a look! New patches attached; details below.
>
> Hi Hackers,
>
> Here is another set of patches. I added isolation tests for FOR
> PORTION OF. In REPEATABLE READ and SERIALIZABLE you get
> easy-to-predict results. In READ COMMITTED you get a lot of lost
> updates/deletes, because the second operation doesn't see the
> leftovers created by the first (and sometimes the first operation
> changes the start/end times in a way that EvalPlanQual no longer sees
> the being-changed row either). I think those results make sense, if
> you think step-by-step what Postgres is doing, but they are not really
> what a user wants.
>
> I tested the same sequences in MariaDB, and they also gave nonsense
> results, although not always the same nonsense as Postgres. At
> UNCOMMITTED READ it actually gave the results you'd want, but at that
> level I assume you will have other problems.
>
> I also tested DB2. It doesn't have READ COMMITTED, but I think READ
> STABILITY is the closest. At that level (as well as CURSOR STABILITY
> and REPEATABLE READ), you get correct results.
>
> Back to Postgres, you can get "desired" results IN READ COMMITTED by
> explicitly locking rows (with SELECT FOR UPDATE) just before
> updating/deleting them. Since you acquire the lock before the
> update/delete starts, there can be no new leftovers created within
> that span of history, and the update/delete sees everything that is
> there. The same approach also gives correct results in MariaDB. I
> think it is just the way you have to do things with temporal tables in
> READ COMMITTED whenever you expect concurrent updates to the same
> history.
>
> I considered whether we should make EvalPlanQual (or something else)
> automatically rescan for leftovers when it's a temporal operation.
> Then you wouldn't have to explicitly lock anything. But it seems like
> that is more than the isolation level "contract", and maybe even plain
> violates it (but arguably not, if you say the update shouldn't *start*
> until the other session commits). But since there is a workaround, and
> since other RDBMSes also scramble temporal data in READ COMMITTED, and
> since it is a lot of work and seems tricky, I didn't attempt it.
>
> Another idea (or maybe nearly the same thing) would be to
> automatically do the same thing that SELECT FOR UPDATE is doing,
> whenever we see a FOR PORTION OF DML command---i.e. scan for rows and
> lock them first, then do the update. But that has similar issues. If
> it adds locks the user doesn't expect, is it really the right thing?
> And it means users pay the cost even when no concurrency is expected.
> It offers strictly fewer options than requiring users to do SELECT FOR
> UPDATE explicitly.
>
> The isolation tests are a separate patch for now, because they felt
> like a significant chunk, and I wanted to emphasize them, but really
> they should be part of the main FOR PORTION OF commit. Probably I'll
> squash them in future submissions. That patch also makes some small
> updates to a comment in ExecForPortionOf and the docs for
> UPDATE/DELETE FOR PORTION OF, to raise awareness of the READ COMMITTED
> issues.
>
> Rebased to 65f4976189.
>
> Yours,
>
> --
> Paul ~{:-)
> pj(at)illuminatedcomputing(dot)com
> <v59-0003-Document-temporal-update-delete.patch><v59-0005-Add-UPDATE-DELETE-FOR-PORTION-OF.patch><v59-0001-Add-docs-section-for-temporal-tables-with-primar.patch><v59-0004-Add-range_minus_multi-and-multirange_minus_multi.patch><v59-0007-Add-tg_temporal-to-TriggerData.patch><v59-0002-Document-temporal-foreign-keys.patch><v59-0008-Look-up-more-temporal-foreign-key-helper-procs.patch><v59-0009-Add-CASCADE-SET-NULL-SET-DEFAULT-for-temporal-fo.patch><v59-0006-Add-isolation-tests-for-UPDATE-DELETE-FOR-PORTIO.patch><v59-0011-Add-PERIODs.patch><v59-0010-Expose-FOR-PORTION-OF-to-plpgsql-triggers.patch>
I tried to review this patch. Though I “git reset” to commit 65f4976189, “git am” still failed at 0009.
Today I only reviewed 0001, it was a happy reading. I found a small typo and got a suggestion:
1 - 0001
```
+ entity described by a table. In a typical non-temporal table, there is
+ single row for each entity. In a temporal table, an entity may have
```
“There is single row” should be “there is a single row”.
2 - 0001 - The doc mentions rangetypes which is the key factor for defining a temporal table, can we add a hyper link on “rangetype” so that readers can easily jump to learn which rangetypes can be used.
I will continue to review the rest of commits tomorrow.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Yugo Nagata | 2025-11-12 09:33:46 | Re: Suggestion to add --continue-client-on-abort option to pgbench |
| Previous Message | Xuneng Zhou | 2025-11-12 09:14:39 | Re: Fix incorrect assignment of InvalidXLogRecPtr to a non-LSN variable. |