Update Schema #290
Update Schema #290
Conversation
|
We're building your pull request over on Zeet. |
WalkthroughSplits each token_balances materialized view into direction-specific _from_mv and _to_mv for ERC20/721/1155/6909, and replaces stateful aggregate functions with standard aggregates in token_balances/owner_balances PROJECTIONs. All edits are SQL changes across two ClickHouse migration files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant ERC20 as token_transfers_erc20
participant ERC721 as token_transfers_erc721
participant ERC1155 as token_transfers_erc1155
participant ERC6909 as token_transfers_erc6909
rect rgb(240,248,255)
note over ERC20,ERC6909: Materialized Views (split by direction)
participant MV20F as erc20_from_mv
participant MV20T as erc20_to_mv
participant MV721F as erc721_from_mv
participant MV721T as erc721_to_mv
participant MV1155F as erc1155_from_mv
participant MV1155T as erc1155_to_mv
participant MV6909F as erc6909_from_mv
participant MV6909T as erc6909_to_mv
end
participant TB as token_balances
ERC20-->>MV20F: insert transfer rows (token_type='erc20')
ERC20-->>MV20T: insert transfer rows (token_type='erc20')
MV20F->>TB: upsert {address=from_address, balance_delta=-amount, direction='from'}
MV20T->>TB: upsert {address=to_address, balance_delta=+amount, direction='to'}
ERC721-->>MV721F: insert transfer rows (token_type='erc721')
ERC721-->>MV721T: insert transfer rows (token_type='erc721')
MV721F->>TB: upsert {address=from_address, balance_delta=-1, direction='from'}
MV721T->>TB: upsert {address=to_address, balance_delta=+1, direction='to'}
ERC1155-->>MV1155F: insert transfer rows (token_type='erc1155')
ERC1155-->>MV1155T: insert transfer rows (token_type='erc1155')
MV1155F->>TB: upsert {address=from_address, balance_delta=-amount, direction='from'}
MV1155T->>TB: upsert {address=to_address, balance_delta=+amount, direction='to'}
ERC6909-->>MV6909F: insert transfer rows (token_type='erc6909')
ERC6909-->>MV6909T: insert transfer rows (token_type='erc6909')
MV6909F->>TB: upsert {address=from_address, balance_delta=-amount, direction='from'}
MV6909T->>TB: upsert {address=to_address, balance_delta=+amount, direction='to'}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks (2 passed, 1 inconclusive)❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
internal/tools/clickhouse/0009_clickhouse_create_token_balances_mv.sql (2)
46-66: ERC721 deltas and columns are sane; align zero-address handling with ERC20 decision.Using ±1 is correct for 721. Apply the same mint/burn policy consistently.
Optional filters:
-WHERE token_type = 'erc721'; +WHERE token_type = 'erc721' AND from_address != '0x0000000000000000000000000000000000000000';-WHERE token_type = 'erc721'; +WHERE token_type = 'erc721' AND to_address != '0x0000000000000000000000000000000000000000';Also applies to: 67-87
5-19: Projection/index alignment with query paths.Given our prior preference to optimize address lookups by (token_type, chain_id, address, token_id), confirm
token_balancesprojections/primary key are still optimal withowner_addressand newdirection. If queries filter bydirection, consider adding it to a projection key.Also applies to: 49-63, 93-107, 136-150
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
internal/tools/clickhouse/0009_clickhouse_create_token_balances_mv.sql(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-22T18:30:59.800Z
Learnt from: iuwqyir
PR: thirdweb-dev/insight#240
File: internal/tools/clickhouse_create_token_balances_mv.sql:8-17
Timestamp: 2025-07-22T18:30:59.800Z
Learning: In the token_balances table projection design, the user prefers to have the address_projection optimized for queries by (token_type, chain_id, address, token_id) without including owner, as the main table is already optimized for owner-based query paths.
Applied to files:
internal/tools/clickhouse/0009_clickhouse_create_token_balances_mv.sql
🔇 Additional comments (4)
internal/tools/clickhouse/0009_clickhouse_create_token_balances_mv.sql (4)
2-22: ERC20 from/to views look correct; confirm type/cast and zero-address policy.
- Casting with
toInt256(amount)assumes source fits signed 256; verify source type and overflow behavior.- Decide whether to exclude mint/burn rows (zero address) from owner balances.
Optional zero-address filter:
-FROM token_transfers -WHERE token_type = 'erc20'; +FROM token_transfers +WHERE token_type = 'erc20' AND from_address != '0x0000000000000000000000000000000000000000';-FROM token_transfers -WHERE token_type = 'erc20'; +FROM token_transfers +WHERE token_type = 'erc20' AND to_address != '0x0000000000000000000000000000000000000000';Also applies to: 23-44
89-109: ERC1155 assumes per-token exploded rows; verify upstream flattening.If
token_transfersholds batched arrays for ERC1155, this SELECT will mis-shape data. Confirm it’s already exploded to (token_id, amount) rows; if not, unnest before inserting.Also applies to: 110-130
132-152: ERC6909 cast and semantics: confirm amount units and event coverage.
- Ensure
amountcarries the correct unit for 6909 and fitsInt256.- Confirm this view is fed only by 6909 transfer-like events you intend to count.
Also applies to: 153-172
2-4: Intentional no-POPULATE?If you need historical backfill on creation, add
POPULATE; otherwise confirm a separate backfill job exists.
| CREATE MATERIALIZED VIEW IF NOT EXISTS token_balances_erc20_from_mv | ||
| TO token_balances | ||
| AS |
There was a problem hiding this comment.
Drop legacy single MVs to avoid double-inserts.
If token_balances_ercXX_mv still exists, it will continue inserting alongside the new _from_mv/_to_mv. Add defensive drops in this migration.
+-- Safety: remove legacy single-sided MVs if present
+DROP MATERIALIZED VIEW IF EXISTS token_balances_erc20_mv;
+DROP MATERIALIZED VIEW IF EXISTS token_balances_erc721_mv;
+DROP MATERIALIZED VIEW IF EXISTS token_balances_erc1155_mv;
+DROP MATERIALIZED VIEW IF EXISTS token_balances_erc6909_mv;Also applies to: 23-26, 46-48, 67-69, 89-91, 110-112, 132-134, 153-155
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/tools/clickhouse/0008_clickhouse_create_token_balances.sql (2)
34-38: Switching from State-aggregates to plain aggregates in a projection is OK; check downstream usage and consider alias cleanup.
- Aggregate projections can use plain sum/min/max; the optimizer will read from the projection when GROUP BY matches. Verify no queries still use -Merge/-Finalization over these aliases. (clickhouse.com)
- The aliases still end with
_statebut now hold finalized values; consider renaming for clarity.- Minor perf/readability: replace
if(is_deleted = 0, 1, -1)with a branchless expression to avoid UInt underflow:- sum(balance_delta * if(is_deleted = 0, 1, -1)) AS balance_state, + sum(balance_delta * (1 - 2*toInt8(is_deleted))) AS balance_state,
49-53: Same note here; also consider including token_type in this projection to align with common query filters.
- Same aggregate/projection concerns as above. (clickhouse.com)
- Given past preference to optimize by
(token_type, chain_id, address, token_id), consider addingtoken_typeto SELECT/GROUP BY so this projection is chosen for queries filtering by token_type without falling back to the base table. Example:-- sketch SELECT chain_id, token_type, token_address, token_id, owner_address, sum(balance_delta * (1 - 2*toInt8(is_deleted))) AS balance_state, min(block_number) AS min_block_number_state, min(block_timestamp) AS min_block_timestamp_state, max(block_number) AS max_block_number_state, max(block_timestamp) AS max_block_timestamp_state GROUP BY chain_id, token_type, token_address, token_id, owner_address
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
internal/tools/clickhouse/0008_clickhouse_create_token_balances.sql(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: iuwqyir
PR: thirdweb-dev/insight#240
File: internal/tools/clickhouse_create_token_balances_mv.sql:8-17
Timestamp: 2025-07-22T18:30:59.800Z
Learning: In the token_balances table projection design, the user prefers to have the address_projection optimized for queries by (token_type, chain_id, address, token_id) without including owner, as the main table is already optimized for owner-based query paths.
📚 Learning: 2025-07-22T18:30:59.800Z
Learnt from: iuwqyir
PR: thirdweb-dev/insight#240
File: internal/tools/clickhouse_create_token_balances_mv.sql:8-17
Timestamp: 2025-07-22T18:30:59.800Z
Learning: In the token_balances table projection design, the user prefers to have the address_projection optimized for queries by (token_type, chain_id, address, token_id) without including owner, as the main table is already optimized for owner-based query paths.
Applied to files:
internal/tools/clickhouse/0008_clickhouse_create_token_balances.sql
Summary by CodeRabbit
New Features
Refactor
Performance
Chores