Skip to content

Commit 312f43d

Browse files
jhfjhf
committed
temporal_merge: Fix bug reported by statbus
1 parent 1a470c3 commit 312f43d

7 files changed

Lines changed: 593 additions & 8 deletions

‎expected/70_temporal_merge_default_value_handling_bug.out‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
\i sql/include/test_setup.sql
2+
--
3+
-- test_setup.sql
4+
--
5+
-- Common setup for regression tests that need to be self-contained.
6+
-- This script creates the extension, a user role, and grants permissions.
7+
--
8+
SET datestyle = 'ISO, YMD';
9+
CREATE EXTENSION IF NOT EXISTS btree_gist;
10+
CREATE EXTENSION IF NOT EXISTS sql_saga CASCADE;
11+
DO $$
12+
BEGIN
13+
CREATE ROLE sql_saga_unprivileged_user;
14+
EXCEPTION WHEN duplicate_object THEN
15+
END
16+
$$;
17+
GRANT USAGE ON SCHEMA sql_saga TO sql_saga_unprivileged_user;
18+
GRANT SELECT ON ALL TABLES IN SCHEMA sql_saga TO sql_saga_unprivileged_user;
19+
GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA sql_saga TO sql_saga_unprivileged_user;
20+
/*
21+
* Allow the unprivileged user to create tables in the public schema.
22+
* This is required for tests that create their own tables.
23+
* PG 15+ restricts this by default.
24+
*/
25+
GRANT CREATE ON SCHEMA public TO PUBLIC;
26+
-- This script provides a minimal, reproducible example of a bug in
27+
-- sql_saga.temporal_merge where a missing column with a DEFAULT value
28+
-- in the source table causes a "structure of query does not match" error.
29+
BEGIN;
30+
-- Setup a clean environment
31+
DROP SCHEMA IF EXISTS repro CASCADE;
32+
NOTICE: schema "repro" does not exist, skipping
33+
CREATE SCHEMA repro;
34+
-- Minimal dependencies for foreign keys
35+
CREATE TABLE repro.auth_user (id int primary key);
36+
INSERT INTO repro.auth_user VALUES (1);
37+
CREATE TABLE repro.legal_unit (
38+
id int primary key,
39+
valid_from date,
40+
valid_until date,
41+
valid_to date
42+
);
43+
INSERT INTO repro.legal_unit VALUES (123, '2020-01-01', 'infinity', 'infinity');
44+
CREATE TABLE repro.stat_definition (
45+
id int primary key,
46+
code text unique,
47+
type text,
48+
name text
49+
);
50+
INSERT INTO repro.stat_definition VALUES (1, 'employees', 'int', 'Employees');
51+
-- 1. Create the target table, mimicking the real one.
52+
-- Note the `created_at` column with a DEFAULT.
53+
CREATE TABLE repro.stat_for_unit (
54+
id SERIAL PRIMARY KEY,
55+
stat_definition_id INT NOT NULL REFERENCES repro.stat_definition(id),
56+
legal_unit_id INT REFERENCES repro.legal_unit(id),
57+
establishment_id INT,
58+
value_int INT,
59+
value_float DOUBLE PRECISION,
60+
value_string VARCHAR,
61+
value_bool BOOLEAN,
62+
valid_from DATE NOT NULL,
63+
valid_to DATE NOT NULL,
64+
valid_until DATE NOT NULL,
65+
data_source_id INT,
66+
created_at TIMESTAMPTZ NOT NULL DEFAULT statement_timestamp(),
67+
edit_by_user_id INT NOT NULL REFERENCES repro.auth_user(id),
68+
edit_at TIMESTAMPTZ NOT NULL DEFAULT statement_timestamp(),
69+
edit_comment VARCHAR(512)
70+
);
71+
-- Enable sql_saga on the target table
72+
SELECT sql_saga.add_era('repro.stat_for_unit'::regclass, p_synchronize_valid_to_column := 'valid_to');
73+
NOTICE: sql_saga: Created trigger "stat_for_unit_synchronize_temporal_columns_trigger" on table repro.stat_for_unit to synchronize columns: valid_to
74+
add_era
75+
---------
76+
t
77+
(1 row)
78+
79+
SELECT sql_saga.add_unique_key('repro.stat_for_unit'::regclass, ARRAY['id']);
80+
add_unique_key
81+
------------------------
82+
stat_for_unit_id_valid
83+
(1 row)
84+
85+
-- 2. Create the source table, which is missing the `created_at` column.
86+
-- This is the hypothesized root cause of the bug.
87+
CREATE TEMP TABLE temp_stat_merge_source (
88+
synthetic_row_id INT GENERATED ALWAYS AS IDENTITY PRIMARY KEY,
89+
founding_key TEXT,
90+
id INT,
91+
legal_unit_id INT,
92+
establishment_id INT,
93+
stat_definition_id INT,
94+
value_string TEXT, value_int INT, value_float DOUBLE PRECISION, value_bool BOOLEAN,
95+
valid_from DATE NOT NULL,
96+
valid_to DATE NOT NULL,
97+
valid_until DATE NOT NULL,
98+
data_source_id INT,
99+
edit_by_user_id INT,
100+
edit_at TIMESTAMPTZ,
101+
edit_comment TEXT
102+
);
103+
-- 3. Insert one row of sample data into the source table.
104+
INSERT INTO temp_stat_merge_source (
105+
founding_key, id,
106+
legal_unit_id, establishment_id, stat_definition_id,
107+
value_string, value_int, value_float, value_bool,
108+
valid_from, valid_to, valid_until,
109+
data_source_id, edit_by_user_id, edit_at, edit_comment
110+
) VALUES (
111+
'101_1', -- founding_key (founding_row_id || '_' || stat_def_id)
112+
NULL, -- id is NULL because this is a new stat_for_unit record
113+
123, -- legal_unit_id (points to our existing LU)
114+
NULL, -- establishment_id
115+
1, -- stat_definition_id (for 'employees')
116+
NULL, 25, NULL, NULL, -- The value is an integer
117+
'2024-01-01', -- valid_from
118+
'2024-12-31', -- valid_to
119+
'2025-01-01', -- valid_until
120+
NULL, -- data_source_id
121+
1, -- edit_by_user_id
122+
NOW(), -- edit_at
123+
'Test import' -- edit_comment
124+
);
125+
-- 4. Call temporal_merge. This is now expected to succeed because the executor
126+
-- correctly handles source tables that are missing columns with DEFAULT values.
127+
\echo '--- Calling temporal_merge (EXPECTED TO SUCCEED) ---'
128+
--- Calling temporal_merge (EXPECTED TO SUCCEED) ---
129+
CALL sql_saga.temporal_merge(
130+
p_target_table => 'repro.stat_for_unit',
131+
p_source_table => 'temp_stat_merge_source',
132+
p_id_columns => ARRAY['id'],
133+
p_ephemeral_columns => ARRAY['edit_comment', 'edit_by_user_id', 'edit_at'],
134+
p_mode => 'MERGE_ENTITY_REPLACE',
135+
p_era_name => 'valid',
136+
p_source_row_id_column => 'synthetic_row_id',
137+
p_founding_id_column => 'founding_key',
138+
p_update_source_with_assigned_entity_ids => true
139+
);
140+
\echo '--- Verification: Check if data was inserted and created_at defaulted ---'
141+
--- Verification: Check if data was inserted and created_at defaulted ---
142+
SELECT id, legal_unit_id, stat_definition_id, value_int, valid_from, valid_to, created_at IS NOT NULL as created_at_is_set
143+
FROM repro.stat_for_unit;
144+
id | legal_unit_id | stat_definition_id | value_int | valid_from | valid_to | created_at_is_set
145+
----+---------------+--------------------+-----------+------------+------------+-------------------
146+
1 | 123 | 1 | 25 | 2024-01-01 | 2024-12-31 | t
147+
(1 row)
148+
149+
ROLLBACK;
150+
\i sql/include/test_teardown.sql
151+
--
152+
-- test_teardown.sql
153+
--
154+
-- Common teardown for regression tests. This script drops the unprivileged
155+
-- user role created by test_setup.sql.
156+
--
157+
-- It is important to reset the role first, in case a test fails and
158+
-- leaves the session role set to the user that is about to be dropped.
159+
RESET ROLE;
160+
-- Drop the extensions to ensure a clean state for the next test.
161+
-- Use CASCADE to remove any dependent objects created by sql_saga.
162+
DROP EXTENSION IF EXISTS sql_saga CASCADE;
163+
DROP EXTENSION IF EXISTS btree_gist CASCADE;
164+
-- Revoke any privileges held by the test user and drop any objects they own.
165+
-- This is necessary before the role can be dropped.
166+
DROP OWNED BY sql_saga_unprivileged_user;
167+
DROP ROLE IF EXISTS sql_saga_unprivileged_user;

