From 6ef44c383a90bf6ae95de531c83e21d2d58da159 Mon Sep 17 00:00:00 2001 From: Astha Mohta <35952883+asthamohta@users.noreply.github.com> Date: Tue, 13 Feb 2024 21:22:39 +0530 Subject: [PATCH] feat: untyped param types (#1869) This features allows customers to not pass paramType in parametrised queries. It gives backend to flexibly match the type. Example: Previously is timestamp was passed as string, it would be passed to backend with string type code and hence an error would be thrown. Now it is passed without any type code and hence backend handles checking its time and the code succeeds. --- samples/dml.js | 15 ++ samples/struct.js | 30 ++++ src/codec.ts | 5 +- src/transaction.ts | 5 +- system-test/spanner.ts | 346 +++++++++++++++++++++++++++++++++++++++++ test/codec.ts | 4 +- test/spanner.ts | 1 - test/transaction.ts | 41 ++--- 8 files changed, 420 insertions(+), 27 deletions(-) diff --git a/samples/dml.js b/samples/dml.js index dface9327..e51c89cba 100644 --- a/samples/dml.js +++ b/samples/dml.js @@ -288,6 +288,21 @@ function updateUsingDmlWithStruct(instanceId, databaseId, projectId) { params: { name: nameStruct, }, + types: { + name: { + type: 'struct', + fields: [ + { + name: 'FirstName', + type: 'string', + }, + { + name: 'LastName', + type: 'string', + }, + ], + }, + }, }); console.log(`Successfully updated ${rowCount} record.`); diff --git a/samples/struct.js b/samples/struct.js index a484b7ee1..3efd08127 100644 --- a/samples/struct.js +++ b/samples/struct.js @@ -111,6 +111,21 @@ async function queryDataWithStruct(instanceId, databaseId, projectId) { params: { name: nameStruct, }, + types: { + name: { + type: 'struct', + fields: [ + { + name: 'FirstName', + type: 'string', + }, + { + name: 'LastName', + type: 'string', + }, + ], + }, + }, }; // Queries rows from the Singers table @@ -250,6 +265,21 @@ async function queryStructField(instanceId, databaseId, projectId) { params: { name: nameStruct, }, + types: { + name: { + type: 'struct', + fields: [ + { + name: 'FirstName', + type: 'string', + }, + { + name: 'LastName', + type: 'string', + }, + ], + }, + }, }; // Queries rows from the Singers table diff --git a/src/codec.ts b/src/codec.ts index a51320636..14e9056b2 100644 --- a/src/codec.ts +++ b/src/codec.ts @@ -597,10 +597,6 @@ function getType(value: Value): Type { return {type: 'bool'}; } - if (is.string(value)) { - return {type: 'string'}; - } - if (Buffer.isBuffer(value)) { return {type: 'bytes'}; } @@ -643,6 +639,7 @@ function getType(value: Value): Type { return {type: 'json'}; } + // String type is also returned as unspecified to allow untyped parameters return {type: 'unspecified'}; } diff --git a/src/transaction.ts b/src/transaction.ts index 11eb03657..4eb564459 100644 --- a/src/transaction.ts +++ b/src/transaction.ts @@ -1300,7 +1300,10 @@ export class Snapshot extends EventEmitter { if (!is.empty(typeMap)) { Object.keys(typeMap).forEach(param => { const type = typeMap[param]; - paramTypes[param] = codec.createTypeObject(type); + const typeObject = codec.createTypeObject(type); + if (typeObject.code !== 'TYPE_CODE_UNSPECIFIED') { + paramTypes[param] = codec.createTypeObject(type); + } }); } diff --git a/system-test/spanner.ts b/system-test/spanner.ts index 5575d5c5f..736e9e5a9 100644 --- a/system-test/spanner.ts +++ b/system-test/spanner.ts @@ -275,6 +275,47 @@ describe('Spanner', () => { }); }); } + function readUntypedData(column, value, dialect, callback) { + const id = generateName('id'); + const insertData = { + Key: id, + [column]: value, + }; + + let table = googleSqlTable; + let query: ExecuteSqlRequest = { + sql: 'SELECT * FROM `' + table.name + '` WHERE ' + column + ' = @value', + params: { + value, + }, + }; + let database = DATABASE; + if (dialect === Spanner.POSTGRESQL) { + table = postgreSqlTable; + query = { + sql: 'SELECT * FROM ' + table.name + ' WHERE "' + column + '" = $1', + params: { + p1: value, + }, + }; + database = PG_DATABASE; + } + table.insert(insertData, (err, insertResp) => { + if (err) { + callback(err); + return; + } + + database.run(query, (err, rows, readResp) => { + if (err) { + callback(err); + return; + } + + callback(null, rows.shift(), insertResp, readResp); + }); + }); + } before(async () => { if (IS_EMULATOR_ENABLED) { @@ -757,6 +798,33 @@ describe('Spanner', () => { done(); }); }); + + it('GOOGLE_STANDARD_SQL should read untyped int64 values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData( + 'IntValue', + '5', + Spanner.GOOGLE_STANDARD_SQL, + (err, row) => { + assert.ifError(err); + assert.deepStrictEqual(row.toJSON().IntValue, 5); + done(); + } + ); + }); + + it('POSTGRESQL should read untyped int64 values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData('IntValue', '5', Spanner.POSTGRESQL, (err, row) => { + assert.ifError(err); + assert.deepStrictEqual(row.toJSON().IntValue, 5); + done(); + }); + }); }); describe('float64s', () => { @@ -905,6 +973,33 @@ describe('Spanner', () => { done(); }); }); + + it('GOOGLE_STANDARD_SQL should read untyped float64 values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData( + 'FloatValue', + 5.6, + Spanner.GOOGLE_STANDARD_SQL, + (err, row) => { + assert.ifError(err); + assert.deepStrictEqual(row.toJSON().FloatValue, 5.6); + done(); + } + ); + }); + + it('POSTGRESQL should read untyped float64 values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData('FloatValue', 5.6, Spanner.POSTGRESQL, (err, row) => { + assert.ifError(err); + assert.deepStrictEqual(row.toJSON().FloatValue, 5.6); + done(); + }); + }); }); describe('numerics', () => { @@ -1055,6 +1150,44 @@ describe('Spanner', () => { done(); }); }); + + it('GOOGLE_STANDARD_SQL should read untyped numeric values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData( + 'NumericValue', + '5.623', + Spanner.GOOGLE_STANDARD_SQL, + (err, row) => { + assert.ifError(err); + assert.deepStrictEqual( + row.toJSON().NumericValue.value, + Spanner.numeric('5.623').value + ); + done(); + } + ); + }); + + it('POSTGRESQL should read untyped numeric values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData( + 'NumericValue', + '5.623', + Spanner.POSTGRESQL, + (err, row) => { + assert.ifError(err); + assert.deepStrictEqual( + row.toJSON().NumericValue, + Spanner.pgNumeric(5.623) + ); + done(); + } + ); + }); }); describe('strings', () => { @@ -1156,6 +1289,38 @@ describe('Spanner', () => { } ); }); + + it('GOOGLE_STANDARD_SQL should read untyped string values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData( + 'StringValue', + 'hello', + Spanner.GOOGLE_STANDARD_SQL, + (err, row) => { + assert.ifError(err); + assert.deepStrictEqual(row.toJSON().StringValue, 'hello'); + done(); + } + ); + }); + + it('POSTGRESQL should read untyped string values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData( + 'StringValue', + 'hello', + Spanner.POSTGRESQL, + (err, row) => { + assert.ifError(err); + assert.deepStrictEqual(row.toJSON().StringValue, 'hello'); + done(); + } + ); + }); }); describe('bytes', () => { @@ -1257,6 +1422,38 @@ describe('Spanner', () => { done(); }); }); + + it('GOOGLE_STANDARD_SQL should read untyped bytes values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData( + 'BytesValue', + Buffer.from('b'), + Spanner.GOOGLE_STANDARD_SQL, + (err, row) => { + assert.ifError(err); + assert.deepStrictEqual(row.toJSON().BytesValue, Buffer.from('b')); + done(); + } + ); + }); + + it('POSTGRESQL should read untyped bytes values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData( + 'BytesValue', + Buffer.from('b'), + Spanner.POSTGRESQL, + (err, row) => { + assert.ifError(err); + assert.deepStrictEqual(row.toJSON().BytesValue, Buffer.from('b')); + done(); + } + ); + }); }); describe('jsons', () => { @@ -1435,6 +1632,46 @@ describe('Spanner', () => { done(); }); }); + + it('GOOGLE_STANDARD_SQL should read untyped timestamp values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData( + 'TimestampValue', + '2014-09-27T12:30:00.45Z', + Spanner.GOOGLE_STANDARD_SQL, + (err, row) => { + assert.ifError(err); + const time = row.toJSON().TimestampValue.getTime(); + assert.strictEqual( + time, + Spanner.timestamp('2014-09-27T12:30:00.45Z').getTime() + ); + done(); + } + ); + }); + + it('POSTGRESQL should read untyped timestamp values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData( + 'TimestampValue', + '2014-09-27T12:30:00.45Z', + Spanner.POSTGRESQL, + (err, row) => { + assert.ifError(err); + const time = row.toJSON().TimestampValue.getTime(); + assert.strictEqual( + time, + Spanner.timestamp('2014-09-27T12:30:00.45Z').getTime() + ); + done(); + } + ); + }); }); describe('dates', () => { @@ -1541,6 +1778,44 @@ describe('Spanner', () => { done(); }); }); + + it('GOOGLE_STANDARD_SQL should read untyped date values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData( + 'DateValue', + '2014-09-27', + Spanner.GOOGLE_STANDARD_SQL, + (err, row) => { + assert.ifError(err); + assert.deepStrictEqual( + Spanner.date(row.toJSON().DateValue), + Spanner.date('2014-09-27') + ); + done(); + } + ); + }); + + it('POSTGRESQL should read untyped date values', function (done) { + if (IS_EMULATOR_ENABLED) { + this.skip(); + } + readUntypedData( + 'DateValue', + '2014-09-27', + Spanner.POSTGRESQL, + (err, row) => { + assert.ifError(err); + assert.deepStrictEqual( + Spanner.date(row.toJSON().DateValue), + Spanner.date('2014-09-27') + ); + done(); + } + ); + }); }); describe('jsonb', () => { @@ -4920,6 +5195,9 @@ describe('Spanner', () => { params: { v: 'abc', }, + types: { + v: 'string', + }, }; stringQuery(done, DATABASE, query, 'abc'); }); @@ -4974,6 +5252,12 @@ describe('Spanner', () => { params: { v: values, }, + types: { + v: { + type: 'array', + child: 'string', + }, + }, }; DATABASE.run(query, (err, rows) => { @@ -5426,6 +5710,21 @@ describe('Spanner', () => { }), p4: Spanner.int(10), }, + types: { + structParam: { + type: 'struct', + fields: [ + { + name: 'userf', + type: 'string', + }, + { + name: 'threadf', + type: 'int64', + }, + ], + }, + }, }; DATABASE.run(query, (err, rows) => { @@ -5481,6 +5780,23 @@ describe('Spanner', () => { }), }), }, + types: { + structParam: { + type: 'struct', + fields: [ + { + name: 'structf', + type: 'struct', + fields: [ + { + name: 'nestedf', + type: 'string', + }, + ], + }, + ], + }, + }, }; DATABASE.run(query, (err, rows) => { @@ -5652,6 +5968,21 @@ describe('Spanner', () => { userf: 'bob', }), }, + types: { + structParam: { + type: 'struct', + fields: [ + { + name: 'threadf', + type: 'int64', + }, + { + name: 'userf', + type: 'string', + }, + ], + }, + }, }; DATABASE.run(query, (err, rows) => { @@ -5673,6 +6004,21 @@ describe('Spanner', () => { threadf: Spanner.int(1), }), }, + types: { + structParam: { + type: 'struct', + fields: [ + { + name: 'userf', + type: 'string', + }, + { + name: 'threadf', + type: 'int64', + }, + ], + }, + }, }; DATABASE.run(query, (err, rows) => { diff --git a/test/codec.ts b/test/codec.ts index 85e694258..43b17bbb4 100644 --- a/test/codec.ts +++ b/test/codec.ts @@ -920,7 +920,7 @@ describe('codec', () => { }); it('should determine if the value is a string', () => { - assert.deepStrictEqual(codec.getType('abc'), {type: 'string'}); + assert.deepStrictEqual(codec.getType('abc'), {type: 'unspecified'}); }); it('should determine if the value is bytes', () => { @@ -957,7 +957,7 @@ describe('codec', () => { assert.deepStrictEqual(type, { type: 'struct', - fields: [{name: 'a', type: 'string'}], + fields: [{name: 'a', type: 'unspecified'}], }); }); diff --git a/test/spanner.ts b/test/spanner.ts index c53a920ac..315fb0596 100644 --- a/test/spanner.ts +++ b/test/spanner.ts @@ -961,7 +961,6 @@ describe('Spanner with mock server', () => { assert.strictEqual(request.paramTypes!['int64'].code, 'INT64'); assert.strictEqual(request.paramTypes!['float64'].code, 'FLOAT64'); assert.strictEqual(request.paramTypes!['numeric'].code, 'NUMERIC'); - assert.strictEqual(request.paramTypes!['string'].code, 'STRING'); assert.strictEqual(request.paramTypes!['bytes'].code, 'BYTES'); assert.strictEqual(request.paramTypes!['json'].code, 'JSON'); assert.strictEqual(request.paramTypes!['date'].code, 'DATE'); diff --git a/test/transaction.ts b/test/transaction.ts index 11a8647e0..8a40f4230 100644 --- a/test/transaction.ts +++ b/test/transaction.ts @@ -1120,10 +1120,11 @@ describe('Transaction', () => { }); it('should guess missing param types', () => { - const fakeParams = {a: 'foo', b: 3}; + const fakeParams = {a: true, b: 3}; const fakeTypes = {b: 'number'}; - const fakeMissingType = {type: 'string'}; - const expectedType = {code: google.spanner.v1.TypeCode.STRING}; + const fakeMissingType = {type: 'boolean'}; + const expectedMissingType = {code: google.spanner.v1.TypeCode.BOOL}; + const expectedKnownType = {code: google.spanner.v1.TypeCode.INT64}; sandbox .stub(codec, 'getType') @@ -1132,15 +1133,17 @@ describe('Transaction', () => { sandbox .stub(codec, 'createTypeObject') + .withArgs('number') + .returns(expectedKnownType as google.spanner.v1.Type) .withArgs(fakeMissingType) - .returns(expectedType as google.spanner.v1.Type); + .returns(expectedMissingType as google.spanner.v1.Type); const {paramTypes} = Snapshot.encodeParams({ params: fakeParams, types: fakeTypes, }); - assert.strictEqual(paramTypes.a, expectedType); + assert.strictEqual(paramTypes.a, expectedMissingType); }); }); }); @@ -1267,17 +1270,17 @@ describe('Transaction', () => { const OBJ_STATEMENTS = [ { - sql: 'INSERT INTO TxnTable (Key, StringValue) VALUES(@key, @str)', + sql: 'INSERT INTO TxnTable (Key, BoolValue) VALUES(@key, @bool)', params: { - key: 'k999', - str: 'abc', + key: 999, + bool: true, }, }, { - sql: 'UPDATE TxnTable t SET t.StringValue = @str WHERE t.Key = @key', + sql: 'UPDATE TxnTable t SET t.BoolValue = @bool WHERE t.Key = @key', params: { - key: 'k999', - str: 'abcd', + key: 999, + bool: false, }, }, ]; @@ -1287,26 +1290,26 @@ describe('Transaction', () => { sql: OBJ_STATEMENTS[0].sql, params: { fields: { - key: {stringValue: OBJ_STATEMENTS[0].params.key}, - str: {stringValue: OBJ_STATEMENTS[0].params.str}, + key: {stringValue: OBJ_STATEMENTS[0].params.key.toString()}, + bool: {boolValue: OBJ_STATEMENTS[0].params.bool}, }, }, paramTypes: { - key: {code: 'STRING'}, - str: {code: 'STRING'}, + key: {code: 'INT64'}, + bool: {code: 'BOOL'}, }, }, { sql: OBJ_STATEMENTS[1].sql, params: { fields: { - key: {stringValue: OBJ_STATEMENTS[1].params.key}, - str: {stringValue: OBJ_STATEMENTS[1].params.str}, + key: {stringValue: OBJ_STATEMENTS[1].params.key.toString()}, + bool: {boolValue: OBJ_STATEMENTS[1].params.bool}, }, }, paramTypes: { - key: {code: 'STRING'}, - str: {code: 'STRING'}, + key: {code: 'INT64'}, + bool: {code: 'BOOL'}, }, }, ];