Skip to content

Remove statement context#5181

Open
Kanasjnr wants to merge 2 commits intostellar:masterfrom
Kanasjnr:remove-statement-context
Open

Remove statement context#5181
Kanasjnr wants to merge 2 commits intostellar:masterfrom
Kanasjnr:remove-statement-context

Conversation

@Kanasjnr
Copy link

Description

Resolves #5171

This PR removes the redundant StatementContext class and refactors the codebase to use soci::statement handles directly.

Rationale

StatementContext was originally an RAII wrapper for std::shared_ptr<soci::statement>, primarily serving the prepared statement cache. Since the cache was removed in #5094, the custom wrapper no longer provides additional benefit. soci::statement is its own handle-based class with internal reference counting, making it the idiomatic way to manage SQL statements when no custom metadata is required.

Changes

  • Deleted StatementContext definition from src/database/Database.h.
  • Updated Database::getPreparedStatement to return soci::statement by value.
  • Refactored all call sites (Ledger, Herder, Overlay, Database, and Tests) to use the soci::statement directly, removing all .statement() calls.
  • Verified that resource lifecycle and cleanup are correctly handled by the underlying SOCI library.

Checklist

  • Reviewed the contributing document
  • Rebased on top of master (no merge commits)
  • Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
  • Compiles
  • Ran all tests
  • If change impacts performance, include supporting evidence per the performance document

Verification Proof

  • Database Tests: src/stellar-core test "[db]" PASSED.
  • Ledger Tests: src/stellar-core test "[ledger]" PASSED.
  • Herder Tests: Persistence tests for dead node detection PASSED.
  • Grep Audit: Confirmed 0 remaining instances of StatementContext in src/.

Copilot AI review requested due to automatic review settings March 14, 2026 01:06
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 was unable to review this pull request because the user who requested the review has reached their quota limit.

@Kanasjnr
Copy link
Author

@drebelsky Kinfdly help review

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 removes the redundant StatementContext wrapper and standardizes prepared-statement usage across Stellar Core by returning and using soci::statement handles directly from Database::getPreparedStatement.

Changes:

  • Deleted StatementContext and changed Database::getPreparedStatement to return soci::statement by value.
  • Refactored SQL call sites to operate directly on the returned soci::statement (remove .statement() indirection).
  • Updated affected helper function signatures (e.g., offer-loading helpers) to accept soci::statement&.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/overlay/PeerManager.cpp Updated peer DB queries to use soci::statement directly.
src/overlay/BanManagerImpl.cpp Updated ban table queries to use soci::statement directly.
src/main/PersistentState.cpp Updated persistent state DB reads/writes to use soci::statement directly.
src/ledger/LedgerTxnOfferSQL.cpp Updated offer SQL loading/upsert/delete paths and helper signatures to use soci::statement directly.
src/ledger/LedgerTxnImpl.h Updated declarations to match new soci::statement& offer-loading signatures.
src/ledger/LedgerHeaderUtils.cpp Updated ledger header insert/load statements to use soci::statement directly.
src/herder/test/HerderTests.cpp Updated test query iteration to use soci::statement directly.
src/herder/HerderPersistenceImpl.cpp Updated SCP history persistence queries to use soci::statement directly.
src/database/test/DatabaseTests.cpp Updated migration test helpers/queries to use soci::statement directly.
src/database/Database.h Removed StatementContext; changed getPreparedStatement API to return soci::statement.
src/database/Database.cpp Implemented getPreparedStatement returning a prepared soci::statement by value.

@Kanasjnr
Copy link
Author

@drebelsky pinging you on this again

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.

Remove StatementContext

2 participants