Skip to content

Conversation

@ManuelBilbao
Copy link
Contributor

Motivation

Migrations of SQL schema are tricky. If new tables were added, it has to be migrated manually.

Description

Make SQL schema setup sentences idempotent so they can be executed on every start up.

@ManuelBilbao ManuelBilbao self-assigned this Dec 1, 2025
Copilot AI review requested due to automatic review settings December 1, 2025 18:19
@ManuelBilbao ManuelBilbao added the L2 Rollup client label Dec 1, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the SQL store initialization to make it idempotent by adding IF NOT EXISTS clauses to all CREATE TABLE statements and ON CONFLICT DO NOTHING clauses to INSERT statements. This allows the schema setup to be executed on every startup without requiring manual migrations when new tables are added.

Key changes:

  • All CREATE TABLE statements now use IF NOT EXISTS to prevent errors on re-execution
  • Initial INSERT statements for operation_count and latest_sent tables now use ON CONFLICT DO NOTHING to avoid duplicate key errors
  • Removed the check for existing tables before running schema setup, allowing the idempotent schema statements to run unconditionally

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// "readers do not block writers and a writer does not block readers. Reading and writing can proceed concurrently"
// https://sqlite.org/wal.html#concurrency
// still a limit of only 1 writer is imposed by sqlite databases
self.query("PRAGMA journal_mode=WAL;", ()).await?;
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

PRAGMA journal_mode=WAL is a write operation that modifies the database configuration, but it's being executed using self.query() which uses the read-only connection (read_conn). This should use self.execute() instead, which uses the write_conn connection. While this may work in some SQLite implementations, it could lead to "database is locked" errors or permission issues as mentioned in the comment on line 19.

Suggested change
self.query("PRAGMA journal_mode=WAL;", ()).await?;
self.execute("PRAGMA journal_mode=WAL;", ()).await?;

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

github-actions bot commented Dec 1, 2025

Lines of code report

Total lines added: 0
Total lines removed: 8
Total lines changed: 8

Detailed view
+----------------------------------------------+-------+------+
| File                                         | Lines | Diff |
+----------------------------------------------+-------+------+
| ethrex/crates/l2/storage/src/store_db/sql.rs | 906   | -8   |
+----------------------------------------------+-------+------+

@avilagaston9 avilagaston9 added this pull request to the merge queue Dec 1, 2025
Merged via the queue into main with commit e982a7d Dec 1, 2025
41 checks passed
@avilagaston9 avilagaston9 deleted the improve_sql_store_init branch December 1, 2025 21:07
@github-project-automation github-project-automation bot moved this to Done in ethrex_l2 Dec 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L2 Rollup client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants