Skip to content

fix: set not null on some columns #930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 22, 2025
Merged

fix: set not null on some columns #930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
894f9a1
chore: remove useless bucket features
gfyrag May 14, 2025
76a4f95
fix: set not null on some columns
gfyrag May 14, 2025
051f884
fix: previous migration
gfyrag May 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 3 additions & 14 deletions internal/storage/bucket/migrations/39-clean-useless-features/up.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,17 @@ do $$
begin
set search_path = '{{ .Schema }}';

for ledger in select * from _system.ledgers where bucket = current_schema loop
vsql = 'drop trigger "transaction_set_addresses_' || ledger.id || '" on transactions';
execute vsql;

vsql = 'drop trigger "accounts_set_address_array_' || ledger.id || '" on accounts';
execute vsql;

vsql = 'drop trigger "transaction_set_addresses_segments_' || ledger.id || '" on transactions';
execute vsql;
end loop;

-- recreate trigger but with a check on sources is null for transactions and address_array is null for accounts
-- this way, the 2.2 (which use triggers) continuer to work, and the 2.3 which does not need them can work too
-- todo(next minor / 2.4): remove triggers and associated functions
for ledger in select * from _system.ledgers where bucket = current_schema loop
vsql = 'create trigger "transaction_set_addresses_' || ledger.id || '" before insert on transactions for each row when (new.ledger = ''' || ledger.name || ''' and new.sources is null) execute procedure set_transaction_addresses()';
vsql = 'create or replace trigger "transaction_set_addresses_' || ledger.id || '" before insert on transactions for each row when (new.ledger = ''' || ledger.name || ''' and new.sources is null) execute procedure set_transaction_addresses()';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use format() with quote_ident/quote_literal for safer dynamic SQL
Concatenating ledger.name directly can break or allow SQL injection if the ledger name contains quotes or special characters. Prefer format() and the %I/%L specifiers:

-   vsql = 'create or replace trigger "transaction_set_addresses_' || ledger.id || 
-          '" before insert on transactions for each row when (new.ledger = ''' || 
-          ledger.name || ''' and new.sources is null) execute procedure set_transaction_addresses()';
+   vsql := format(
+     $f$CREATE OR REPLACE TRIGGER "transaction_set_addresses_%s"
+       BEFORE INSERT ON transactions
+       FOR EACH ROW 
+       WHEN (NEW.ledger = %L AND NEW.sources IS NULL)
+       EXECUTE PROCEDURE set_transaction_addresses()$f$,
+     ledger.id,
+     ledger.name
+   );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
vsql = 'create or replace trigger "transaction_set_addresses_' || ledger.id || '" before insert on transactions for each row when (new.ledger = ''' || ledger.name || ''' and new.sources is null) execute procedure set_transaction_addresses()';
-- replace manual concatenation with format() for safe quoting
vsql := format(
$f$CREATE OR REPLACE TRIGGER "transaction_set_addresses_%s"
BEFORE INSERT ON transactions
FOR EACH ROW
WHEN (NEW.ledger = %L AND NEW.sources IS NULL)
EXECUTE PROCEDURE set_transaction_addresses()$f$,
ledger.id,
ledger.name
);
🤖 Prompt for AI Agents 
In internal/storage/bucket/migrations/39-clean-useless-features/up.sql at line
12, the dynamic SQL concatenates ledger.name directly, risking SQL injection or
syntax errors with special characters. Replace the string concatenation with the
use of the format() function, applying %I for identifiers and %L for literals to
safely quote ledger.id and ledger.name within the trigger creation statement.

execute vsql;

vsql = 'create trigger "accounts_set_address_array_' || ledger.id || '" before insert on accounts for each row when (new.ledger = ''' || ledger.name || ''' and new.address_array is null) execute procedure set_address_array_for_account()';
vsql = 'create or replace trigger "accounts_set_address_array_' || ledger.id || '" before insert on accounts for each row when (new.ledger = ''' || ledger.name || ''' and new.address_array is null) execute procedure set_address_array_for_account()';
execute vsql;

vsql = 'create trigger "transaction_set_addresses_segments_' || ledger.id || '" before insert on "transactions" for each row when (new.ledger = ''' || ledger.name || ''' and new.sources is null) execute procedure set_transaction_addresses_segments()';
vsql = 'create or replace trigger "transaction_set_addresses_segments_' || ledger.id || '" before insert on "transactions" for each row when (new.ledger = ''' || ledger.name || ''' and new.sources_arrays is null) execute procedure set_transaction_addresses_segments()';
execute vsql;
end loop;

Expand Down
1 change: 1 addition & 0 deletions internal/storage/bucket/migrations/40-refine-columns-types/notes.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
name: Refine some column types
61 changes: 61 additions & 0 deletions internal/storage/bucket/migrations/40-refine-columns-types/up.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
do $$
begin
set search_path = '{{ .Schema }}';

-- short-time exclusive lock
alter table transactions
add constraint sources_not_null
check (sources is not null) not valid;

-- seqscan, but without exclusive lock, concurrent sessions can read/write
alter table transactions validate constraint sources_not_null;

-- exclusive lock, fast because the constraints already check the values
alter table transactions alter column sources set not null;

-- not needed anymore
alter table transactions drop constraint sources_not_null;

-- short-time exclusive lock
alter table transactions
add constraint destinations_not_null
check (destinations is not null) not valid;

-- seqscan, but without exclusive lock, concurrent sessions can read/write
alter table transactions validate constraint destinations_not_null;

-- exclusive lock, fast because the constraints already check the values
alter table transactions alter column destinations set not null;

-- not needed anymore
alter table transactions drop constraint destinations_not_null;

-- short-time exclusive lock
alter table transactions
add constraint sources_arrays_not_null
check (sources_arrays is not null) not valid;

-- seqscan, but without exclusive lock, concurrent sessions can read/write
alter table transactions validate constraint sources_arrays_not_null;

-- exclusive lock, fast because the constraints already check the values
alter table transactions alter column sources_arrays set not null;

-- not needed anymore
alter table transactions drop constraint sources_arrays_not_null;

-- short-time exclusive lock
alter table transactions
add constraint destinations_arrays_not_null
check (destinations_arrays is not null) not valid;

-- seqscan, but without exclusive lock, concurrent sessions can read/write
alter table transactions validate constraint destinations_arrays_not_null;

-- exclusive lock, fast because the constraints already check the values
alter table transactions alter column destinations_arrays set not null;

-- not needed anymore
alter table transactions drop constraint destinations_arrays_not_null;
end
$$;
Loading