Skip to content
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

Change SQL statement InsertNewAsset into an upsert. #684

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Empty file.
31 changes: 31 additions & 0 deletions tapdb/sqlc/migrations/000013_assets_unique_constraint.up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
-- We will apply a new unique constraint on the assets table. This constraint
Copy link
Member

Choose a reason for hiding this comment

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

nit: we should add a comment to the .down.sql file that it's empty on purpose.

-- will be on the columns (anchor_utxo_id, genesis_id, script_key_id).
-- Rows in the existing table may violate this constraint, so we need to delete
-- all but one row in each violating set of rows before applying the constraint.
WITH duplicate_rows AS (
SELECT
asset_id,
anchor_utxo_id,
genesis_id,
script_key_id,

-- This is the row number of the row within the set of rows that violate
-- the constraint. We will delete all rows with a row number greater
-- than 1.
ROW_NUMBER() OVER (PARTITION BY anchor_utxo_id, genesis_id, script_key_id ORDER BY asset_id) AS row_num
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see now why we'd want to have a way to test migration scripts...
I think this probably works, but we definitely need to make sure it does the correct thing for both Postgres and SQLite.

FROM assets
)
DELETE FROM assets
WHERE (anchor_utxo_id, genesis_id, script_key_id) IN (
SELECT anchor_utxo_id, genesis_id, script_key_id
FROM duplicate_rows
-- Delete all rows with a row number greater than 1. This should leave
-- exactly one row (row number 0) for each set of rows that violate the
-- constraint.
WHERE row_num > 1
);

-- Create a unique index on the new table
CREATE UNIQUE INDEX assets_uniqueness_index_anchor_utxo_id_genesis_id_script_key_id
Copy link
Member

Choose a reason for hiding this comment

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

Do we have a way to remove the index created by the previous UNIQUE(asset_id, genesis_id, script_key_id) part? I guess that's probably dependent on the actual DB implementation.
This is likely a downside of using the "inline" unique key creation, that it doesn't have a name and therefore can't easily be updated.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I think I google that before at some point... And if I remember correctly, we basically need to re-create the table to get rid of the original unique constraint. But I think we'd probably want to do that, as otherwise this is an expensive but useless index we're carrying along.
IIRC this is how you replace a table "correctly":

  1. CREATE TABLE assets_tmp ... (full definition, but without the inline UNIQUE constraint)
  2. INSERT INTO assets_tmp AS SELECT * from assets
  3. Alter any foreign keys that point to the old assets table
  4. DROP TABLE assets
  5. ALTER TABLE assets_tmp RENAME TO assets

ON assets (anchor_utxo_id, genesis_id, script_key_id);