From 49fa60dd0735fe66db33f7b9137dba0821eb5184 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Mon, 4 Mar 2024 12:40:16 +0000 Subject: [PATCH] fix: Revert untyped param type feature (#2012) This fix is reverting the feature untyped param type added in [PR](https://togithub.com/googleapis/nodejs-spanner/pull/1869) and released in [7.4.0](https://togithub.com/googleapis/nodejs-spanner/releases/tag/v7.4.0) This fixes issue #2009 which was introduced by untyped param type feature. --- 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, 27 insertions(+), 420 deletions(-) diff --git a/samples/dml.js b/samples/dml.js index e51c89cba..dface9327 100644 --- a/samples/dml.js +++ b/samples/dml.js @@ -288,21 +288,6 @@ 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 3efd08127..a484b7ee1 100644 --- a/samples/struct.js +++ b/samples/struct.js @@ -111,21 +111,6 @@ 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 @@ -265,21 +250,6 @@ 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 663a76873..ac018b310 100644 --- a/src/codec.ts +++ b/src/codec.ts @@ -629,6 +629,10 @@ function getType(value: Value): Type { return {type: 'bool'}; } + if (is.string(value)) { + return {type: 'string'}; + } + if (Buffer.isBuffer(value)) { return {type: 'bytes'}; } @@ -671,7 +675,6 @@ 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 4eb564459..11eb03657 100644 --- a/src/transaction.ts +++ b/src/transaction.ts @@ -1300,10 +1300,7 @@ export class Snapshot extends EventEmitter { if (!is.empty(typeMap)) { Object.keys(typeMap).forEach(param => { const type = typeMap[param]; - const typeObject = codec.createTypeObject(type); - if (typeObject.code !== 'TYPE_CODE_UNSPECIFIED') { - paramTypes[param] = codec.createTypeObject(type); - } + paramTypes[param] = codec.createTypeObject(type); }); } diff --git a/system-test/spanner.ts b/system-test/spanner.ts index a31b89f01..6690b5a18 100644 --- a/system-test/spanner.ts +++ b/system-test/spanner.ts @@ -384,47 +384,6 @@ 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) { @@ -907,33 +866,6 @@ 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('oids', () => { @@ -1114,33 +1046,6 @@ 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', () => { @@ -1291,44 +1196,6 @@ 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', () => { @@ -1430,38 +1297,6 @@ 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', () => { @@ -1563,38 +1398,6 @@ 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', () => { @@ -1773,46 +1576,6 @@ 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', () => { @@ -1919,44 +1682,6 @@ 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', () => { @@ -5382,9 +5107,6 @@ describe('Spanner', () => { params: { v: 'abc', }, - types: { - v: 'string', - }, }; stringQuery(done, DATABASE, query, 'abc'); }); @@ -5439,12 +5161,6 @@ describe('Spanner', () => { params: { v: values, }, - types: { - v: { - type: 'array', - child: 'string', - }, - }, }; DATABASE.run(query, (err, rows) => { @@ -5897,21 +5613,6 @@ describe('Spanner', () => { }), p4: Spanner.int(10), }, - types: { - structParam: { - type: 'struct', - fields: [ - { - name: 'userf', - type: 'string', - }, - { - name: 'threadf', - type: 'int64', - }, - ], - }, - }, }; DATABASE.run(query, (err, rows) => { @@ -5967,23 +5668,6 @@ describe('Spanner', () => { }), }), }, - types: { - structParam: { - type: 'struct', - fields: [ - { - name: 'structf', - type: 'struct', - fields: [ - { - name: 'nestedf', - type: 'string', - }, - ], - }, - ], - }, - }, }; DATABASE.run(query, (err, rows) => { @@ -6155,21 +5839,6 @@ describe('Spanner', () => { userf: 'bob', }), }, - types: { - structParam: { - type: 'struct', - fields: [ - { - name: 'threadf', - type: 'int64', - }, - { - name: 'userf', - type: 'string', - }, - ], - }, - }, }; DATABASE.run(query, (err, rows) => { @@ -6191,21 +5860,6 @@ 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 64085fce3..b604e4354 100644 --- a/test/codec.ts +++ b/test/codec.ts @@ -969,7 +969,7 @@ describe('codec', () => { }); it('should determine if the value is a string', () => { - assert.deepStrictEqual(codec.getType('abc'), {type: 'unspecified'}); + assert.deepStrictEqual(codec.getType('abc'), {type: 'string'}); }); it('should determine if the value is bytes', () => { @@ -1006,7 +1006,7 @@ describe('codec', () => { assert.deepStrictEqual(type, { type: 'struct', - fields: [{name: 'a', type: 'unspecified'}], + fields: [{name: 'a', type: 'string'}], }); }); diff --git a/test/spanner.ts b/test/spanner.ts index 32a1b8e58..38f1105d3 100644 --- a/test/spanner.ts +++ b/test/spanner.ts @@ -960,6 +960,7 @@ 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 8a40f4230..11a8647e0 100644 --- a/test/transaction.ts +++ b/test/transaction.ts @@ -1120,11 +1120,10 @@ describe('Transaction', () => { }); it('should guess missing param types', () => { - const fakeParams = {a: true, b: 3}; + const fakeParams = {a: 'foo', b: 3}; const fakeTypes = {b: 'number'}; - const fakeMissingType = {type: 'boolean'}; - const expectedMissingType = {code: google.spanner.v1.TypeCode.BOOL}; - const expectedKnownType = {code: google.spanner.v1.TypeCode.INT64}; + const fakeMissingType = {type: 'string'}; + const expectedType = {code: google.spanner.v1.TypeCode.STRING}; sandbox .stub(codec, 'getType') @@ -1133,17 +1132,15 @@ describe('Transaction', () => { sandbox .stub(codec, 'createTypeObject') - .withArgs('number') - .returns(expectedKnownType as google.spanner.v1.Type) .withArgs(fakeMissingType) - .returns(expectedMissingType as google.spanner.v1.Type); + .returns(expectedType as google.spanner.v1.Type); const {paramTypes} = Snapshot.encodeParams({ params: fakeParams, types: fakeTypes, }); - assert.strictEqual(paramTypes.a, expectedMissingType); + assert.strictEqual(paramTypes.a, expectedType); }); }); }); @@ -1270,17 +1267,17 @@ describe('Transaction', () => { const OBJ_STATEMENTS = [ { - sql: 'INSERT INTO TxnTable (Key, BoolValue) VALUES(@key, @bool)', + sql: 'INSERT INTO TxnTable (Key, StringValue) VALUES(@key, @str)', params: { - key: 999, - bool: true, + key: 'k999', + str: 'abc', }, }, { - sql: 'UPDATE TxnTable t SET t.BoolValue = @bool WHERE t.Key = @key', + sql: 'UPDATE TxnTable t SET t.StringValue = @str WHERE t.Key = @key', params: { - key: 999, - bool: false, + key: 'k999', + str: 'abcd', }, }, ]; @@ -1290,26 +1287,26 @@ describe('Transaction', () => { sql: OBJ_STATEMENTS[0].sql, params: { fields: { - key: {stringValue: OBJ_STATEMENTS[0].params.key.toString()}, - bool: {boolValue: OBJ_STATEMENTS[0].params.bool}, + key: {stringValue: OBJ_STATEMENTS[0].params.key}, + str: {stringValue: OBJ_STATEMENTS[0].params.str}, }, }, paramTypes: { - key: {code: 'INT64'}, - bool: {code: 'BOOL'}, + key: {code: 'STRING'}, + str: {code: 'STRING'}, }, }, { sql: OBJ_STATEMENTS[1].sql, params: { fields: { - key: {stringValue: OBJ_STATEMENTS[1].params.key.toString()}, - bool: {boolValue: OBJ_STATEMENTS[1].params.bool}, + key: {stringValue: OBJ_STATEMENTS[1].params.key}, + str: {stringValue: OBJ_STATEMENTS[1].params.str}, }, }, paramTypes: { - key: {code: 'INT64'}, - bool: {code: 'BOOL'}, + key: {code: 'STRING'}, + str: {code: 'STRING'}, }, }, ];