-
Notifications
You must be signed in to change notification settings - Fork 117
fix: missing denormalized data in logs #880
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
do $$ | ||
begin | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's the 1st time I see tests after migration. Nice! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And it's the first project where I write them ^^ |
||
set search_path = '{{.Schema}}'; | ||
assert (select count(*) from transactions where inserted_at is null) = 0, 'inserted_at should not be null'; | ||
end; | ||
$$ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
name: Fill log data for reverted transactions |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
do $$ | ||
declare | ||
_offset integer := 0; | ||
_batch_size integer := 1000; | ||
begin | ||
set search_path = '{{ .Schema }}'; | ||
|
||
drop table if exists txs_view; | ||
|
||
create temp table txs_view as | ||
with reversed as ( | ||
select | ||
ledger, | ||
id, | ||
(convert_from(memento, 'UTF-8')::jsonb ->> 'revertedTransactionID')::numeric as revertedTransactionID | ||
from logs | ||
where type = 'REVERTED_TRANSACTION' and data->>'revertedTransactionID' is not null | ||
) | ||
select reversed.id as log_id, transactions.* | ||
from transactions | ||
join reversed on | ||
reversed.revertedTransactionID = transactions.id and | ||
reversed.ledger = transactions.ledger; | ||
|
||
create index txs_view_idx on txs_view(log_id, id); | ||
|
||
if (select count(*) from txs_view) = 0 then | ||
return; | ||
end if; | ||
|
||
perform pg_notify('migrations-{{ .Schema }}', 'init: ' || (select count(*) from txs_view)); | ||
|
||
loop | ||
with data as ( | ||
select * | ||
from txs_view | ||
order by ledger, log_id, id | ||
offset _offset | ||
limit _batch_size | ||
) | ||
update logs | ||
set data = data || jsonb_build_object('revertedTransaction', jsonb_build_object( | ||
'id', data.id, | ||
fguery marked this conversation as resolved.
Show resolved
Hide resolved
|
||
'postings', data.postings::jsonb, | ||
'metadata', data.metadata, | ||
'reverted', true, | ||
'revertedAt', data.reverted_at, | ||
'insertedAt', data.inserted_at, | ||
'timestamp', data.timestamp, | ||
'reference', case when data.reference is not null and data.reference <> '' then data.reference end, | ||
'postCommitVolumes', data.post_commit_volumes | ||
)) | ||
from data | ||
where logs.id = data.log_id and | ||
logs.ledger = data.ledger; | ||
|
||
exit when not found; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. worth adding a pg_notify when we exit too? (I assume this is a kind of log) |
||
|
||
_offset = _offset + _batch_size; | ||
|
||
perform pg_notify('migrations-{{ .Schema }}', 'continue: ' || _batch_size); | ||
|
||
commit; | ||
end loop; | ||
|
||
drop table if exists txs_view; | ||
end | ||
$$; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
do $$ | ||
declare | ||
expected varchar = '{"transaction": {"id": 22, "metadata": {"tax": "1%"}, "postings": [{"asset": "USD", "amount": 99, "source": "sellers:0", "destination": "orders:10"}, {"asset": "USD", "amount": 1, "source": "fees", "destination": "orders:10"}, {"asset": "USD", "amount": 100, "source": "orders:10", "destination": "world"}, {"asset": "SELL", "amount": 1, "source": "sellers:0", "destination": "world"}], ' || | ||
'"timestamp": ' || | ||
(select to_json(timestamp) from "{{.Schema}}".transactions where id = 22 and ledger = 'ledger0') | ||
|| '}, "revertedTransaction": {"id": 2, "metadata": {"tax": "1%"}, "postings": [{"asset": "SELL", "amount": 1, "source": "world", "destination": "sellers:0"}, {"asset": "USD", "amount": 100, "source": "world", "destination": "orders:10"}, {"asset": "USD", "amount": 1, "source": "orders:10", "destination": "fees"}, {"asset": "USD", "amount": 99, "source": "orders:10", "destination": "sellers:0"}], "reverted": true, "reference": null, "timestamp": ' || | ||
(select to_json(timestamp) from "{{.Schema}}".transactions where id = 2 and ledger = 'ledger0') || | ||
', "insertedAt": ' || | ||
(select to_json(inserted_at) from "{{.Schema}}".transactions where id = 2 and ledger = 'ledger0') || | ||
', "revertedAt": ' || | ||
(select to_json(reverted_at) from "{{.Schema}}".transactions where id = 2 and ledger = 'ledger0') || | ||
', "postCommitVolumes": {"fees": {"USD": {"input": 3, "output": 0}}, "world": {"USD": {"input": 0, "output": 300}, "SELL": {"input": 0, "output": 3}}, "orders:10": {"USD": {"input": 100, "output": 100}}, "sellers:0": {"USD": {"input": 297, "output": 0}, "SELL": {"input": 3, "output": 0}}}}, "revertedTransactionID": "2"}'; | ||
begin | ||
set search_path = '{{.Schema}}'; | ||
assert (select data::varchar from logs where id = 22 and ledger = 'ledger0') = expected, | ||
'data should be equals to ' || expected || ' but was ' || (select data::varchar from logs where id = 22 and ledger = 'ledger0'); | ||
end; | ||
$$ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't this be already executed, as it's a pre-existing migration? Don't we need to create a new migration file for that?
Or is just a simple refacto to simplify a bit, in which case nevermind.