chore(spanner-driver): add Client and Pool classes with Query support

[surbhigarg92]

Summary

This PR implements the core Client, Pool, and Query execution interfaces for @google-cloud/spanner-driver, providing compatibility with node-postgres (pg) driver layer for Google Cloud Spanner

---

Key Changes

1. Client Connection & Execution Queue (src/lib/client.ts)

• Client Class: Implements node-postgres compatible Client handle managing connection state (isConnected), transaction status tracking (txStatus: 'I' | 'T' | 'E'), and DSN resolution.
• Sequential Task Queue (queryQueue): Enforces sequential query execution order per client connection handle.
• Transaction Status Lifecycle: Updates txStatus = 'I' only after statement execution completes (COMMIT, ROLLBACK, ABORT).
• release() Method: Added client.release() method delegating to connection teardown for node-postgres compatibility.
• Unhandled Error Prevention: Added query.listenerCount('error') > 0 check prior to emitting 'error' events, preventing Node process crashes when queries are consumed via Promises (await client.query()).

2. Query Class & Thenable / EventEmitter Integration (src/lib/query.ts)

• Dual Invocation Model: Extends EventEmitter for row streaming (.on('row', cb), .on('end', cb)) while implementing the Thenable interface (then, catch, finally) for async/await support.
• Overload Support: Supports query strings, QueryConfig objects, Query instances, positional value arrays ($1, $2), and Node callbacks ((err, res) => void).
• Constructor Robustness:
  • Added text !== null guard when initializing from objects (typeof text === 'object' && text !== null).
  • Added support for overriding positional values and callback when instantiating from an existing Query instance.

3. Pool Scaffolding & Callback Single-Invocation Rule (src/lib/pool.ts)

• Pool Class: Implements connect(), query(), and end().
• Client Binding: Binds client.release = client.end.bind(client) during client acquisition with TODO(PR 4 - Connection Pooling) markers for pool recycling in PR 4.
• Single Invocation Guarantee: Isolated _doConnect() connection acquisition error handling from query execution, ensuring connection failures call callbacks exactly once without hanging or double callbacks.
• 3rd-Argument Callback Overload Resolution: Resolved 3rd-argument callback parameters when executing pool.query(query, values, callback) or client.query(query, values, callback).

[gemini-code-assist]

Code Review

This pull request introduces a high-performance, node-postgres compatible Node.js driver for Google Cloud Spanner, implementing Client, Pool, and Query classes along with comprehensive unit tests. The review feedback highlights several key issues, including transaction status (txStatus) not resetting on completion, potential Node.js process crashes from unhandled 'error' events, hanging callback-based queries on connection failures, and a naive semicolon-splitting approach for SQL commands. Additionally, suggestions were made to avoid redundant error enrichment, add a .release() method to clients for full node-postgres compatibility, and prevent a runtime crash when query text is null.

[surbhigarg92]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces the initial implementation of the Google Cloud Spanner Node.js Driver, providing Client, Pool, and Query classes compatible with the node-postgres API, along with unit tests. The review feedback highlights several issues: SQL command and transaction status parsing in Client can fail if queries contain leading comments; both Client.query and Pool.query ignore the third callback argument when a Query instance is passed; the Query constructor ignores overridden values or callback arguments when instantiated from an existing Query instance; and a unit test in pool_test.ts deletes process.env.GOOGLE_CLOUD_PROJECT without restoring it, leading to potential test pollution.

[olavloite]

These tests show the potential failures/corner cases pointed out in the various comments:

// Copyright 2026 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//      http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import * as assert from 'assert';
import {describe, it} from 'mocha';
import {Client, Pool, Query} from '../../src/index.js';

