Skip to content

Comments

Fixes #2702 disable connection pooling and foreign key enforcement#2703

Merged
rwmcintosh merged 3 commits intoV13from
2702-disable-connection-pooling-and-foreign-key-enforcement
Nov 10, 2025
Merged

Fixes #2702 disable connection pooling and foreign key enforcement#2703
rwmcintosh merged 3 commits intoV13from
2702-disable-connection-pooling-and-foreign-key-enforcement

Conversation

@rwmcintosh
Copy link
Member

Fixes #2702

Description

To make the behavior of the new driver more similar to the behavior of the older driver we need to set some parameters explictly.

Type of change

Please mark relevant options with an x in the brackets.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires documentation changes (link at least one user or developer documentation issue):
  • Algorithm update - updates algorithm documentation/questions/answers etc.
  • Other (please describe):

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Integration tests
  • Unit tests
  • Manual tests
  • No tests required

Reviewer checklist

Mark everything that needs to be checked before merging the PR.

  • Check if the code is well documented
  • Check if the behavior is what is expected
  • Check if the code is well tested
  • Check if the code is readable and well formatted
  • Additional checks (document below if any)
  • Check if documentation update issue(s) are created if the option This change requires a documentation update above is selected

Screenshots (if appropriate):

Questions (if appropriate):

@rwmcintosh rwmcintosh self-assigned this Nov 10, 2025
@rwmcintosh rwmcintosh added this to v13 Nov 10, 2025
@rwmcintosh rwmcintosh added the dev-only Assign to issues that do not need to be included in any release notes label Nov 10, 2025
@Yuri05 Yuri05 requested a review from Copilot November 10, 2025 18:46
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 modifies SQLite database connection configuration across the codebase by disabling foreign key constraints and connection pooling. The changes affect both journal database operations and project file command execution.

Key Changes

  • Disabled foreign key constraint enforcement by changing Foreign Keys=True to Foreign Keys=False in ConnectionProvider
  • Added Foreign Keys=False;Pooling=False to connection strings in SQLiteProjectCommandExecuter
  • Applied these configuration changes consistently across all SQLite connection points

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/OSPSuite.Infrastructure.Serialization/Journal/IConnectionProvider.cs Changed foreign keys from True to False and added Pooling=False to the journal database connection string
src/OSPSuite.Infrastructure.Serialization/Services/SQLiteProjectCommandExecuter.cs Added Foreign Keys=False and Pooling=False to both ExecuteCommand method overloads' connection strings

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

@@ -12,7 +12,7 @@ public class ConnectionProvider : IConnectionProvider
{
public IDbConnection CreateConnection(string databasePath)
{
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

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

Disabling foreign key constraints (Foreign Keys=False) is a significant change that removes database referential integrity protection. The database schema in JournalDatabase.cs defines foreign key constraints (e.g., fk_Parent_JournalPage, fk_Tagging_JournalPage, fk_RelatedItem_JournalPage), which will no longer be enforced.

This change should be documented with a comment explaining why foreign key enforcement needs to be disabled. If this is to resolve a specific issue (e.g., legacy database compatibility or performance), that rationale should be captured in the code.

Suggested change
{
{
// Foreign key constraints are disabled for legacy database compatibility.
// The database schema defines foreign key constraints, but enforcement is turned off
// to allow working with older database files and avoid migration issues.
// If referential integrity is required, ensure it is enforced at the application level.

Copilot uses AI. Check for mistakes.
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@rwmcintosh rwmcintosh requested a review from Yuri05 November 10, 2025 19:03
@rwmcintosh rwmcintosh merged commit 5aca8cf into V13 Nov 10, 2025
1 check passed
@rwmcintosh rwmcintosh deleted the 2702-disable-connection-pooling-and-foreign-key-enforcement branch November 10, 2025 19:09
@github-project-automation github-project-automation bot moved this to Done in v13 Nov 10, 2025
@rwmcintosh rwmcintosh restored the 2702-disable-connection-pooling-and-foreign-key-enforcement branch November 11, 2025 17:49
@rwmcintosh rwmcintosh linked an issue Nov 11, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev-only Assign to issues that do not need to be included in any release notes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Disable connection pooling and foreign key enforcement

2 participants