Re: SQL:2011 Application Time Update & Delete

Updated patches attached.

On Wed, Mar 25, 2026 at 9:05 AM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> > 4. The SQL standard doesn't have RETURNING. But it does say that to
> > insert the leftovers the system should execute a separate insert
> > "statement". So we should do something very similar to the trigger
> > case.
>
> UPDATE ... RETURNING is in my mind equivalent to the SQL standard SELECT
> ... FROM NEW TABLE (UPDATE ...) (see <data change delta table>). So I
> was hoping for an answer there, but it just says:
>
> "If TP simply contains a <data change delta table> DCDT, then let S be
> the <data change statement> simply contained in TP. S shall not contain
> FOR PORTION OF."
>
> So we can pick our own behavior. Your explanation makes sense. (I
> suppose an alternative is that we also don't allow using FOR PORTION OF
> together with RETURNING?)

Okay, thanks! I think it's worth supporting RETURNING, and this
behavior seems like the most useful.

> > Going back to the UNBOUNDED keyword: if we forbid nulls, then a
> > keyword doesn't really add clarity, since users would already say
> > `-Infinity` or `Infinity`. It's really just a way to express what null
> > means in this context. Assuming we keep nulls, I'd like to keep
> > working on a keyword. But I think we could add it later.
>
> Yeah, this seems like something we could change later with relative
> ease. Maybe solicit some input from the public during beta?

That sounds good to me. Also if we did want to forbid nulls here, we
could choose to do it when valid_at is a PERIOD (once that lands) but
permit them when it's a range column. That seems like it best matches
the expected behavior in both scenarios. In fact, if the PERIOD's
start/end columns are NOT NULL, then we are already enforcing the rule
indirectly.

> > Btw what do you think of the READ COMMITTED issues I brought up in my
> > third patch? We follow MariaDB here, but not DB2. DB2's behavior is
> > less problematic for users, although their isolation levels don't
> > quite match ours. If we're not okay with those results, we should
> > address them before merging the main patch.
>
> It's still hard to understand. I would be ok in general to say that
> results might be unexpected unless you use REPEATABLE READ. Especially
> as it seems that a technical solution to improve this would be possible
> later. But we should document this in more detail. The verbal
> explanations are hard to interpret. Could you maybe come up with a
> couple of ASCII-art flow charts that explains how things could go
> strange that we could put into the documentation?

I made a sequence diagram and rewrote that section of the docs. I
think it's much clearer now.

> Could we actually put some of these strange/unexpected behaviors into
> the isolation tests? Right now we only test that the workaround works
> but not the initial problem. Is this possible? (Would we need
> injection points?)

That's a good idea; I should have done it before. Done. No injection
points needed.

> Could we cut back the isolation tests a bit? They are the second slowest
> isolation test now, and the second largest expected file. Maybe we
> don't need to test SERIALIZE separately? (Assume that SERIALIZE is as
> good as or better than REPEATABLE READ?)

I removed all but a couple of the SERIALIZABLE tests.

> Attached is a patch with a few small cosmetic corrections. In
> ExecForPortionOfLeftovers(), the comment and code that I delete is
> duplicated before and inside the loop. The one before the
> loop is probably sufficient.

Applied.

> Other than all that, this patch set (0001 through 0003) seems good to me.

Thanks! These v70 patches are rebased onto f39cb8c011.

Yours,

--
Paul ~{:-)
pj(at)illuminatedcomputing(dot)com