Re: WIP: System Versioned Temporal Table

On Mon, Jan 11, 2021 at 7:02 AM Simon Riggs <simon(dot)riggs(at)enterprisedb(dot)com>
wrote:

> On Sat, Jan 9, 2021 at 10:39 AM Simon Riggs
> <simon(dot)riggs(at)enterprisedb(dot)com> wrote:
> >
> > On Fri, Jan 8, 2021 at 9:19 PM Ryan Lambert <ryan(at)rustprooflabs(dot)com>
> wrote:
> >
> > >> Updated v11 with additional docs and some rewording of messages/tests
> > >> to use "system versioning" correctly.
> > >>
> > >> No changes on the points previously raised.
> > >>
> > > Thank you! The v11 applies and installs. I tried a simple test,
> unfortunately it appears the versioning is not working. The initial value
> is not preserved through an update and a new row does not appear to be
> created.
> >
> > Agreed. I already noted this in my earlier review comments.
>
> I'm pleased to note that UPDATE-not-working was a glitch, possibly in
> an earlier patch merge. That now works as advertised.
>

It is working as expected now, Thank you!

> I've added fairly clear SGML docs to explain how the current patch
> works, which should assist wider review.
>

The default column names changed to start_timestamp and end_timestamp. A
number of places in the docs still refer to StartTime and EndTime. I
prefer the new names without MixedCase.

>
> Also moved test SQL around a bit, renamed some things in code for
> readability, but not done any structural changes.
>
> This is looking much better now... with the TODO/issues list now
> looking like this...
>
> * Anomalies around use of CURRENT_TIMESTAMP are not discussed or resolved.
> Probably need to add a test that end_timestamp > start_timestamp or ERROR,
> which effectively enforces serializability.
>
> * No discussion, comments or tests around freezing and whether that
> causes issues here
>
> * What happens if you ask for a future time?
> It will give an inconsistent result as it scans, so we should refuse a
> query for time > current_timestamp.

* ALTER TABLE needs some work, it's a bit klugey at the moment and
> needs extra tests.
> Should refuse DROP COLUMN on a system time column, but currently doesn't
>
> * UPDATE foo SET start_timestamp = DEFAULT should fail but currently
> doesn't
>
> * Do StartTime and EndTime show in SELECT *? Currently, yes. Would
> guess we wouldn't want them to, not sure what standard says.
>
> From here, the plan would be to set this to "Ready For Committer" in
> about a week. That is not the same thing as me saying it is
> ready-for-commit, but we need some more eyes on this patch to decide
> if it is something we want and, if so, are the code changes cool.
>
>
Should I invest time now into further testing with more production-like
scenarios on this patch? Or would it be better to wait on putting effort
into that until it has had more review? I don't have much to offer for
help on your current todo list.

Thanks,

Ryan Lambert