Hi, This doesn't pass tests because of lack of some file. Can we fix that please and send the patch again?
On Tue, Aug 10, 2021 at 7:20 AM Simon Riggs <simon.ri...@enterprisedb.com> wrote: > On Wed, 14 Jul 2021 at 12:48, vignesh C <vignes...@gmail.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/ > --
