On Jan 27, 2021, at 12:39 AM, Surafel Temesgen 
<surafel3...@gmail.com<mailto:surafel3...@gmail.com>> wrote:



On Tue, Jan 26, 2021 at 2:33 PM Vik Fearing 
<v...@postgresfriends.org<mailto:v...@postgresfriends.org>> wrote:
I'm still in the weeds of reviewing this patch, but why should this
fail?  It should not fail.

Attached is rebased patch that include isolation test


Thanks for updating the patch.  However it cannot apply to master (e5d8a9990).

Here are some comments on system-versioning-temporal-table_2021_v13.patch.

+</programlisting>
+    When system versioning is specified two columns are added which
+    record the start timestamp and end timestamp of each row verson.

verson -> version

+    By default, the column names will be StartTime and EndTime, though
+    you can specify different names if you choose.

In fact, it is start_time and end_time, not StartTime and EndTime.
I think it's better to use <literal> label around start_time and end_time.

+    column will be automatically added to the Primary Key of the
+    table.

Should we mention the unique constraints?

+    The system versioning period end column will be added to the
+    Primary Key of the table as a way of ensuring that concurrent
+    INSERTs conflict correctly.

Same as above.

Since the get_row_start_time_col_name() and get_row_end_time_col_name()
are similar, IMO we can pass a flag to get StartTime/EndTime column name,
thought?

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

Reply via email to