Node spanner pg driver

[surbhigarg92]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces @google-cloud/spanner-pg, a Node.js pg-compatible driver wrapper for Google Cloud Spanner, implementing Client, Pool, Query, Codec, and Error mapping layers alongside comprehensive unit tests. The code review identified several critical issues that must be addressed before merging. Most notably, a hardcoded local file path dependency in package.json will break CI/CD, and potential runtime crashes exist due to unhandled error events, missing BigInt support, unsafe array decoding, fragile SQL command inference, and potential TypeErrors when handling primitive errors or mismatched metadata fields.

[gemini-code-assist]

The dependency spannerlib-node points to a hardcoded local file path on your machine (file:/Users/gargsurbhi/...). This will cause installation and build failures in CI/CD environments and for other developers. Please replace this with a published version or a workspace reference.

Suggested change

	"dependencies": {
	"spannerlib-node": "file:/Users/gargsurbhi/Work/Github/go-sql-spanner/spannerlib/wrappers/spannerlib-node",
	"@google-cloud/spanner": "^8.7.1"
	},
	"dependencies": {
	"spannerlib-node": "^0.1.0",
	"@google-cloud/spanner": "^8.7.1"
	},

[gemini-code-assist]

If this.ending is true and query() is called, emitting an 'error' event when there are no listeners registered on the Query instance will throw an unhandled exception and crash the Node.js process. It is safer to check if there are active listeners before emitting the error.

Suggested change

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

[gemini-code-assist]

The encodeValue function does not support BigInt values. Modern Node.js applications frequently use BigInt for 64-bit integers (e.g., database INT8 columns). Passing a BigInt parameter will currently throw an unsupported type error. Adding explicit support for bigint prevents this.

Suggested change

	if (typeof val === 'number') {
	if (Number.isInteger(val)) {
	return {
	valueProto: {stringValue: val.toString()},
	typeProto: {code: TypeCode.INT64},
	};
	} else {
	return {
	valueProto: {numberValue: val},
	typeProto: {code: TypeCode.FLOAT64},
	};
	}
	}
	if (typeof val === 'bigint') {
	return {
	valueProto: {stringValue: val.toString()},
	typeProto: {code: TypeCode.INT64},
	};
	}
	if (typeof val === 'number') {
	if (Number.isInteger(val)) {
	return {
	valueProto: {stringValue: val.toString()},
	typeProto: {code: TypeCode.INT64},
	};
	} else {
	return {
	valueProto: {numberValue: val},
	typeProto: {code: TypeCode.FLOAT64},
	};
	}
	}

[gemini-code-assist]

When decoding TypeCode.ARRAY, if rawVal is a plain JavaScript array, accessing rawVal.values will return Array.prototype.values (a function), which will cause rawVal.values.map to throw a TypeError. Checking if rawVal is already an array or safely extracting the values array prevents potential runtime crashes.

Suggested change

	case TypeCode.ARRAY: {
	if (!rawVal || !rawVal.values) return [];
	const elemType = typeProto.arrayElementType || {
	code: TypeCode.TYPE_CODE_UNSPECIFIED,
	};
	return rawVal.values.map((v: any) => decodeValue(v, elemType));
	}
	case TypeCode.ARRAY: {
	const values = Array.isArray(rawVal)
	? rawVal
	: (rawVal && Array.isArray(rawVal.values) ? rawVal.values : null);
	if (!values) return [];
	const elemType = typeProto.arrayElementType || {
	code: TypeCode.TYPE_CODE_UNSPECIFIED,
	};
	return values.map((v: any) => decodeValue(v, elemType));
	}

[gemini-code-assist]

The current command inference logic checks if the SQL query starts with keywords like INSERT, UPDATE, etc. However, if the query starts with comments (e.g., -- comment or /* comment */), the check will fail and default to SELECT. Stripping leading comments before checking the keyword makes command inference much more robust.

Suggested change

	let command = 'SELECT';
	const trimmedSql = sqlText.trim().toUpperCase();
	if (trimmedSql.startsWith('INSERT')) command = 'INSERT';
	else if (trimmedSql.startsWith('UPDATE')) command = 'UPDATE';
	else if (trimmedSql.startsWith('DELETE')) command = 'DELETE';
	else if (trimmedSql.startsWith('CREATE')) command = 'CREATE';
	else if (trimmedSql.startsWith('DROP')) command = 'DROP';
	// Inferred PG command
	let command = 'SELECT';
	const cleanSql = sqlText.replace(/\/\*[\s\S]*?\*\/|--.*$/gm, '').trim().toUpperCase();
	if (cleanSql.startsWith('INSERT')) command = 'INSERT';
	else if (cleanSql.startsWith('UPDATE')) command = 'UPDATE';
	else if (cleanSql.startsWith('DELETE')) command = 'DELETE';
	else if (cleanSql.startsWith('CREATE')) command = 'CREATE';
	else if (cleanSql.startsWith('DROP')) command = 'DROP';

[gemini-code-assist]

If there is a mismatch between the number of fields in metadata.rowType.fields and the values in listValue.values, accessing metadata.rowType.fields[colIdx] could return undefined, leading to a TypeError when accessing .type. Adding a guard to ensure the field is defined prevents this.

                  const field: any = metadata.rowType.fields[colIdx];
                  if (!field) continue;
                  const fieldType: any = field.type;
                  const valProto: any = listValue.values[colIdx];
                  rowData.push(decodeValue(valProto, fieldType));

[gemini-code-assist]

If there is a mismatch between the number of fields in metadata.rowType.fields and the values in listValue.values, accessing metadata.rowType.fields[colIdx] could return undefined, leading to a TypeError when accessing .name or .type. Adding a guard to ensure the field is defined prevents this.

                  const field: any = metadata.rowType.fields[colIdx];
                  if (!field) continue;
                  const fieldName: string = field.name || '';
                  const fieldType: any = field.type;
                  const valProto: any = listValue.values[colIdx];
                  rowData[fieldName] = decodeValue(valProto, fieldType);

[gemini-code-assist]

If a non-Error primitive (such as a string or number) is thrown and passed to enrichPgError, attempting to set properties like severity or code on it will throw a TypeError. Wrapping non-Error primitives in an Error object first ensures safe property assignment.

export function enrichPgError(err: any): PgError {
  if (!err) {
    err = new Error('Unknown error');
  } else if (typeof err === 'string') {
    err = new Error(err);
  } else if (!(err instanceof Error)) {
    err = new Error(String(err));
  }

  const pgErr = err as PgError;

[gemini-code-assist]

If google.spanner.v1.TypeCode is undefined (e.g., due to differences in how the protobuf module is bundled or loaded), accessing it directly will throw a TypeError. Initializing TypeCode with a safe fallback object containing standard Spanner type codes ensures robust execution.

import * as pkg from '@google-cloud/spanner/build/protos/protos.js';
const google = pkg.google || (pkg as any).default?.google;
const TypeCode = google?.spanner?.v1?.TypeCode || {
  TYPE_CODE_UNSPECIFIED: 0,
  BOOL: 1,
  INT64: 2,
  FLOAT64: 3,
  TIMESTAMP: 4,
  DATE: 5,
  STRING: 6,
  BYTES: 7,
  ARRAY: 8,
  STRUCT: 9,
  NUMERIC: 10,
  JSON: 11,
};