Skip to content

Commit 62182ac

Browse files
jhfjhf
committed
Fix bug in add_era validation logic
1 parent a52c14c commit 62182ac

File tree

5 files changed

+215
-12
lines changed

5 files changed

+215
-12
lines changed

‎expected/13_issues.out‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,139 @@ SELECT sql_saga.drop_era('uk', 'p');
201201
(1 row)
202202

203203
DROP TABLE uk;
204+
-- Test for issue with duplicated column in UPDATE OF list for FK trigger
205+
CREATE TABLE t_uk (
206+
id int,
207+
valid_after date,
208+
valid_to date
209+
);
210+
SELECT sql_saga.add_era('t_uk', 'valid_after', 'valid_to');
211+
add_era
212+
---------
213+
t
214+
(1 row)
215+
216+
SELECT sql_saga.add_unique_key('t_uk', ARRAY['id']);
217+
add_unique_key
218+
----------------
219+
t_uk_id_valid
220+
(1 row)
221+
222+
CREATE TABLE t_fk (
223+
id int,
224+
uk_id int,
225+
valid_after date,
226+
valid_to date
227+
);
228+
SELECT sql_saga.add_era('t_fk', 'valid_after', 'valid_to');
229+
add_era
230+
---------
231+
t
232+
(1 row)
233+
234+
-- This call failed in a downstream project because it generated:
235+
-- CREATE ... TRIGGER ... AFTER UPDATE OF uk_id, valid_after, valid_to, valid_to ON ...
236+
-- The bug is that 'valid_to' is added to the list twice if it's both the
237+
-- valid_until_column_name and a column named 'valid_to' exists.
238+
SAVEPOINT s_fk_error;
239+
SELECT sql_saga.add_foreign_key('t_fk', ARRAY['uk_id'], 'valid', 't_uk_id_valid');
240+
add_foreign_key
241+
------------------
242+
t_fk_uk_id_valid
243+
(1 row)
244+
245+
ROLLBACK TO SAVEPOINT s_fk_error;
246+
-- cleanup
247+
-- Since the transaction for creating the FK was rolled back, we can just
248+
-- clean up the tables and eras.
249+
SELECT sql_saga.drop_era('t_fk');
250+
drop_era
251+
----------
252+
t
253+
(1 row)
254+
255+
DROP TABLE t_fk;
256+
SELECT sql_saga.drop_unique_key('t_uk', ARRAY['id'], 'valid');
257+
drop_unique_key
258+
-----------------
259+
260+
(1 row)
261+
262+
SELECT sql_saga.drop_era('t_uk');
263+
drop_era
264+
----------
265+
t
266+
(1 row)
267+
268+
DROP TABLE t_uk;
269+
-- Test generic synchronized column for FK trigger
270+
CREATE TABLE t_uk_gen (
271+
id int,
272+
valid_from date,
273+
valid_until date,
274+
valid_end_date date -- synchronized column
275+
);
276+
SELECT sql_saga.add_era('t_uk_gen', p_synchronize_valid_to_column := 'valid_end_date');
277+
NOTICE: sql_saga: Created trigger "t_uk_gen_synchronize_temporal_columns_trigger" on table t_uk_gen to synchronize columns: valid_end_date
278+
add_era
279+
---------
280+
t
281+
(1 row)
282+
283+
SELECT sql_saga.add_unique_key('t_uk_gen', ARRAY['id']);
284+
add_unique_key
285+
-------------------
286+
t_uk_gen_id_valid
287+
(1 row)
288+
289+
CREATE TABLE t_fk_gen (
290+
id int,
291+
uk_id int,
292+
valid_from date,
293+
valid_until date,
294+
valid_end_date date -- synchronized column
295+
);
296+
SELECT sql_saga.add_era('t_fk_gen', p_synchronize_valid_to_column := 'valid_end_date');
297+
NOTICE: sql_saga: Created trigger "t_fk_gen_synchronize_temporal_columns_trigger" on table t_fk_gen to synchronize columns: valid_end_date
298+
add_era
299+
---------
300+
t
301+
(1 row)
302+
303+
-- This should succeed and create a trigger that watches valid_end_date
304+
SELECT sql_saga.add_foreign_key('t_fk_gen', ARRAY['uk_id'], 'valid', 't_uk_gen_id_valid');
305+
add_foreign_key
306+
----------------------
307+
t_fk_gen_uk_id_valid
308+
(1 row)
309+
310+
-- cleanup
311+
SELECT sql_saga.drop_foreign_key('t_fk_gen', ARRAY['uk_id'], 'valid');
312+
drop_foreign_key
313+
------------------
314+
315+
(1 row)
316+
317+
SELECT sql_saga.drop_era('t_fk_gen');
318+
drop_era
319+
----------
320+
t
321+
(1 row)
322+
323+
DROP TABLE t_fk_gen;
324+
SELECT sql_saga.drop_unique_key('t_uk_gen', ARRAY['id'], 'valid');
325+
drop_unique_key
326+
-----------------
327+
328+
(1 row)
329+
330+
SELECT sql_saga.drop_era('t_uk_gen');
331+
drop_era
332+
----------
333+
t
334+
(1 row)
335+
336+
DROP TABLE t_uk_gen;
204337
-- Test case for bug with infinite parent validity
205338
CREATE TABLE legal_unit_bug (
206339
id INT NOT NULL,

‎expected/23_create_temporal_foreign_key_test.out‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ SELECT sql_saga.add_foreign_key(
325325
unique_key_name => 'houses_id_valid'
326326
);
327327
psql:sql/support/shifts_houses_rooms_enable_saga.sql:18: ERROR: insert or update on table "rooms" violates foreign key constraint "rooms_house_id_valid"
328-
CONTEXT: PL/pgSQL function sql_saga.add_foreign_key(regclass,name[],name,name,sql_saga.fk_match_types,sql_saga.fk_actions,sql_saga.fk_actions,name,name,name,name,name) line 360 at RAISE
328+
CONTEXT: PL/pgSQL function sql_saga.add_foreign_key(regclass,name[],name,name,sql_saga.fk_match_types,sql_saga.fk_actions,sql_saga.fk_actions,name,name,name,name,name) line 362 at RAISE
329329
ROLLBACK TO SAVEPOINT s;
330330
-- it fails on a table with a completely-uncovered foreign key
331331
SAVEPOINT s;
@@ -378,7 +378,7 @@ SELECT sql_saga.add_foreign_key(
378378
unique_key_name => 'houses_id_valid'
379379
);
380380
psql:sql/support/shifts_houses_rooms_enable_saga.sql:18: ERROR: insert or update on table "rooms" violates foreign key constraint "rooms_house_id_valid"
381-
CONTEXT: PL/pgSQL function sql_saga.add_foreign_key(regclass,name[],name,name,sql_saga.fk_match_types,sql_saga.fk_actions,sql_saga.fk_actions,name,name,name,name,name) line 360 at RAISE
381+
CONTEXT: PL/pgSQL function sql_saga.add_foreign_key(regclass,name[],name,name,sql_saga.fk_match_types,sql_saga.fk_actions,sql_saga.fk_actions,name,name,name,name,name) line 362 at RAISE
382382
ROLLBACK TO SAVEPOINT s;
383383
-- it fails on a table with a partially-covered foreign key
384384
SAVEPOINT s;
@@ -431,7 +431,7 @@ SELECT sql_saga.add_foreign_key(
431431
unique_key_name => 'houses_id_valid'
432432
);
433433
psql:sql/support/shifts_houses_rooms_enable_saga.sql:18: ERROR: insert or update on table "rooms" violates foreign key constraint "rooms_house_id_valid"
434-
CONTEXT: PL/pgSQL function sql_saga.add_foreign_key(regclass,name[],name,name,sql_saga.fk_match_types,sql_saga.fk_actions,sql_saga.fk_actions,name,name,name,name,name) line 360 at RAISE
434+
CONTEXT: PL/pgSQL function sql_saga.add_foreign_key(regclass,name[],name,name,sql_saga.fk_match_types,sql_saga.fk_actions,sql_saga.fk_actions,name,name,name,name,name) line 362 at RAISE
435435
ROLLBACK TO SAVEPOINT s;
436436
ROLLBACK;
437437
\i sql/include/test_teardown.sql

‎sql/13_issues.sql‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,72 @@ SELECT sql_saga.drop_unique_key('uk', ARRAY['id'], 'p');
7979
SELECT sql_saga.drop_era('uk', 'p');
8080
DROP TABLE uk;
8181

82+
-- Test for issue with duplicated column in UPDATE OF list for FK trigger
83+
CREATE TABLE t_uk (
84+
id int,
85+
valid_after date,
86+
valid_to date
87+
);
88+
SELECT sql_saga.add_era('t_uk', 'valid_after', 'valid_to');
89+
SELECT sql_saga.add_unique_key('t_uk', ARRAY['id']);
90+
91+
CREATE TABLE t_fk (
92+
id int,
93+
uk_id int,
94+
valid_after date,
95+
valid_to date
96+
);
97+
SELECT sql_saga.add_era('t_fk', 'valid_after', 'valid_to');
98+
99+
-- This call failed in a downstream project because it generated:
100+
-- CREATE ... TRIGGER ... AFTER UPDATE OF uk_id, valid_after, valid_to, valid_to ON ...
101+
-- The bug is that 'valid_to' is added to the list twice if it's both the
102+
-- valid_until_column_name and a column named 'valid_to' exists.
103+
SAVEPOINT s_fk_error;
104+
SELECT sql_saga.add_foreign_key('t_fk', ARRAY['uk_id'], 'valid', 't_uk_id_valid');
105+
ROLLBACK TO SAVEPOINT s_fk_error;
106+
107+
-- cleanup
108+
-- Since the transaction for creating the FK was rolled back, we can just
109+
-- clean up the tables and eras.
110+
SELECT sql_saga.drop_era('t_fk');
111+
DROP TABLE t_fk;
112+
SELECT sql_saga.drop_unique_key('t_uk', ARRAY['id'], 'valid');
113+
SELECT sql_saga.drop_era('t_uk');
114+
DROP TABLE t_uk;
115+
116+
117+
-- Test generic synchronized column for FK trigger
118+
CREATE TABLE t_uk_gen (
119+
id int,
120+
valid_from date,
121+
valid_until date,
122+
valid_end_date date -- synchronized column
123+
);
124+
SELECT sql_saga.add_era('t_uk_gen', p_synchronize_valid_to_column := 'valid_end_date');
125+
SELECT sql_saga.add_unique_key('t_uk_gen', ARRAY['id']);
126+
127+
CREATE TABLE t_fk_gen (
128+
id int,
129+
uk_id int,
130+
valid_from date,
131+
valid_until date,
132+
valid_end_date date -- synchronized column
133+
);
134+
SELECT sql_saga.add_era('t_fk_gen', p_synchronize_valid_to_column := 'valid_end_date');
135+
136+
-- This should succeed and create a trigger that watches valid_end_date
137+
SELECT sql_saga.add_foreign_key('t_fk_gen', ARRAY['uk_id'], 'valid', 't_uk_gen_id_valid');
138+
139+
-- cleanup
140+
SELECT sql_saga.drop_foreign_key('t_fk_gen', ARRAY['uk_id'], 'valid');
141+
SELECT sql_saga.drop_era('t_fk_gen');
142+
DROP TABLE t_fk_gen;
143+
SELECT sql_saga.drop_unique_key('t_uk_gen', ARRAY['id'], 'valid');
144+
SELECT sql_saga.drop_era('t_uk_gen');
145+
DROP TABLE t_uk_gen;
146+
147+
82148
-- Test case for bug with infinite parent validity
83149
CREATE TABLE legal_unit_bug (
84150
id INT NOT NULL,

‎src/20_add_foreign_key.sql‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -436,15 +436,17 @@ BEGIN
436436
INTO foreign_columns_with_era_columns
437437
FROM unnest(fk_column_names || fk_era_row.valid_from_column_name || fk_era_row.valid_until_column_name) WITH ORDINALITY AS u (column_name, ordinality);
438438

439-
-- If a 'valid_to' column exists, add it to the list of columns that
440-
-- trigger the fk_update_check. This handles cases where a BEFORE trigger
441-
-- synchronizes valid_to and valid_until, ensuring validation fires correctly
442-
-- without making the trigger an overly-broad row-level trigger.
443-
IF EXISTS (
444-
SELECT 1 FROM pg_catalog.pg_attribute
445-
WHERE attrelid = fk_table_oid AND attname = 'valid_to' AND NOT attisdropped
446-
) THEN
447-
foreign_columns_with_era_columns := foreign_columns_with_era_columns || ', ' || quote_ident('valid_to');
439+
-- If a synchronized 'valid_to'-style column is defined for the era, add it
440+
-- to the list of columns that trigger the fk_update_check. This is only
441+
-- done if the column is not already part of the core era columns to avoid
442+
-- duplication. This handles cases where a separate BEFORE trigger
443+
-- synchronizes this column with valid_until, ensuring validation fires
444+
-- correctly without making the trigger an overly-broad row-level trigger.
445+
IF fk_era_row.synchronize_valid_to_column IS NOT NULL
446+
AND fk_era_row.synchronize_valid_to_column <> fk_era_row.valid_from_column_name
447+
AND fk_era_row.synchronize_valid_to_column <> fk_era_row.valid_until_column_name
448+
THEN
449+
foreign_columns_with_era_columns := foreign_columns_with_era_columns || ', ' || quote_ident(fk_era_row.synchronize_valid_to_column);
448450
END IF;
449451

450452
SELECT string_agg(quote_ident(u.column_name), ', ' ORDER BY u.ordinality)

‎todo.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ Keep a journal.md that tracks the state of the current ongoing task and relevant
99
- [x] **Improve `temporal_merge` parameter validation:** Added server-side checks to `temporal_merge` to provide clear, immediate error messages for invalid parameters, such as `NULL` or non-existent column names, improving developer experience.
1010
- [x] **Implement `sql_saga.temporal_merge` (Set-Based Upsert API):** Provided a single, high-performance, set-based function for `INSERT`/`UPDATE`/`DELETE` operations on temporal tables. The API is simplified via `regclass` parameters, era introspection, and auto-detection of defaulted columns. This is the official solution for bulk data modifications.
1111

12+
- [x] **Fix duplicated column in FK trigger `UPDATE OF` list:** Corrected `add_foreign_key` to prevent adding a synchronized `valid_to`-style column to the trigger's `UPDATE OF` list if it is already part of the era's temporal columns, resolving a "column specified more than once" error. The fix is generic and respects the `synchronize_valid_to_column` era metadata.
13+
1214
- [x] **Improve `rename_following` to support column renames:** The event trigger now correctly detects when a column in a foreign key is renamed and automatically updates all relevant metadata, including the foreign key name, column list, and associated trigger names.
1315

1416
- [x] **Foreign key validation fails for tables in different schemas:** Fixed. The `fk_update_trigger` is now created with a dynamic column list that includes `valid_to` (if present) to ensure validation fires correctly for synchronized columns without being an overly-broad row-level trigger.

0 commit comments

Comments
 (0)