Skip to content

Document Session interface and Pool convenience methods#460

Open
EchoEllet wants to merge 2 commits into
isoos:masterfrom
EchoEllet:docs/session-doc-comments
Open

Document Session interface and Pool convenience methods#460
EchoEllet wants to merge 2 commits into
isoos:masterfrom
EchoEllet:docs/session-doc-comments

Conversation

@EchoEllet

Copy link
Copy Markdown
  • Documents the Session interface

    Reason: When I first saw Session, I assumed this might be "a PostgreSQL backend session"

    If that was true, then every implementation of Session would need to correspond to exactly one backend session.

    But Pool implements Session:

     abstract class Pool<L> implements Session, SessionExecutor

    Pool is lazy by design and does not acquire a connection on Pool construction (Pool.withEndpoints is a factory method, not a static async method like Connection.open).

    When running Pool.execute() (execute() is from Session),
    the pool first acquires a connection and then uses the connection.

    So Pool itself is not a session, and in this case Session is a capability interface that allows executing SQL statements, but not every implementation is the session itself.

    This changes attempts to fix this confusion by documenting what Session interface represents in this codebase (it is not necessarily a PostgreSQL physical database connection on its own).

  • Explains why Pool implements Session and SessionExecutor

    This is the other half of the fix, which attempts to explain why Pool implements Session and SessionExecutor. It seems it is a convenience API (example) that internally acquires a pooled connection via [withConnection] for each operation (except SessionExecutor.close()).

    The current Pool implementation code
     // PoolImplementation class
     
    @override
    Future<Result> execute(
      Object query, {
      Object? parameters,
      bool ignoreRows = false,
      QueryMode? queryMode,
      Duration? timeout,
    }) {
      return withConnection(
        (connection) => connection.execute(
          query,
          parameters: parameters,
          ignoreRows: ignoreRows,
          queryMode: queryMode,
          timeout: timeout,
        ),
      );
    }
    
     @override
    Future<R> run<R>(
      Future<R> Function(Session session) fn, {
      SessionSettings? settings,
      L? locality,
    }) {
      return withConnection(
        (connection) => connection.run(fn, settings: settings),
        locality: locality,
      );
    }
    
    @override
    Future<R> runTx<R>(
      Future<R> Function(TxSession session) fn, {
      TransactionSettings? settings,
      L? locality,
    }) {
      return withConnection(
        (connection) => connection.runTx(fn, settings: settings),
        locality: locality,
      );
    }
    
    // Same goes for prepare
    

Note

Keep in mind that I'm not yet knowledgeable when it comes to database drivers and complex database operation designs. So the correction might be inaccurate or misleading. In that case, the PR should be either closed or fixed.

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.

1 participant