describe('PR #9023 Regression Tests', () => {
  const config = {
    project: 'p',
    instance: 'i',
    database: 'd',
  };

  // ============================================================================
  // Section 2: Correctness Errors in the Implementation
  // ============================================================================

  it('2A: should invoke callback and NOT emit error event when callback is provided on validation error', async () => {
    // Currently fails because validation errors in client.ts emit both an 'error' event
    // and invoke the callback, violating mutual exclusion.
    const client = new Client(config);
    let errorEventEmitted = false;
    let callbackInvoked = false;

    const q = new Query('');
    q.on('error', () => {
      errorEventEmitted = true;
    });

    await new Promise<void>(resolve => {
      client.query(q, undefined, () => {
        callbackInvoked = true;
        setTimeout(resolve, 20);
      });
    });

    assert.strictEqual(
      errorEventEmitted,
      false,
      'error event should not be emitted when callback is provided',
    );
    assert.strictEqual(callbackInvoked, true);
  });

  it('2B: should not throw TypeError when calling catch() or then() on a newly constructed Query', () => {
    // Currently fails with "TypeError: Cannot read properties of undefined (reading 'catch')"
    // because `this.promise` is declared on Query but left uninitialized in constructor.
    const q = new Query('SELECT 1');
    assert.doesNotThrow(() => {
      q.catch(() => {});
    });
  });

  // ============================================================================
  // Section 3: Potential Edge Cases That Will Cause Unexpected Errors
  // ============================================================================

  it('3A: should reject queries executed after client.end() without reconnecting', async () => {
    // Currently fails because task.run() checks `if (!this.isConnected) { await this.connect(); }`
    // and automatically reconnects an ended client instead of rejecting.
    const client = new Client(config);
    await client.connect();
    assert.strictEqual(client.isConnected, true);
    await client.end();
    assert.strictEqual(client.isConnected, false);

    try {
      await client.query('SELECT 1');
      assert.fail('Should have thrown an error when querying an ended client');
    } catch (err: unknown) {
      assert.strictEqual(client.isConnected, false);
      assert.match(
        (err as Error).message,
        /Client has already been connected|Connection terminated|Client was closed/,
      );
    }
  });

  it('3B(i): should parse command verb as SELECT for CTE WITH queries', async () => {
    // Currently fails because command extraction (`cleanUpper.replace(/^[^A-Z]+/, '').split(/\s+/)[0]`)
    // returns 'WITH' instead of the underlying statement verb 'SELECT'.
    const client = new Client(config);
    const res = await client.query('WITH cte AS (SELECT 1) SELECT * FROM cte');
    assert.strictEqual(res.command, 'SELECT');
    await client.end();
  });

  it('3B(ii): should not transition txStatus to T when statement starts with BEGIN identifier prefix', async () => {
    // Currently fails because `cleanUpper.startsWith('BEGIN')` lacks word-boundary checks
    // and matches identifiers like 'BEGIN_LOG(1)'.
    const client = new Client(config);
    await client.query('BEGIN_LOG(1)');
    assert.strictEqual(
      client.txStatus,
      'I',
      'txStatus should remain I for regular queries starting with BEGIN prefix',
    );
    await client.end();
  });

  it('3C: should resolve a Query instance passed to pool.query() only after client.release() completes', async () => {
    // Currently fails because `client.query(q)` overwrites `q.promise` with client.query's promise,
    // causing `await q` to resolve BEFORE `pool.query`'s `finally { await client.release(); }` finishes.
    const pool = new Pool(config);
    let releaseCompleted = false;

    const origConnect = pool['_doConnect'].bind(pool);
    pool['_doConnect'] = async () => {
      const c = await origConnect();
      const origRelease = c.release.bind(c);
      c.release = async () => {
        await new Promise(r => setTimeout(r, 40));
        releaseCompleted = true;
        return origRelease();
      };
      return c;
    };

    const q = new Query('SELECT 1');
    pool.query(q);

    // Give client.query time to execute and potentially overwrite q.promise
    await new Promise(r => setTimeout(r, 10));
    await q;

    assert.strictEqual(
      releaseCompleted,
      true,
      'client.release() should complete before the Query promise resolves',
    );
  });

  // ============================================================================
  // Section 4: Race Conditions and Concurrency / Lifecycle Issues
  // ============================================================================

  it('4B: should deduplicate concurrent connect() calls and initiate connection exactly once', async () => {
    // Currently fails when _doConnect is asynchronous because `if (this.isConnected) return;`
    // is not guarded against concurrent connection establishment attempts.
    const client = new Client(config);
    let connectInvocations = 0;

    // Simulate async connection establishment in PR 4
    client['_doConnect'] = async () => {
      if (client.isConnected) return;
      connectInvocations++;
      await new Promise(r => setTimeout(r, 20));
      client.isConnected = true;
    };

    await Promise.all([client.connect(), client.connect(), client.connect()]);

    assert.strictEqual(
      connectInvocations,
      1,
      'concurrent connect() calls should only initiate connection once',
    );
  });
});

Can we add these tests (at least for the cases where we decide to keep the logic in Node.js and not move it to the shared library)?

[olavloite]

super-nit: we should try as much as possible to use the name 'Google Spanner' or just 'Spanner'. This driver should also work with Omni.

[olavloite]

nit: PostgreSQL will put the transaction status into 'E' for any error. Even simple syntax errors in a SHOW statement in a transaction will cause the transaction to enter into the Error state, and be unusable from that point.

[olavloite]

This is something that preferably should be delegated to the shared library. Is this copied from node-pg?

[olavloite]

This also should preferably be handled by the library.

One potential bug here is that we support the statements START BATCH DDL|DML, RUN BATCH and ABORT BATCH. Those statements will interfere with this logic.

[olavloite]

There is a potential (minor) bug here: You are setting the status to 'T' here based on the assumption that the statement will start a transaction. However, if the statement for example is begin my weird transaction, then it will fail, and the catch block below will transition the state to 'E'. That is not 100% correct, as no transaction was ever started, and the actual state should have been 'I'.

[olavloite]

I think that this should use an else if to prevent potential duplicate calls to both query.emit(..) and the callback (if the user has both passed a callback and attached an error listener on the query)

Suggested change

	if (query.listenerCount('error') > 0) {
	query.emit('error', err);
	}
	if (actualCallback) {
	process.nextTick(() => actualCallback!(err));
	}
	if (query.listenerCount('error') > 0) {
	query.emit('error', err);
	} else if (actualCallback) {
	process.nextTick(() => actualCallback!(err));
	}

[olavloite]

Same here: this should probably use an else if

[olavloite]

This should probably also clear any pending queries in the queue. Otherwise, those queries will still try to execute using this client, and that again will cause the client to reconnect.

[olavloite]

This is also something that should preferably be moved to the shared library. This will for example return WITH for queries that use a common table expression. However, the correct command that PostgreSQL would return in that case is the actual command (SELECT, INSERT, UPDATE, DELETE).

Note that currently in Spanner, common table expressions can only be used with queries, so any query that starts with WITH can be assumed to be a SELECT (for now).

[olavloite]

According to Gemini:

• Lifecycle Race: When client.query(...) finishes, it invokes actualCallback!(null, result) via process.nextTick (client.ts:L218). In the Node.js event loop, nextTick callbacks run before Promise microtasks.
• Impact: The user callback ((err, res) => { ... }) will execute before finally { await client.release(); } runs. If the callback immediately executes another query against the pool (pool.query('SELECT 2', ...)), the first client has not yet been released back to the pool. When connection pool recycling is implemented in PR 4, this order of execution could cause unexpected pool exhaustion or connection growth.