Re: WIP: System Versioned Temporal Table

On Wed, 14 Jul 2021 at 12:48, vignesh C <vignesh21(at)gmail(dot)com> wrote:

> The patch does not apply on Head anymore, could you rebase and post a
> patch. I'm changing the status to "Waiting for Author".

OK, so I've rebased the patch against current master to take it to v15.

I've then worked on the patch some myself to make v16 (attached),
adding these things:

* Add code, docs and test to remove the potential anomaly where
endtime < starttime, using the sqlstate 2201H as pointed out by Vik
* Add code and tests to handle multiple changes in a transaction
correctly, according to SQL Std
* Add code and tests to make Foreign Keys behave correctly, according to SQL Std
* Fixed nascent bug in relcache setup code
* Various small fixes from Japin's review - thanks! I've used
starttime and endtime as default column names
* Additional tests and docs to show that the functionality works with
or without PKs on the table

I am now satisfied that the patch does not have any implementation
anomalies in behavioral design, but it is still a long way short in
code architecture.

There are various aspects still needing work. This is not yet ready
for Commit, but it is appropriate now to ask for initial design
guidance on architecture and code placement by a Committer, so I am
setting this to Ready For Committer, in the hope that we get the
review in SeptCF and a later version can be submitted for later commit
in JanCF. With the right input, this patch is about a person-month
away from being ready, assuming we don't hit any blocking issues.

Major Known Issues
* SQLStd says that it should not be possible to update historical
rows, but those tests show we fail to prevent that and there is code
marked NOT_USED in those areas
* The code is structured poorly around
parse-analyze/rewriter/optimizer/executor and that needs positive
design recommendations, rather than critical review
* Joins currently fail because of the botched way WHERE clauses are
added, resulting in duplicate names
* Views probably don't work, but there are no tests
* CREATE TABLE (LIKE foo) doesn't correctly copy across all features -
test for that added, with test failure accepted for now
* ALTER TABLE is still incomplete and also broken; I suggest we remove
that for the first version of the patch to reduce patch size for an
initial commit.

Minor Known Issues
* Logical replication needs some minor work, no tests yet
* pg_dump support looks like it exists and might work easily, but
there are no tests yet
* Correlation names don't work in FROM clause - shift/reduce errors
from double use of AS
* Add test and code to prevent triggers referencing period cols in the
WHEN clause
* No tests yet to prove you can't set various parameters/settings on
the period time start/end cols
* Code needs some cleanup in a few places
* Not really sure what value is added by
lock-update-delete-system-versioned.spec

* IMHO we should make the PK definition use "endtime DESC", so that
the current version is always the first row found in the PK for any
key, since historical indexes will grow bigger over time

There are no expected issues with integration with MERGE, since SQLStd
explains how to handle that.

Other reviews are welcome.

--
Simon Riggs http://www.EnterpriseDB.com/