hettie-d / pg_bitemporal

Bitemporal tables in Postgres
BSD 3-Clause "New" or "Revised" License
44 stars 10 forks source link

bitemporal_internal.ll_bitemporal_correction_select fails because asserted time uses now() as the start of the interval #6

Open joelsvictor opened 2 years ago

joelsvictor commented 2 years ago

I was referring to this https://github.com/hettie-d/pg_bitemporal/blob/master/generate_bitemporal/generate_bt_update_trigger_template.sql#L39 to implement a similar trigger and was trying to use bitemporal_internal.ll_bitemporal_correction_select instead of bitemporal_internal.ll_bitemporal_correction. I am inside a transaction and doing two updates, the second update triggers a correction. When using the bitemporal_internal.ll_bitemporal_correction_select, the operation fails because of the asserted interval of both the correct and incorrect record overlap.

I looked at the implementation at ll_bitemporal_correction and found that upper(asserted) (https://github.com/hettie-d/pg_bitemporal/blob/master/sql/ll_bitemporal_correction.sql#L51) is being used instead of now() (https://github.com/hettie-d/pg_bitemporal/blob/master/sql/ll_bitemporal_correction_select.sql#L42)

Is this intentional, or is this a bug?

Additionally, the arguments to the ll_bitemporal_correction_select are not qualified by a schema which causes the function creation to fail if the search_path is not set https://github.com/hettie-d/pg_bitemporal/blob/master/sql/ll_bitemporal_correction_select.sql#L7

A small example to reproduce this problem.

select bitemporal_internal.ll_create_bitemporal_table(
    'common', 'person', $$id int not null, 
    name text, phone_number int$$, $$id$$
  );

select 
  bitemporal_internal.ll_bitemporal_insert(
    'common.person', 
    $$id, 
    name, 
    phone_number$$, 
    $$2, 
    'Joel Victor', 
    0123456789$$, 
    temporal_relationships.timeperiod(now(), 'infinity'), 
    temporal_relationships.timeperiod(now(), 'infinity')
  );

begin;
select 
  bitemporal_internal.ll_bitemporal_update(
    'common', 'person', 
    $$name,phone_number$$, 
    $$'Joel S. Victor',0123456789$$, 
    $$id$$,$$ 2$$,
    temporal_relationships.timeperiod(now(), 'infinity'), 
    temporal_relationships.timeperiod(now(), 'infinity')
  );

 -- this fails
 select 
  bitemporal_internal.ll_bitemporal_correction_select (
    'common.person'::text, 
    $$name,phone_number$$::text, 
    $$'Joel S V',0123456789$$::text, 
    $$id = '2'$$::text,
    now(), 
    now()
  );
 -- this also fails
 select 
  bitemporal_internal.ll_bitemporal_correction_select (
    'common.person'::text, 
    $$name,phone_number$$::text, 
    $$'Joel S V',0123456789$$::text, 
    $$id = '2'$$::text,
    now(), 
    clock_timestamp()
  );

 -- this works
 select 
  bitemporal_internal.ll_bitemporal_correction (
    'common'::text,'person'::text, 
    $$name,phone_number$$::text, 
    $$'Joel S V',0123456789$$::text, 
    $$id$$::text,
    $$2$$::text,
    temporal_relationships.timeperiod(now(), 'infinity'), 
    now()
  );
abort;
SQL Error [23P01]: ERROR: conflicting key value violates exclusion constraint "person_id_assert_eff_excl"
  Detail: Key (id, asserted, effective)=(2, ["2022-08-23 16:22:25.25041+05:30",infinity), ["2022-08-23 16:22:23.056263+05:30",infinity)) conflicts with existing key (id, asserted, effective)=(2, ["2022-08-23 16:22:25.25041+05:30",infinity), ["2022-08-23 16:22:23.056263+05:30","2022-08-23 16:22:25.25041+05:30")).
  Where: SQL statement "INSERT INTO common.person ( id,name,phone_number, effective, asserted )
                SELECT id,name,phone_number ,effective, temporal_relationships.timeperiod_range(now(),
                 'infinity', '[)')
                  FROM common.person WHERE  id = '2'  AND '2022-08-23 16:22:25.25041+05:30'::timestamptz <@ effective
                          AND upper(asserted)= '2022-08-23 16:22:25.25041+05:30' 
                                 "
PL/pgSQL function bitemporal_internal.ll_bitemporal_correction_select(text,text,text,text,time_endpoint,time_endpoint) line 31 at EXECUTE
hettie-d commented 2 years ago

Hi @joelsvictor, thank you for reporting a problem. The transactional behavior is a bit tricky, and we had a lot of internal discussions before implementing it that way. I will look closely at your example to make sure there are no other previously unknown bugs and will get back to you with a more detailed explanation.

hettie-d commented 2 years ago

Hi @joelsvictor, it looks like this problem was fixed in bitemporal_correction and not fixed in the bitemporal_correct_select. As you could see for yourself, bitemporal_corrrection worked, and the bitemporal_correct_select errored. In fact, https://github.com/hettie-d/pg_bitemporal/blob/master/sql/ll_bitemporal_correction.sql#L51 is right, while https://github.com/hettie-d/pg_bitemporal/blob/master/sql/ll_bitemporal_correction_select.sql#L43 is wrong.

In short, our goal was to have consistent transactional behavior, which means that what is happening inside a transaction is not visible outside the transaction, so we do not want to show a history of updates on the same record if it happened in one transaction. That's why in this case the update will be overwritten with correction.

I will work on this fix on Friday, and if you could also share the DDL of the table you used, it would be great; then I can reproduce the results.

Thank you!

joelsvictor commented 2 years ago

Hey @hettie-d, Thanks for the explanation.

Here is the DDL for bitemporal table in the example:

select bitemporal_internal.ll_create_bitemporal_table(
    'common', 'person', $$id int not null, 
    name text, phone_number int$$, $$id$$
  );
hettie-d commented 2 years ago

Hi @joelsvictor , I took a close look at this bug, and the problem is more than using lower(asserted on a place of now()). When we fixed the bug which manifested itself when you perform several bitemporal operations on one logical record within one transaction, we introduced a conditional UPDATE - see https://github.com/hettie-d/pg_bitemporal/blob/master/sql/ll_bitemporal_correction.sql#L67 Applying the same fix to correction_select is not automatic, because correction_select allows to correct a record specified by "some timepoint within effective," so I will need a little bit more time to fix it.

In general, however, applying correction over update within one transaction does not make much sense because the update is immediately overwritten by a subsequent correction. I will most definitely fix it because the operation should not fail, but it might take me some time. I will let you know when the fix will be in place. Thank you!

joelsvictor commented 2 years ago

Hey @hettie-d, thanks for the update!