‎expected/71_temporal_merge_data_type_inconsistency_bug.out‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
\i sql/include/test_setup.sql
2+
--
3+
-- test_setup.sql
4+
--
5+
-- Common setup for regression tests that need to be self-contained.
6+
-- This script creates the extension, a user role, and grants permissions.
7+
--
8+
SET datestyle = 'ISO, YMD';
9+
CREATE EXTENSION IF NOT EXISTS btree_gist;
10+
CREATE EXTENSION IF NOT EXISTS sql_saga CASCADE;
11+
DO $$
12+
BEGIN
13+
CREATE ROLE sql_saga_unprivileged_user;
14+
EXCEPTION WHEN duplicate_object THEN
15+
END
16+
$$;
17+
GRANT USAGE ON SCHEMA sql_saga TO sql_saga_unprivileged_user;
18+
GRANT SELECT ON ALL TABLES IN SCHEMA sql_saga TO sql_saga_unprivileged_user;
19+
GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA sql_saga TO sql_saga_unprivileged_user;
20+
/*
21+
* Allow the unprivileged user to create tables in the public schema.
22+
* This is required for tests that create their own tables.
23+
* PG 15+ restricts this by default.
24+
*/
25+
GRANT CREATE ON SCHEMA public TO PUBLIC;
26+
-- This script provides a minimal, reproducible example of a bug in
27+
-- sql_saga.temporal_merge giving a bigint[] vs integer[] error.
28+
BEGIN;
29+
-- Setup a clean environment
30+
DROP SCHEMA IF EXISTS repro CASCADE;
31+
NOTICE: schema "repro" does not exist, skipping
32+
CREATE SCHEMA repro;
33+
-- Minimal dependencies for foreign keys
34+
CREATE TABLE repro.auth_user (id int primary key);
35+
INSERT INTO repro.auth_user VALUES (1);
36+
CREATE TABLE repro.legal_unit (
37+
id int primary key,
38+
valid_from date,
39+
valid_until date,
40+
valid_to date
41+
);
42+
INSERT INTO repro.legal_unit VALUES (123, '2020-01-01', 'infinity', 'infinity');
43+
CREATE TABLE repro.stat_definition (
44+
id int primary key,
45+
code text unique,
46+
type text,
47+
name text
48+
);
49+
INSERT INTO repro.stat_definition VALUES (1, 'employees', 'int', 'Employees');
50+
-- 1. Create the target table, mimicking the real one.
51+
-- Note the `created_at` column with a DEFAULT.
52+
CREATE TABLE repro.stat_for_unit (
53+
id SERIAL PRIMARY KEY,
54+
stat_definition_id INT NOT NULL REFERENCES repro.stat_definition(id),
55+
legal_unit_id INT REFERENCES repro.legal_unit(id),
56+
establishment_id INT,
57+
value_int INT,
58+
value_float DOUBLE PRECISION,
59+
value_string VARCHAR,
60+
value_bool BOOLEAN,
61+
valid_from DATE NOT NULL,
62+
valid_to DATE NOT NULL,
63+
valid_until DATE NOT NULL,
64+
data_source_id INT,
65+
created_at TIMESTAMPTZ NOT NULL DEFAULT statement_timestamp(),
66+
edit_by_user_id INT NOT NULL REFERENCES repro.auth_user(id),
67+
edit_at TIMESTAMPTZ NOT NULL DEFAULT statement_timestamp(),
68+
edit_comment VARCHAR(512)
69+
);
70+
-- Enable sql_saga on the target table
71+
SELECT sql_saga.add_era('repro.stat_for_unit'::regclass, p_synchronize_valid_to_column := 'valid_to');
72+
NOTICE: sql_saga: Created trigger "stat_for_unit_synchronize_temporal_columns_trigger" on table repro.stat_for_unit to synchronize columns: valid_to
73+
add_era
74+
---------
75+
t
76+
(1 row)
77+
78+
SELECT sql_saga.add_unique_key('repro.stat_for_unit'::regclass, ARRAY['id']);
79+
add_unique_key
80+
------------------------
81+
stat_for_unit_id_valid
82+
(1 row)
83+
84+
-- 2. Create the source table, which is missing the `created_at` column.
85+
-- This is the hypothesized root cause of the bug.
86+
CREATE TEMP TABLE temp_stat_merge_source (
87+
synthetic_row_id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY,
88+
founding_key TEXT,
89+
id INT,
90+
legal_unit_id INT,
91+
establishment_id INT,
92+
stat_definition_id INT,
93+
value_string TEXT, value_int INT, value_float DOUBLE PRECISION, value_bool BOOLEAN,
94+
valid_from DATE NOT NULL,
95+
valid_to DATE NOT NULL,
96+
valid_until DATE NOT NULL,
97+
data_source_id INT,
98+
edit_by_user_id INT,
99+
edit_at TIMESTAMPTZ,
100+
edit_comment TEXT
101+
);
102+
-- 3. Insert one row of sample data into the source table.
103+
INSERT INTO temp_stat_merge_source (
104+
founding_key, id,
105+
legal_unit_id, establishment_id, stat_definition_id,
106+
value_string, value_int, value_float, value_bool,
107+
valid_from, valid_to, valid_until,
108+
data_source_id, edit_by_user_id, edit_at, edit_comment
109+
) VALUES (
110+
'101_1', -- founding_key (founding_row_id || '_' || stat_def_id)
111+
NULL, -- id is NULL because this is a new stat_for_unit record
112+
123, -- legal_unit_id (points to our existing LU)
113+
NULL, -- establishment_id
114+
1, -- stat_definition_id (for 'employees')
115+
NULL, 25, NULL, NULL, -- The value is an integer
116+
'2024-01-01', -- valid_from
117+
'2024-12-31', -- valid_to
118+
'2025-01-01', -- valid_until
119+
NULL, -- data_source_id
120+
1, -- edit_by_user_id
121+
NOW(), -- edit_at
122+
'Test import' -- edit_comment
123+
);
124+
-- 4. Call temporal_merge. This is now expected to succeed because the executor
125+
-- correctly handles BIGINT source row IDs and source tables that are missing columns
126+
-- with DEFAULT values.
127+
\echo '--- Calling temporal_merge (EXPECTED TO SUCCEED) ---'
128+
--- Calling temporal_merge (EXPECTED TO SUCCEED) ---
129+
CALL sql_saga.temporal_merge(
130+
p_target_table => 'repro.stat_for_unit',
131+
p_source_table => 'temp_stat_merge_source',
132+
p_id_columns => ARRAY['id'],
133+
p_ephemeral_columns => ARRAY['edit_comment', 'edit_by_user_id', 'edit_at'],
134+
p_mode => 'MERGE_ENTITY_REPLACE',
135+
p_era_name => 'valid',
136+
p_source_row_id_column => 'synthetic_row_id',
137+
p_founding_id_column => 'founding_key',
138+
p_update_source_with_assigned_entity_ids => true
139+
);
140+
\echo '--- Verification: Check if data was inserted and created_at defaulted ---'
141+
--- Verification: Check if data was inserted and created_at defaulted ---
142+
SELECT id, legal_unit_id, stat_definition_id, value_int, valid_from, valid_to, created_at IS NOT NULL as created_at_is_set
143+
FROM repro.stat_for_unit;
144+
id | legal_unit_id | stat_definition_id | value_int | valid_from | valid_to | created_at_is_set
145+
----+---------------+--------------------+-----------+------------+------------+-------------------
146+
1 | 123 | 1 | 25 | 2024-01-01 | 2024-12-31 | t
147+
(1 row)
148+
149+
ROLLBACK;
150+
\i sql/include/test_teardown.sql
151+
--
152+
-- test_teardown.sql
153+
--
154+
-- Common teardown for regression tests. This script drops the unprivileged
155+
-- user role created by test_setup.sql.
156+
--
157+
-- It is important to reset the role first, in case a test fails and
158+
-- leaves the session role set to the user that is about to be dropped.
159+
RESET ROLE;
160+
-- Drop the extensions to ensure a clean state for the next test.
161+
-- Use CASCADE to remove any dependent objects created by sql_saga.
162+
DROP EXTENSION IF EXISTS sql_saga CASCADE;
163+
DROP EXTENSION IF EXISTS btree_gist CASCADE;
164+
-- Revoke any privileges held by the test user and drop any objects they own.
165+
-- This is necessary before the role can be dropped.
166+
DROP OWNED BY sql_saga_unprivileged_user;
167+
DROP ROLE IF EXISTS sql_saga_unprivileged_user;

0 commit comments

Comments
 (0)