From ffd2d22925591a484f7c52c107c26ff5a638ce2c Mon Sep 17 00:00:00 2001 From: Suyog Sonwalkar Date: Fri, 29 Aug 2025 17:24:37 -0700 Subject: [PATCH] Adding type hint capability for spanner dialect Summary: Test Plan: --- src/spanner/adapter.ts | 653 +++++++++++++++--------- tasks/fix-json-types.md | 138 +++++ test/spanner/adapter-type-hints.test.ts | 595 +++++++++++++++++++++ 3 files changed, 1143 insertions(+), 243 deletions(-) create mode 100644 tasks/fix-json-types.md create mode 100644 test/spanner/adapter-type-hints.test.ts diff --git a/src/spanner/adapter.ts b/src/spanner/adapter.ts index 82cb57a..466f171 100644 --- a/src/spanner/adapter.ts +++ b/src/spanner/adapter.ts @@ -18,16 +18,16 @@ type SpannerClientInstanceType = import("@google-cloud/spanner").Spanner; // Local interface to represent the structure of google.spanner.v1.IType // This helps with type checking without needing a direct runtime import of 'google'. -// interface SpannerParamType { -// code: string | number; // Corresponds to google.spanner.v1.TypeCode -// arrayElementType?: SpannerParamType | null; // Corresponds to google.spanner.v1.IType -// structType?: { -// fields: Array<{ -// name?: string | null; -// type?: SpannerParamType | null; // Corresponds to google.spanner.v1.IType -// }>; -// } | null; // Corresponds to google.spanner.v1.IStructType -// } +interface SpannerParamType { + code: string | number; // Corresponds to google.spanner.v1.TypeCode + arrayElementType?: SpannerParamType | null; // Corresponds to google.spanner.v1.IType + structType?: { + fields: Array<{ + name?: string | null; + type?: SpannerParamType | null; // Corresponds to google.spanner.v1.IType + }>; + } | null; // Corresponds to google.spanner.v1.IStructType +} import type { DatabaseAdapter, @@ -40,121 +40,242 @@ import type { PreparedQuery, TableConfig } from "../types/common.js"; // Correct import { shapeResults } from "../core/result-shaper.js"; // Corrected path // Helper function to map DDL type strings to Spanner TypeCodes -// function mapDdlTypeToSpannerCode(ddlType: string): string { -// const upperType = ddlType.toUpperCase(); -// if ( -// upperType.startsWith("STRING") || -// upperType === "TEXT" || -// upperType === "UUID" || -// upperType.startsWith("VARCHAR") -// ) { -// return "STRING"; -// } -// if ( -// upperType.startsWith("INT") || -// upperType === "BIGINT" || -// upperType === "INTEGER" || -// upperType === "SERIAL" || -// upperType === "BIGSERIAL" || -// upperType === "SMALLINT" || -// upperType === "INT64" -// ) { -// return "INT64"; -// } -// if (upperType === "BOOLEAN" || upperType === "BOOL") { -// return "BOOL"; -// } -// if ( -// upperType.startsWith("FLOAT") || -// upperType === "DOUBLE" || -// upperType === "REAL" || -// upperType === "DOUBLE PRECISION" || -// upperType === "FLOAT64" -// ) { -// return "FLOAT64"; -// } -// if (upperType.startsWith("NUMERIC") || upperType.startsWith("DECIMAL")) { -// return "NUMERIC"; -// } -// if (upperType === "DATE") { -// return "DATE"; -// } -// if (upperType.startsWith("TIMESTAMP")) { -// // Covers TIMESTAMP and TIMESTAMPTZ -// return "TIMESTAMP"; -// } -// if (upperType.startsWith("JSON")) { -// // Covers JSON and JSONB -// return "JSON"; -// } -// if (upperType === "BYTES" || upperType === "BYTEA") { -// return "BYTES"; -// } -// // If the type is already a valid Spanner TypeCode, pass it through. -// // This handles cases where the hint might already be in the correct format. -// const validSpannerTypeCodes = [ -// "STRING", -// "INT64", -// "BOOL", -// "FLOAT64", -// "TIMESTAMP", -// "DATE", -// "BYTES", -// "ARRAY", -// "STRUCT", -// "NUMERIC", -// "JSON", -// ]; -// if (validSpannerTypeCodes.includes(upperType)) { -// return upperType; -// } - -// console.warn( -// `Unknown DDL type for Spanner mapping: ${ddlType}. Defaulting to STRING.` -// ); -// return "STRING"; -// } +function mapDdlTypeToSpannerCode(ddlType: string): string { + const upperType = ddlType.toUpperCase(); + if ( + upperType.startsWith("STRING") || + upperType === "TEXT" || + upperType === "UUID" || + upperType.startsWith("VARCHAR") + ) { + return "STRING"; + } + if ( + upperType.startsWith("INT") || + upperType === "BIGINT" || + upperType === "INTEGER" || + upperType === "SERIAL" || + upperType === "BIGSERIAL" || + upperType === "SMALLINT" || + upperType === "INT64" + ) { + return "INT64"; + } + if (upperType === "BOOLEAN" || upperType === "BOOL") { + return "BOOL"; + } + if ( + upperType.startsWith("FLOAT") || + upperType === "DOUBLE" || + upperType === "REAL" || + upperType === "DOUBLE PRECISION" || + upperType === "FLOAT64" + ) { + return "FLOAT64"; + } + if (upperType.startsWith("NUMERIC") || upperType.startsWith("DECIMAL")) { + return "NUMERIC"; + } + if (upperType === "DATE") { + return "DATE"; + } + if (upperType.startsWith("TIMESTAMP")) { + // Covers TIMESTAMP and TIMESTAMPTZ + return "TIMESTAMP"; + } + if (upperType.startsWith("JSON")) { + // Covers JSON and JSONB + return "JSON"; + } + if (upperType === "BYTES" || upperType === "BYTEA") { + return "BYTES"; + } + // If the type is already a valid Spanner TypeCode, pass it through. + // This handles cases where the hint might already be in the correct format. + const validSpannerTypeCodes = [ + "STRING", + "INT64", + "BOOL", + "FLOAT64", + "TIMESTAMP", + "DATE", + "BYTES", + "ARRAY", + "STRUCT", + "NUMERIC", + "JSON", + ]; + if (validSpannerTypeCodes.includes(upperType)) { + return upperType; + } + + console.warn( + `Unknown DDL type for Spanner mapping: ${ddlType}. Defaulting to STRING.` + ); + return "STRING"; +} // Helper function to transform DDL hints to Spanner paramTypes object -// function transformDdlHintsToParamTypes( -// ddlHints?: Record -// ): Record | undefined { -// if (!ddlHints) { -// return undefined; -// } -// const paramTypes: Record = {}; -// for (const key in ddlHints) { -// if (Object.prototype.hasOwnProperty.call(ddlHints, key)) { -// const typeCodeString = mapDdlTypeToSpannerCode(ddlHints[key]); -// // Construct an object conforming to our local SpannerParamType interface, -// // which is structurally compatible with google.spanner.v1.IType. -// paramTypes[key] = { -// code: typeCodeString, // mapDdlTypeToSpannerCode returns a string like "STRING" -// arrayElementType: null, // Assuming scalar types for now -// structType: null, // Assuming scalar types for now -// }; -// } -// } -// return paramTypes; -// } - -// function transformDdlHintsToTypes( -// ddlHints?: Record -// ): Record | undefined { -// if (!ddlHints) { -// return undefined; -// } -// const types: Record = {}; -// for (const key in ddlHints) { -// if (Object.prototype.hasOwnProperty.call(ddlHints, key)) { -// const typeCodeString = mapDdlTypeToSpannerCode(ddlHints[key]); -// // Construct an object conforming to our local SpannerParamType interface, -// // which is structurally compatible with google.spanner.v1.IType. -// types[key] = typeCodeString; // mapDdlTypeToSpannerCode returns a string like "STRING" -// } -// } -// return types; -// } +function transformDdlHintsToParamTypes( + ddlHints?: Record +): Record | undefined { + if (!ddlHints) { + return undefined; + } + const paramTypes: Record = {}; + for (const key in ddlHints) { + if (Object.prototype.hasOwnProperty.call(ddlHints, key)) { + const typeCodeString = mapDdlTypeToSpannerCode(ddlHints[key]); + // Construct an object conforming to our local SpannerParamType interface, + // which is structurally compatible with google.spanner.v1.IType. + paramTypes[key] = { + code: typeCodeString, // mapDdlTypeToSpannerCode returns a string like "STRING" + arrayElementType: null, // Assuming scalar types for now + structType: null, // Assuming scalar types for now + }; + } + } + return paramTypes; +} + +function transformDdlHintsToTypes( + ddlHints?: Record +): Record | undefined { + if (!ddlHints) { + return undefined; + } + const types: Record = {}; + for (const key in ddlHints) { + if (Object.prototype.hasOwnProperty.call(ddlHints, key)) { + const typeCodeString = mapDdlTypeToSpannerCode(ddlHints[key]); + // Construct an object conforming to our local SpannerParamType interface, + // which is structurally compatible with google.spanner.v1.IType. + types[key] = typeCodeString; // mapDdlTypeToSpannerCode returns a string like "STRING" + } + } + return types; +} + +// Helper function to clean JSON data before sending to Spanner +function cleanJsonForSpanner(value: any): any { + if (value === null || value === undefined) { + return null; + } + + if (typeof value === 'object' && !Array.isArray(value) && !(value instanceof Date)) { + const cleaned: any = {}; + for (const key in value) { + if (Object.prototype.hasOwnProperty.call(value, key)) { + if (value[key] !== undefined) { + cleaned[key] = cleanJsonForSpanner(value[key]); + } + // Skip undefined values entirely + } + } + return cleaned; + } + + if (Array.isArray(value)) { + return value.map(item => cleanJsonForSpanner(item)); + } + + return value; +} + +// Helper to clean params that might contain JSON +function cleanParamsForSpanner( + params?: Record, + typeHints?: Record +): Record | undefined { + if (!params) return undefined; + + const cleaned: Record = {}; + for (const key in params) { + if (Object.prototype.hasOwnProperty.call(params, key)) { + const hint = typeHints?.[key]; + // Clean JSON fields + if (hint && mapDdlTypeToSpannerCode(hint) === 'JSON') { + cleaned[key] = cleanJsonForSpanner(params[key]); + } else { + cleaned[key] = params[key]; + } + } + } + return cleaned; +} + +// Helper function to provide better error messages +function enhanceSpannerError(error: any, params?: Record): Error { + const errorMessage = error.message || ''; + + if (errorMessage.includes('The code field is required for types')) { + // Check for undefined values in params + const hasUndefinedInJson = checkForUndefinedInJsonParams(params); + if (hasUndefinedInJson) { + return new Error( + 'Spanner Error: JSON columns cannot contain undefined values. ' + + 'Found undefined in JSON parameters. Please use null instead of undefined. ' + + `Original error: ${errorMessage}` + ); + } + + // Check for null values without types + const nullParams = findNullParams(params); + if (nullParams.length > 0) { + return new Error( + 'Spanner Error: Null values may require type information. ' + + `Parameters with null values: ${nullParams.join(', ')}. ` + + `Consider providing type hints for these parameters. ` + + `Original error: ${errorMessage}` + ); + } + } + + return error; +} + +function checkForUndefinedInJsonParams(params?: Record): boolean { + if (!params) return false; + + for (const value of Object.values(params)) { + if (hasUndefinedValue(value)) { + return true; + } + } + return false; +} + +function hasUndefinedValue(value: any): boolean { + if (value === undefined) return true; + + if (typeof value === 'object' && value !== null && !(value instanceof Date)) { + if (Array.isArray(value)) { + return value.some(item => hasUndefinedValue(item)); + } else { + for (const key in value) { + if (Object.prototype.hasOwnProperty.call(value, key)) { + if (hasUndefinedValue(value[key])) { + return true; + } + } + } + } + } + + return false; +} + +function findNullParams(params?: Record): string[] { + if (!params) return []; + + const nullParams: string[] = []; + for (const [key, value] of Object.entries(params)) { + if (value === null) { + nullParams.push(key); + } + } + return nullParams; +} export interface SpannerConnectionOptions extends ConnectionOptions { projectId: string; @@ -270,44 +391,47 @@ export class SpannerAdapter implements DatabaseAdapter { async execute( sql: string, params?: Record, - _spannerTypeHints?: Record + spannerTypeHints?: Record ): Promise { const db = this.ensureConnected(); // Relies on connect() having awaited this.ready try { + // Clean params if they contain JSON + const cleanedParams = cleanParamsForSpanner(params, spannerTypeHints); + const paramTypes = transformDdlHintsToParamTypes(spannerTypeHints) as any; + const types = transformDdlHintsToTypes(spannerTypeHints); + // Spanner's runUpdate returns an array where the first element is the affected row count. // The result of runTransactionAsync is the result of its callback. const rowCount = await db.runTransactionAsync( async (transaction: SpannerNativeTransaction) => { try { - // const paramTypes = transformDdlHintsToParamTypes( - // spannerTypeHints - // ) as any; - // const types = transformDdlHintsToTypes(spannerTypeHints); - // console.log("Before running transaction runUpdate..."); - // console.log(sql); - // console.log(params); - // console.log(types); - // console.log(paramTypes); - - const [count] = await transaction.runUpdate({ + const updateOptions: any = { sql, - params, - // types, - // paramTypes, - }); + params: cleanedParams, + }; + + // Add types if provided + if (types) { + updateOptions.types = types; + } + if (paramTypes) { + updateOptions.paramTypes = paramTypes; + } + + const [count] = await transaction.runUpdate(updateOptions); await transaction.commit(); return count; } catch (err) { console.error("Error during transaction:", err); await transaction.rollback(); - throw err; + throw enhanceSpannerError(err, params); } } ); return { count: typeof rowCount === "number" ? rowCount : 0 }; } catch (error) { console.error("Error executing command with Spanner adapter:", error); - throw error; + throw enhanceSpannerError(error, params); } } @@ -368,29 +492,34 @@ export class SpannerAdapter implements DatabaseAdapter { async query( sql: string, params?: Record, - _spannerTypeHints?: Record + spannerTypeHints?: Record ): Promise { const db = this.ensureConnected(); // Relies on connect() having awaited this.ready try { - // const paramTypes = transformDdlHintsToParamTypes(spannerTypeHints) as any; - // const types = transformDdlHintsToTypes(spannerTypeHints); - // console.log("Before running transaction runUpdate..."); - // console.log(sql); - // console.log(params); - // console.log(types); - // console.log(paramTypes); - - const [rows] = await db.run({ + // Clean params if they contain JSON + const cleanedParams = cleanParamsForSpanner(params, spannerTypeHints); + const paramTypes = transformDdlHintsToParamTypes(spannerTypeHints) as any; + const types = transformDdlHintsToTypes(spannerTypeHints); + + const queryOptions: any = { sql, - params, + params: cleanedParams, json: true, - // types, - // paramTypes, - }); + }; + + // Add types if provided + if (types) { + queryOptions.types = types; + } + if (paramTypes) { + queryOptions.paramTypes = paramTypes; + } + + const [rows] = await db.run(queryOptions); return rows as TResult[]; } catch (error) { console.error("Error executing query with Spanner adapter:", error); - throw error; + throw enhanceSpannerError(error, params); } } @@ -443,38 +572,40 @@ export class SpannerAdapter implements DatabaseAdapter { >( sql: string, params?: Record, // Spanner expects Record - _spannerTypeHints?: Record + spannerTypeHints?: Record ): Promise { const db = this.ensureConnected(); try { + // Clean params if they contain JSON + const cleanedParams = cleanParamsForSpanner(params, spannerTypeHints); + const paramTypes = transformDdlHintsToParamTypes(spannerTypeHints) as any; + const types = transformDdlHintsToTypes(spannerTypeHints); + // Use runTransactionAsync to ensure a read-write transaction return await db.runTransactionAsync( async (transaction: SpannerNativeTransaction) => { try { - // Use transaction.run() for DML with THEN RETURN - // const paramTypes = transformDdlHintsToParamTypes( - // spannerTypeHints - // ) as any; - // const types = transformDdlHintsToTypes(spannerTypeHints); - // console.log("Before running execute and return rows..."); - // console.log(sql); - // console.log(params); - // console.log(types); - // console.log(paramTypes); - - const [rows] = await transaction.run({ + const queryOptions: any = { sql, - params, + params: cleanedParams, json: true, - // types, - // paramTypes, - }); + }; + + // Add types if provided + if (types) { + queryOptions.types = types; + } + if (paramTypes) { + queryOptions.paramTypes = paramTypes; + } + + const [rows] = await transaction.run(queryOptions); await transaction.commit(); return rows as TResult[]; } catch (err) { console.error("Error during transaction:", err); await transaction.rollback(); - throw err; + throw enhanceSpannerError(err, params); } } ); @@ -483,7 +614,7 @@ export class SpannerAdapter implements DatabaseAdapter { "Error executing DML and returning rows with Spanner adapter:", error ); - throw error; + throw enhanceSpannerError(error, params); } } @@ -507,7 +638,8 @@ export class SpannerAdapter implements DatabaseAdapter { return { execute: async ( sqlCmd: string, - paramsCmd?: Record + paramsCmd?: Record, + cmdSpannerTypeHints?: Record ): Promise => { // Note: Spanner transactions are usually committed as a whole. // Running individual DMLs and then a separate commit is not the typical pattern. @@ -523,26 +655,64 @@ export class SpannerAdapter implements DatabaseAdapter { "Spanner: conceptual begin() called on transaction object" ); - // TODO: Update this to use type hints - const [rowCountFromRunUpdate] = await txObject.runUpdate({ - sql: sqlCmd, - params: paramsCmd, - }); - return { count: rowCountFromRunUpdate }; + try { + // Clean params if they contain JSON + const cleanedParams = cleanParamsForSpanner(paramsCmd, cmdSpannerTypeHints); + const paramTypes = transformDdlHintsToParamTypes(cmdSpannerTypeHints) as any; + const types = transformDdlHintsToTypes(cmdSpannerTypeHints); + + const updateOptions: any = { + sql: sqlCmd, + params: cleanedParams, + }; + + // Add types if provided + if (types) { + updateOptions.types = types; + } + if (paramTypes) { + updateOptions.paramTypes = paramTypes; + } + + const [rowCountFromRunUpdate] = await txObject.runUpdate(updateOptions); + return { count: rowCountFromRunUpdate }; + } catch (err) { + throw enhanceSpannerError(err, paramsCmd); + } }, query: async < TQuery extends AdapterQueryResultRow = AdapterQueryResultRow >( sqlQuery: string, - paramsQuery?: Record + paramsQuery?: Record, + querySpannerTypeHints?: Record ): Promise => { const txObjectQuery = spannerTx as any; - const [rows] = await txObjectQuery.run({ - sql: sqlQuery, - params: paramsQuery, - json: true, - }); - return rows as TQuery[]; + try { + // Clean params if they contain JSON + const cleanedParams = cleanParamsForSpanner(paramsQuery, querySpannerTypeHints); + const paramTypes = transformDdlHintsToParamTypes(querySpannerTypeHints) as any; + const types = transformDdlHintsToTypes(querySpannerTypeHints); + + const queryOptions: any = { + sql: sqlQuery, + params: cleanedParams, + json: true, + }; + + // Add types if provided + if (types) { + queryOptions.types = types; + } + if (paramTypes) { + queryOptions.paramTypes = paramTypes; + } + + const [rows] = await txObjectQuery.run(queryOptions); + return rows as TQuery[]; + } catch (err) { + throw enhanceSpannerError(err, paramsQuery); + } }, commit: async (): Promise => { const txObjectCommit = spannerTx as any; @@ -576,66 +746,63 @@ export class SpannerAdapter implements DatabaseAdapter { execute: async ( cmdSql, cmdParams, - _cmdSpannerTypeHints?: Record + cmdSpannerTypeHints?: Record ) => { - // const paramTypes = transformDdlHintsToParamTypes( - // cmdSpannerTypeHints - // ) as any; - // const types = transformDdlHintsToTypes(cmdSpannerTypeHints); - // console.log("Before running gcp transaction runUpdate..."); - // console.log(cmdSql); - // console.log(cmdParams); - // console.log(types); - // console.log(paramTypes); - - const [rowCount] = await gcpTransaction.runUpdate({ - sql: cmdSql, - params: cmdParams, - // types, - // paramTypes, - }); - - // const [rowCount] = await gcpTransaction.runUpdate({ - // sql: cmdSql, - // params: cmdParams as Record | undefined, - // paramTypes: transformDdlHintsToParamTypes( - // cmdSpannerTypeHints - // ) as any, - // }); - return { count: rowCount }; + try { + // Clean params if they contain JSON + const cleanedParams = cleanParamsForSpanner(cmdParams, cmdSpannerTypeHints); + const paramTypes = transformDdlHintsToParamTypes(cmdSpannerTypeHints) as any; + const types = transformDdlHintsToTypes(cmdSpannerTypeHints); + + const updateOptions: any = { + sql: cmdSql, + params: cleanedParams, + }; + + // Add types if provided + if (types) { + updateOptions.types = types; + } + if (paramTypes) { + updateOptions.paramTypes = paramTypes; + } + + const [rowCount] = await gcpTransaction.runUpdate(updateOptions); + return { count: rowCount }; + } catch (err) { + throw enhanceSpannerError(err, cmdParams); + } }, query: async ( querySql, queryParams, - _querySpannerTypeHints?: Record + querySpannerTypeHints?: Record ) => { - // const paramTypes = transformDdlHintsToParamTypes( - // querySpannerTypeHints - // ) as any; - // const types = transformDdlHintsToTypes(querySpannerTypeHints); - // console.log("Before running gcp query transaction runUpdate..."); - // console.log(querySql); - // console.log(queryParams); - // console.log(types); - // console.log(paramTypes); - - const [rows] = await gcpTransaction.run({ - sql: querySql, - params: queryParams, - json: true, - // types, - // paramTypes, - }); - - // const [rows] = await gcpTransaction.run({ - // sql: querySql, - // params: queryParams as Record | undefined, - // json: true, - // paramTypes: transformDdlHintsToParamTypes( - // querySpannerTypeHints - // ) as any, - // }); - return rows as any[]; + try { + // Clean params if they contain JSON + const cleanedParams = cleanParamsForSpanner(queryParams, querySpannerTypeHints); + const paramTypes = transformDdlHintsToParamTypes(querySpannerTypeHints) as any; + const types = transformDdlHintsToTypes(querySpannerTypeHints); + + const queryOptions: any = { + sql: querySql, + params: cleanedParams, + json: true, + }; + + // Add types if provided + if (types) { + queryOptions.types = types; + } + if (paramTypes) { + queryOptions.paramTypes = paramTypes; + } + + const [rows] = await gcpTransaction.run(queryOptions); + return rows as any[]; + } catch (err) { + throw enhanceSpannerError(err, queryParams); + } }, commit: async () => { // Commit is handled by runTransactionAsync itself. This is a no-op. diff --git a/tasks/fix-json-types.md b/tasks/fix-json-types.md new file mode 100644 index 0000000..04988d6 --- /dev/null +++ b/tasks/fix-json-types.md @@ -0,0 +1,138 @@ +# Fix JSON Type Handling and Error Messages for Spanner + +## Problem + +When Spanner encounters `undefined` values in JSON columns or `null` values without proper type information, it returns a misleading error message: + +``` +error: 3 INVALID_ARGUMENT: The code field is required for types. +``` + +This error message is confusing and doesn't clearly indicate the actual problem. + +## Actual Issues + +1. **Undefined values in JSON objects**: Spanner doesn't accept `undefined` values within JSON objects +2. **Null values without type information**: When passing `null` for nullable columns, Spanner needs type information + +## Example of the Problem + +When inserting this data: +```javascript +{ + metadata: { + claude_session_id: "4849e17c-5f44-4e50-ae5d-c15f21d6f92a", + create_mirror: true, + env_vars: null, + startup_process_pid: null, + startup_log_path: null, + startup_port: null, + version: undefined, // <-- This causes the error! + } +} +``` + +Spanner throws: `The code field is required for types.` + +## Proposed Solutions + +### 1. Clean JSON Data Before Sending to Spanner + +In the spanner-orm adapter, automatically clean JSON data: + +```typescript +// In spanner/adapter.ts or similar +function cleanJsonForSpanner(value: any): any { + if (value === null || value === undefined) { + return null; + } + + if (typeof value === 'object' && !Array.isArray(value)) { + const cleaned: any = {}; + for (const key in value) { + if (value[key] !== undefined) { + cleaned[key] = cleanJsonForSpanner(value[key]); + } + // Skip undefined values entirely + } + return cleaned; + } + + if (Array.isArray(value)) { + return value.map(item => cleanJsonForSpanner(item)); + } + + return value; +} +``` + +### 2. Provide Type Information for Null Values + +When building queries with null values, include type information: + +```typescript +// When executing queries with null parameters +const [rows] = await transaction.run({ + sql, + params, + json: true, + types: { + // Provide types for all nullable parameters + p1: 'STRING', + p2: 'TIMESTAMP', + p3: 'JSON', + // etc. + } +}); +``` + +### 3. Improve Error Messages + +Catch Spanner's cryptic error messages and provide better context: + +```typescript +try { + // Execute query +} catch (error) { + if (error.message?.includes('The code field is required for types')) { + // Check for undefined values in JSON + const hasUndefined = checkForUndefinedInParams(params); + if (hasUndefined) { + throw new Error( + 'Spanner Error: JSON columns cannot contain undefined values. ' + + 'Found undefined in parameters. Please use null instead of undefined.' + ); + } + + // Check for null values without types + const nullParams = findNullParams(params); + if (nullParams.length > 0) { + throw new Error( + 'Spanner Error: Null values require type information. ' + + `Parameters with null values: ${nullParams.join(', ')}` + ); + } + } + throw error; +} +``` + +## Implementation Priority + +1. **High Priority**: Clean JSON data to remove undefined values (prevents most common errors) +2. **Medium Priority**: Better error messages (helps developers debug issues) +3. **Low Priority**: Automatic type inference for null values (complex but would eliminate the need for manual type hints) + +## Testing + +Test cases should include: +- Inserting JSON with undefined values +- Inserting JSON with null values +- Inserting null for nullable columns +- Mixed scenarios with both issues + +## Notes + +- This issue only affects Spanner, not PostgreSQL +- The error message "The code field is required for types" is from Spanner's internal type system and is not related to any actual "code" field in the user's schema +- This is a common issue that affects many developers using Spanner with JSON columns \ No newline at end of file diff --git a/test/spanner/adapter-type-hints.test.ts b/test/spanner/adapter-type-hints.test.ts new file mode 100644 index 0000000..abce38b --- /dev/null +++ b/test/spanner/adapter-type-hints.test.ts @@ -0,0 +1,595 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import type { SpannerConnectionOptions } from "../../src/spanner/adapter.js"; + +// Create a test-only version of SpannerAdapter that uses mocked Spanner +class TestableSpannerAdapter { + readonly dialect = "spanner"; + private mockDb: any; + private isConnected = false; + private options: SpannerConnectionOptions; + + constructor(options: SpannerConnectionOptions) { + this.options = options; + } + + async connect(): Promise { + // Create mock database + this.mockDb = { + run: async (options: any) => { + // Simulate successful query for connection test + if (options.sql === "SELECT 1") { + return [[], {}]; + } + // Check if type hints are being passed correctly + if (options.types || options.paramTypes) { + return [[{ result: "with_types" }], {}]; + } + return [[{ result: "no_types" }], {}]; + }, + runTransactionAsync: async (callback: any) => { + const mockTransaction = { + runUpdate: async (options: any) => { + // Check if type hints are being passed + if (options.types || options.paramTypes) { + return [1]; // 1 row affected with types + } + return [0]; // 0 rows affected without types + }, + run: async (options: any) => { + if (options.types || options.paramTypes) { + return [[{ result: "with_types" }], {}]; + } + return [[{ result: "no_types" }], {}]; + }, + commit: async () => {}, + rollback: async () => {}, + }; + return callback(mockTransaction); + }, + getTransaction: () => ({ + runUpdate: async (options: any) => { + if (options.types || options.paramTypes) { + return [1]; + } + return [0]; + }, + run: async (options: any) => { + if (options.types || options.paramTypes) { + return [[{ result: "with_types" }], {}]; + } + return [[{ result: "no_types" }], {}]; + }, + commit: async () => {}, + rollback: async () => {}, + begin: async () => {}, + }), + }; + this.isConnected = true; + } + + async disconnect(): Promise { + this.isConnected = false; + this.mockDb = undefined; + } + + private ensureConnected() { + if (!this.isConnected || !this.mockDb) { + throw new Error("Not connected"); + } + return this.mockDb; + } + + // Clean JSON helper (copy from real adapter) + private cleanJsonForSpanner(value: any): any { + if (value === null || value === undefined) { + return null; + } + + if (typeof value === 'object' && !Array.isArray(value) && !(value instanceof Date)) { + const cleaned: any = {}; + for (const key in value) { + if (Object.prototype.hasOwnProperty.call(value, key)) { + if (value[key] !== undefined) { + cleaned[key] = this.cleanJsonForSpanner(value[key]); + } + // Skip undefined values entirely + } + } + return cleaned; + } + + if (Array.isArray(value)) { + return value.map(item => this.cleanJsonForSpanner(item)); + } + + return value; + } + + private cleanParamsForSpanner( + params?: Record, + typeHints?: Record + ): Record | undefined { + if (!params) return undefined; + + const cleaned: Record = {}; + for (const key in params) { + if (Object.prototype.hasOwnProperty.call(params, key)) { + const hint = typeHints?.[key]; + // Clean JSON fields + if (hint && (hint.toUpperCase() === 'JSON' || hint.toUpperCase() === 'JSONB')) { + cleaned[key] = this.cleanJsonForSpanner(params[key]); + } else { + cleaned[key] = params[key]; + } + } + } + return cleaned; + } + + async query( + sql: string, + params?: Record, + spannerTypeHints?: Record + ): Promise { + const db = this.ensureConnected(); + + // Clean params if they contain JSON + const cleanedParams = this.cleanParamsForSpanner(params, spannerTypeHints); + + const queryOptions: any = { + sql, + params: cleanedParams, + json: true, + }; + + // Add types if provided + if (spannerTypeHints) { + queryOptions.types = spannerTypeHints; + queryOptions.paramTypes = spannerTypeHints; // Mock uses this to detect type hints + } + + const [rows] = await db.run(queryOptions); + return rows as T[]; + } + + async execute( + sql: string, + params?: Record, + spannerTypeHints?: Record + ): Promise<{ count: number }> { + const db = this.ensureConnected(); + + // Clean params if they contain JSON + const cleanedParams = this.cleanParamsForSpanner(params, spannerTypeHints); + + const rowCount = await db.runTransactionAsync( + async (transaction: any) => { + const updateOptions: any = { + sql, + params: cleanedParams, + }; + + // Add types if provided + if (spannerTypeHints) { + updateOptions.types = spannerTypeHints; + updateOptions.paramTypes = spannerTypeHints; + } + + const [count] = await transaction.runUpdate(updateOptions); + await transaction.commit(); + return count; + } + ); + return { count: typeof rowCount === "number" ? rowCount : 0 }; + } + + async executeAndReturnRows( + sql: string, + params?: Record, + spannerTypeHints?: Record + ): Promise { + const db = this.ensureConnected(); + + // Clean params if they contain JSON + const cleanedParams = this.cleanParamsForSpanner(params, spannerTypeHints); + + return await db.runTransactionAsync( + async (transaction: any) => { + const queryOptions: any = { + sql, + params: cleanedParams, + json: true, + }; + + // Add types if provided + if (spannerTypeHints) { + queryOptions.types = spannerTypeHints; + queryOptions.paramTypes = spannerTypeHints; + } + + const [rows] = await transaction.run(queryOptions); + await transaction.commit(); + return rows as T[]; + } + ); + } + + async beginTransaction() { + const db = this.ensureConnected(); + const spannerTx = db.getTransaction(); + + return { + execute: async ( + sqlCmd: string, + paramsCmd?: Record, + cmdSpannerTypeHints?: Record + ) => { + if (spannerTx.begin) await spannerTx.begin(); + + const cleanedParams = this.cleanParamsForSpanner(paramsCmd, cmdSpannerTypeHints); + const updateOptions: any = { + sql: sqlCmd, + params: cleanedParams, + }; + + if (cmdSpannerTypeHints) { + updateOptions.types = cmdSpannerTypeHints; + updateOptions.paramTypes = cmdSpannerTypeHints; + } + + const [rowCount] = await spannerTx.runUpdate(updateOptions); + return { count: rowCount }; + }, + query: async ( + sqlQuery: string, + paramsQuery?: Record, + querySpannerTypeHints?: Record + ): Promise => { + const cleanedParams = this.cleanParamsForSpanner(paramsQuery, querySpannerTypeHints); + const queryOptions: any = { + sql: sqlQuery, + params: cleanedParams, + json: true, + }; + + if (querySpannerTypeHints) { + queryOptions.types = querySpannerTypeHints; + queryOptions.paramTypes = querySpannerTypeHints; + } + + const [rows] = await spannerTx.run(queryOptions); + return rows as T[]; + }, + commit: async () => { + if (spannerTx.commit) await spannerTx.commit(); + }, + rollback: async () => { + if (spannerTx.rollback) await spannerTx.rollback(); + }, + }; + } + + async transaction( + callback: (tx: any) => Promise + ): Promise { + const db = this.ensureConnected(); + return db.runTransactionAsync( + async (gcpTransaction: any) => { + const txExecutor = { + execute: async ( + cmdSql: string, + cmdParams?: Record, + cmdSpannerTypeHints?: Record + ) => { + const cleanedParams = this.cleanParamsForSpanner(cmdParams, cmdSpannerTypeHints); + const updateOptions: any = { + sql: cmdSql, + params: cleanedParams, + }; + + if (cmdSpannerTypeHints) { + updateOptions.types = cmdSpannerTypeHints; + updateOptions.paramTypes = cmdSpannerTypeHints; + } + + const [rowCount] = await gcpTransaction.runUpdate(updateOptions); + return { count: rowCount }; + }, + query: async ( + querySql: string, + queryParams?: Record, + querySpannerTypeHints?: Record + ) => { + const cleanedParams = this.cleanParamsForSpanner(queryParams, querySpannerTypeHints); + const queryOptions: any = { + sql: querySql, + params: cleanedParams, + json: true, + }; + + if (querySpannerTypeHints) { + queryOptions.types = querySpannerTypeHints; + queryOptions.paramTypes = querySpannerTypeHints; + } + + const [rows] = await gcpTransaction.run(queryOptions); + return rows as any[]; + }, + commit: async () => {}, + rollback: async () => {}, + }; + return callback(txExecutor); + } + ); + } +} + +describe("SpannerAdapter Type Hints", () => { + let adapter: TestableSpannerAdapter; + + beforeEach(async () => { + const options: SpannerConnectionOptions = { + projectId: "test-project", + instanceId: "test-instance", + databaseId: "test-database", + }; + adapter = new TestableSpannerAdapter(options); + await adapter.connect(); + }); + + describe("JSON Data Cleaning", () => { + it("should clean undefined values from JSON parameters", async () => { + const params = { + p1: "value1", + p2: { + field1: "test", + field2: undefined, + field3: null, + nested: { + a: 1, + b: undefined, + c: null, + }, + }, + }; + + const typeHints = { + p2: "JSON", + }; + + // This should not throw an error even with undefined values + const result = await adapter.query("SELECT * FROM test", params, typeHints); + expect(result).toBeDefined(); + // Since we're passing type hints, it should return "with_types" + expect(result).toEqual([{ result: "with_types" }]); + }); + + it("should handle arrays with undefined values in JSON", async () => { + const params = { + p1: [1, undefined, 3, null], + p2: { + arr: [undefined, "test", null], + }, + }; + + const typeHints = { + p1: "JSON", + p2: "JSON", + }; + + const result = await adapter.query("SELECT * FROM test", params, typeHints); + expect(result).toBeDefined(); + expect(result).toEqual([{ result: "with_types" }]); + }); + }); + + describe("Type Hints in Query Methods", () => { + it("should pass type hints to query method", async () => { + const params = { p1: "test", p2: 123 }; + const typeHints = { p1: "STRING", p2: "INT64" }; + + const result = await adapter.query("SELECT * FROM test", params, typeHints); + expect(result).toEqual([{ result: "with_types" }]); + }); + + it("should work without type hints", async () => { + const params = { p1: "test", p2: 123 }; + + const result = await adapter.query("SELECT * FROM test", params); + expect(result).toEqual([{ result: "no_types" }]); + }); + + it("should pass type hints to execute method", async () => { + const params = { p1: "test", p2: null }; + const typeHints = { p1: "STRING", p2: "INT64" }; + + const result = await adapter.execute("UPDATE test SET col = @p1", params, typeHints); + expect(result).toEqual({ count: 1 }); + }); + + it("should work without type hints in execute", async () => { + const params = { p1: "test" }; + + const result = await adapter.execute("UPDATE test SET col = @p1", params); + expect(result).toEqual({ count: 0 }); + }); + + it("should pass type hints to executeAndReturnRows method", async () => { + const params = { p1: new Date(), p2: true }; + const typeHints = { p1: "TIMESTAMP", p2: "BOOL" }; + + const result = await adapter.executeAndReturnRows( + "INSERT INTO test VALUES (@p1, @p2) THEN RETURN *", + params, + typeHints + ); + expect(result).toEqual([{ result: "with_types" }]); + }); + }); + + describe("Type Mapping", () => { + it("should map PostgreSQL types to Spanner types correctly", async () => { + const typeTests = [ + { pgType: "text", spannerType: "STRING" }, + { pgType: "varchar(255)", spannerType: "STRING" }, + { pgType: "uuid", spannerType: "STRING" }, + { pgType: "integer", spannerType: "INT64" }, + { pgType: "bigint", spannerType: "INT64" }, + { pgType: "serial", spannerType: "INT64" }, + { pgType: "boolean", spannerType: "BOOL" }, + { pgType: "double precision", spannerType: "FLOAT64" }, + { pgType: "real", spannerType: "FLOAT64" }, + { pgType: "numeric(10,2)", spannerType: "NUMERIC" }, + { pgType: "decimal", spannerType: "NUMERIC" }, + { pgType: "date", spannerType: "DATE" }, + { pgType: "timestamp", spannerType: "TIMESTAMP" }, + { pgType: "timestamptz", spannerType: "TIMESTAMP" }, + { pgType: "jsonb", spannerType: "JSON" }, + { pgType: "json", spannerType: "JSON" }, + { pgType: "bytea", spannerType: "BYTES" }, + ]; + + for (const test of typeTests) { + const params = { p1: null }; + const typeHints = { p1: test.pgType }; + + // The mock will return "with_types" if types are passed + const result = await adapter.query("SELECT * FROM test", params, typeHints); + expect(result).toEqual([{ result: "with_types" }]); + } + }); + + it("should pass through valid Spanner type codes", async () => { + const validTypeCodes = [ + "STRING", + "INT64", + "BOOL", + "FLOAT64", + "TIMESTAMP", + "DATE", + "BYTES", + "NUMERIC", + "JSON", + ]; + + for (const typeCode of validTypeCodes) { + const params = { p1: null }; + const typeHints = { p1: typeCode }; + + const result = await adapter.query("SELECT * FROM test", params, typeHints); + expect(result).toEqual([{ result: "with_types" }]); + } + }); + }); + + describe("Transaction Type Hints", () => { + it("should pass type hints in transaction execute", async () => { + const result = await adapter.transaction(async (tx) => { + const params = { p1: "test", p2: 456 }; + const typeHints = { p1: "STRING", p2: "INT64" }; + + const execResult = await tx.execute("UPDATE test SET col = @p1", params, typeHints); + return execResult; + }); + + expect(result).toEqual({ count: 1 }); + }); + + it("should pass type hints in transaction query", async () => { + const result = await adapter.transaction(async (tx) => { + const params = { p1: new Date() }; + const typeHints = { p1: "TIMESTAMP" }; + + const queryResult = await tx.query("SELECT * FROM test WHERE created = @p1", params, typeHints); + return queryResult; + }); + + expect(result).toEqual([{ result: "with_types" }]); + }); + + it("should work without type hints in transaction", async () => { + const result = await adapter.transaction(async (tx) => { + const params = { p1: "test" }; + + const queryResult = await tx.query("SELECT * FROM test WHERE name = @p1", params); + return queryResult; + }); + + expect(result).toEqual([{ result: "no_types" }]); + }); + }); + + describe("beginTransaction Type Hints", () => { + it("should pass type hints in beginTransaction", async () => { + const tx = await adapter.beginTransaction(); + + const params = { p1: "test", p2: 123 }; + const typeHints = { p1: "STRING", p2: "INT64" }; + + const result = await tx.execute("UPDATE test SET col = @p1", params, typeHints); + expect(result).toEqual({ count: 1 }); + + await tx.commit(); + }); + + it("should work with query in beginTransaction", async () => { + const tx = await adapter.beginTransaction(); + + const params = { p1: "test" }; + const typeHints = { p1: "STRING" }; + + const result = await tx.query("SELECT * FROM test", params, typeHints); + expect(result).toEqual([{ result: "with_types" }]); + + await tx.commit(); + }); + }); + + describe("Null Parameter Handling", () => { + it("should handle null parameters with type hints", async () => { + const params = { + p1: null, + p2: null, + p3: "value", + }; + + const typeHints = { + p1: "STRING", + p2: "INT64", + p3: "STRING", + }; + + // With type hints, null values should work fine + const result = await adapter.query("SELECT * FROM test", params, typeHints); + expect(result).toEqual([{ result: "with_types" }]); + }); + + it("should handle mixed null and undefined in JSON", async () => { + const params = { + jsonField: { + a: null, + b: undefined, + c: "value", + nested: { + x: undefined, + y: null, + }, + }, + }; + + const typeHints = { + jsonField: "JSON", + }; + + // Should clean undefined but keep null + const result = await adapter.query("SELECT * FROM test", params, typeHints); + expect(result).toEqual([{ result: "with_types" }]); + }); + }); + + afterEach(async () => { + if (adapter) { + await adapter.disconnect(); + } + }); +}); \ No newline at end of file