fix: add domain.name indexes to subgraph schema#1856
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
📝 WalkthroughWalkthroughUpdated the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Re-enables an index on subgraph_domains.name in the ENS subgraph-compatible Ponder/Drizzle schema, switching from a btree index to a hash index to avoid Postgres’ btree index-row size limit when spam/oversized names are present.
Changes:
- Add a hash index on
subgraph_domains.name(byExactName) to support exact-match lookups without hitting the btree 8191-byte limit. - Remove the prior commented-out/disabled
byNameindex block.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| // For more details, see: https://github.com/namehash/ensnode/issues/1819 | ||
| // byName: index().on(t.name), | ||
| // uses a hash index because some name values exceed the btree max row size (8191 bytes) | ||
| byExactName: index().using("hash", t.name), |
There was a problem hiding this comment.
| byExactName: index().using("hash", t.name), | |
| byExactName: index().using("hash").on(t.name), |
Incorrect hash index syntax: index().using("hash", t.name) passes column as second parameter instead of using .on() method
|
The update looks good to me 👍 As discussed on Slack, it'd great to enable the |
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@shrugs Hey shared a suggestion. Please feel welcome to merge when ready 🫡
| // Temporarily disable the `byName` index to avoid index creation issues. | ||
| // For more details, see: https://github.com/namehash/ensnode/issues/1819 | ||
| // byName: index().on(t.name), | ||
| // uses a hash index because some name values exceed the btree max row size (8191 bytes) |
There was a problem hiding this comment.
@shrugs Hey, I note that the PR @tk-o was working on to add initializeIndexingSetup has been merged now in #1843.
What do you think about exploring how we might add logic in that function to activate the pg_trgm extension in Postgres?
If we do this now:
- Suggest to add some extra docs somewhere in
ensdb-sdktalking about this extension.
If on the other hand we don't do this now:
- Suggest to create a follow-up issue to log this opportunity.
Appreciate your advice 👍
references #1819
Summary
domain.nameindex (disabled in CreatebyNameindex onsubgraph_domainstable #1819) using a hash index to avoid the btree 8191-byte row size limit caused by spam namesOut of Scope
gin_trgm_ops) for partial match operations (_contains,_starts_with,_ends_with)pg_trgmextension, which is obviously available, but not sure how best to ensure that theCREATE EXTENSION IF NOT EXISTS pg_trgmcommand is executed before ponder applies indexes. maybe could be part of ensdbwriterworker?