Skip to content

Add DB model with connection init#111

Merged
ChallaHalla merged 4 commits intomainfrom
add-db-model
Aug 18, 2025
Merged

Add DB model with connection init#111
ChallaHalla merged 4 commits intomainfrom
add-db-model

Conversation

@ChallaHalla
Copy link
Contributor

This PR adds a DB model to the codebase that can be used for initializing DB connections.

The important areas for review are:

  • Db connection initialization and schema loading
    • Assuming that going forward we can initialize a new DB connection for each query we need to run
    • Added schema versioning so that it's clear when the schema needs to be loaded
  • Db type (memory vs disk) and path config
  • db as a new attribute on Graph
  • Database schema within schema.sql

Changes related to writing Index data to the DB will be added in a follow up PR.

Simplify getting DB connection by removing oncelock

linting

Add schema version to DB to determine if schema load is required
@ChallaHalla ChallaHalla requested a review from a team as a code owner August 15, 2025 18:18
};

let index = Arc::new(Mutex::new(Graph::new()));
let db_path = std::env::temp_dir().join("index.db").to_string_lossy().to_string();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This path should be within the project being indexed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will leave this for now and address in a follow up PR once we determine where temp files for the indexer should be placed in a project.

#[must_use]
pub fn new(uri: String) -> Self {
let mut local_index = Graph::new();
let mut local_index = Graph::new(String::new());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Choosing to simply pass an empty string here since this instance won't be using a DB connection

@ChallaHalla ChallaHalla requested a review from vinistock August 18, 2025 14:21
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

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

Just a couple of comments, but I think this is enough for us to move forward and start iterating on it

/// # Errors
/// Will return an Error if we fail to establish or set a connection
pub fn get_connection(&self) -> Result<Connection, Box<dyn Error>> {
let mut conn = if self.path == ":memory:" {
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove the memory thing for now since we're not using it. If we were to keep it, I'd argue this should leverage an enum instead of a string, so that we make better use of the type system.

enum ConnectionType {
  Path(String),
  Memory
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True the magic string comparison isn't great. Will add an enum we can check here

let current_version: i32 = conn.query_row("PRAGMA user_version", [], |row| row.get(0))?;

if current_version < SCHEMA_VERSION {
if self.path == ":memory:" {
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

@ChallaHalla ChallaHalla merged commit 213d621 into main Aug 18, 2025
8 checks passed
@ChallaHalla ChallaHalla deleted the add-db-model branch August 18, 2025 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants