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)?

[surbhigarg92]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements the core components of the Google Cloud Spanner Node.js Driver, including the Client, Pool, and Query classes, along with comprehensive unit tests. The feedback highlights several important issues and improvement opportunities: Pool.query fails to copy rowMode and types properties to the internal query; normalizeQueryArgs ignores separate values or callbacks when a Query instance is passed; Client.connect should check connection status early to avoid redundant promise allocations; query validation should be performed synchronously to prevent queue pollution; and Query.setPromise should clear its resolver after use to avoid memory retention.

[olavloite]

LGTM, with two minor issues. Can we also add these tests to verify that the issues are fixed:

// 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} from '../../src/index.js';

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

  it('should emit end event on Pool.query() only after client.release() completes', async () => {
    // Currently fails because pool.query() emits the 'end' event inside the try block
    // before `finally { await client.release(); }` runs.
    // Fix: Move `query.emit('end', result!)` after `finally { await client.release(); }`.
    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;
    };

    let releaseStatusWhenEndEmitted = false;
    await new Promise<void>((resolve, reject) => {
      const q = pool.query('SELECT 1');
      q.on('end', () => {
        releaseStatusWhenEndEmitted = releaseCompleted;
      });
      q.then(() => setTimeout(resolve, 50)).catch(reject);
    });

    assert.strictEqual(
      releaseStatusWhenEndEmitted,
      true,
      'client.release() should complete before end event is emitted on pool.query()',
    );
  });

  it('should emit error event on validation error even when listener is attached after client.query() returns', async () => {
    // Currently fails because dispatchQueryError() checks `query.listenerCount('error')`
    // synchronously during `client.query('')`, before callers attach `.on('error', ...)` on the next line.
    // Fix: In dispatchQueryError(), wrap the event emission in `process.nextTick(...)` when callback is undefined.
    const client = new Client(config);
    let errorEventEmitted = false;

    await new Promise<void>(resolve => {
      const q = client.query(''); // empty SQL triggers validation error
      q.on('error', () => {
        errorEventEmitted = true;
        resolve();
      });
      setTimeout(resolve, 50);
    });

    assert.strictEqual(
      errorEventEmitted,
      true,
      'error event should be emitted even when listener is attached immediately after client.query() returns',
    );
  });
});

[olavloite]

This should probably be moved to after the try-catch-finally block. Otherwise, this 'end' event will be emitted before the client has been released. So like this:

try {
  result = await client.query(queryForClient);
} catch (err: unknown) {
  queryErr = err instanceof Error ? err : new Error(String(err));
} finally {
  await client.release();
}

if (queryErr) {
  dispatchQueryError(queryErr, query, actualCallback);
  throw queryErr;
}

query.emit('end', result!);
if (actualCallback) {
  process.nextTick(() => actualCallback!(null, result!));
}

[olavloite]

This should probably be rewritten as:

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

The reason is that a user that executes a query like this would otherwise miss the error:

const q = client.query(''); // throws validation error synchronously inside query()
q.on('error', err => { ... }); // listener is attached on the next line, missing the error, as the event has been emitted