From b406ef23dd4ec8b67a4965812c110bc4087ae7d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 16 Sep 2026 09:32:24 +0200 Subject: [PATCH] perf(spanner): single-pass protobuf value encoding in codec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `codec.encode` previously executed two passes for every query parameter and mutation value: first calling `encodeValue` to construct an intermediate JavaScript representation, and then passing that to `GrpcService.encodeValue_`. `GrpcService.encodeValue_` allocated a new `ObjectToStructConverter` instance and an internal `Set` for each value, using `.map()` and bound closures for array transformations. This change converts `codec.encode` to emit `google.protobuf.Value` messages directly in a single pass: - Primitives dispatch directly via a `typeof` switch. - Array members are converted in an indexed loop into a pre-sized array without intermediate collections or closures. - The import of `GrpcService` is removed from `src/codec.ts`, eliminating the dependency on `./common-grpc/service`. - Encoding rules for Spanner types are preserved: finite non-integers encode as numbers; integers, `NaN`, and `±Infinity` encode as strings; and wrapped `Float`/`Float32` instances retain their numeric representation. - Cross-realm `Date` instances (e.g. from `vm` contexts) are handled via a tag check fallback on the object path. - Sparse arrays now fail immediately on missing indices rather than producing arrays with holes that fail during protobuf serialization. --- handwritten/spanner/src/codec.ts | 136 ++++++++++--- handwritten/spanner/test/codec.ts | 327 ++++++++++++++++++++++++------ 2 files changed, 373 insertions(+), 90 deletions(-) diff --git a/handwritten/spanner/src/codec.ts b/handwritten/spanner/src/codec.ts index be713e7e130f..0bd839ae5bd4 100644 --- a/handwritten/spanner/src/codec.ts +++ b/handwritten/spanner/src/codec.ts @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import {GrpcService} from './common-grpc/service'; import {PreciseDate} from '@google-cloud/precise-date'; import { isArray, @@ -1299,79 +1298,150 @@ function parsePreciseDate(isoString: string): PreciseDate { /** * Encode a value in the format the API expects. * + * This emits the `google.protobuf.Value` shape in a single pass: values are + * normalized and wrapped in their protobuf kind at the same time, so neither an + * intermediate representation nor a per-value `ObjectToStructConverter` is + * allocated. The branches prioritize primitive and common types for fast + * dispatch in hot query execution paths. + * * @private * * @param {*} value The value to be encoded. * @returns {object} google.protobuf.Value */ function encode(value: Value): p.IValue { - return GrpcService.encodeValue_(encodeValue(value)); + switch (typeof value) { + case 'string': + return {stringValue: value}; + case 'number': + // Only non-integer, finite numbers are sent as a protobuf number. Every + // other numeric value — integers (INT64 is a string on the wire) as well + // as NaN and ±Infinity — is sent as a string. + return Number.isFinite(value) && !Number.isInteger(value) + ? {numberValue: value} + : {stringValue: value.toString()}; + case 'boolean': + return {boolValue: value}; + case 'object': + return value === null ? {nullValue: 0} : encodeObject(value); + default: + throw new Error(`Value of type ${typeof value} not recognized.`); + } } /** - * Formats values into expected format of google.protobuf.Value. The actual - * conversion to a google.protobuf.Value object happens via - * `Service.encodeValue_` + * Encodes a non-null object into a google.protobuf.Value. * * @private * - * @param {*} value The value to be encoded. - * @returns {*} + * @param {object} value The value to be encoded. + * @returns {object} google.protobuf.Value */ -function encodeValue(value: Value): Value { - if (isNumber(value) && !isDecimal(value)) { - return value.toString(); +function encodeObject(value: object): p.IValue { + if (value instanceof Date) { + // `Date.prototype.toJSON` returns null for invalid dates. + const json: string | null = value.toJSON(); + return json === null ? {nullValue: 0} : {stringValue: json}; } - if (isDate(value)) { - return value.toJSON(); + if (value instanceof WrappedNumber) { + // `Float`/`Float32` hold a number, `Int`/`PGOid` hold a string. + return encodeNumberOrString(value.value); } - if (value instanceof WrappedNumber) { - return value.value; + if (Array.isArray(value)) { + return {listValue: {values: encodeArrayValues(value)}}; } - if (value instanceof Numeric) { - return value.value; + if (Buffer.isBuffer(value)) { + return {stringValue: value.toString('base64')}; } - if (value instanceof PGNumeric) { - return value.value; + if (value instanceof Numeric || value instanceof PGNumeric) { + return {stringValue: value.value}; } - if (Buffer.isBuffer(value)) { - return value.toString('base64'); + if (value instanceof Interval) { + return {stringValue: value.toISO8601()}; } if (value instanceof ProtoMessage) { - return value.value.toString('base64'); + return {stringValue: value.value.toString('base64')}; } if (value instanceof ProtoEnum) { - return value.value; + // Holds either the numeric constant or its string representation. + return encodeNumberOrString(value.value); } - if (value instanceof Struct) { - return Array.from(value).map(field => encodeValue(field.value)); + if (value instanceof PGJsonb) { + return {stringValue: value.toString()}; } - if (isArray(value)) { - return value.map(encodeValue); + if (isObject(value)) { + const json = JSON.stringify(value); + // `JSON.stringify` returns undefined only when an object's `toJSON()` + // method returns undefined. Other objects without a JSON representation + // (such as Functions, Symbols, and Maps) fail `isObject` and throw below. + // Note: `JSON.stringify` will throw a TypeError for circular structures + // or BigInt property values. + if (json !== undefined) { + return {stringValue: json}; + } } - if (value instanceof PGJsonb) { - return value.toString(); + if ( + value instanceof Number || + value instanceof String || + value instanceof Boolean + ) { + // Boxed primitives are never produced by the public API, but were accepted + // by the previous implementation. + return encode(value.valueOf()); } - if (value instanceof Interval) { - return value.toISO8601(); + if (isDate(value)) { + // A `Date` from another realm (e.g. a `vm` context) fails `instanceof`. + const json: string | null = (value as Date).toJSON(); + return json === null ? {nullValue: 0} : {stringValue: json}; } - if (isObject(value)) { - return JSON.stringify(value); + throw new Error('Value of type object not recognized.'); +} + +/** + * Encodes the members of an array — or the field values of a {@link Struct}, + * which is itself an array — into google.protobuf.Value messages. + * + * @private + * + * @param {Array} value The array to be encoded. + * @returns {object[]} google.protobuf.Value[] + */ +function encodeArrayValues(value: Value[]): p.IValue[] { + const {length} = value; + const values: p.IValue[] = new Array(length); + const isStruct = value instanceof Struct; + + for (let i = 0; i < length; i++) { + values[i] = encode(isStruct ? value[i].value : value[i]); } - return value; + return values; +} + +/** + * Wraps an already normalized value in the matching google.protobuf.Value kind. + * + * @private + * + * @param {string|number} value The value to be wrapped. + * @returns {object} google.protobuf.Value + */ +function encodeNumberOrString(value: string | number): p.IValue { + return typeof value === 'number' + ? {numberValue: value} + : {stringValue: value}; } /** diff --git a/handwritten/spanner/test/codec.ts b/handwritten/spanner/test/codec.ts index 6d3dddae9382..aa1910e3ca87 100644 --- a/handwritten/spanner/test/codec.ts +++ b/handwritten/spanner/test/codec.ts @@ -15,12 +15,11 @@ */ import * as assert from 'assert'; -import {before, beforeEach, afterEach, describe, it} from 'mocha'; -import * as proxyquire from 'proxyquire'; +import {afterEach, beforeEach, describe, it} from 'mocha'; import * as sinon from 'sinon'; +import * as vm from 'vm'; import {Big} from 'big.js'; import {PreciseDate} from '@google-cloud/precise-date'; -import {GrpcService} from '../src/common-grpc/service'; import {protos} from '@google-cloud/spanner-api'; import google = protos.google; import {GoogleError} from 'google-gax'; @@ -28,25 +27,15 @@ import {util} from 'protobufjs'; import * as crypto from 'crypto'; import Long = util.Long; import {isString} from '../src/helper'; +import {codec as realCodec} from '../src/codec'; +// Typed as any to support legacy test fixtures that pass duck-typed objects or loose parameters. +const codec: any = realCodec; const singer = require('./data/singer'); const music = singer.examples.spanner.music; describe('codec', () => { - let codec; - const sandbox = sinon.createSandbox(); - before(() => { - codec = proxyquire('../src/codec.js', { - './common-grpc/service': {GrpcService}, - }).codec; - }); - - beforeEach(() => { - sandbox.stub(GrpcService, 'encodeValue_').callsFake(value => value); - sandbox.stub(GrpcService, 'decodeValue_').callsFake(value => value); - }); - afterEach(() => sandbox.restore()); describe('SpannerDate', () => { @@ -1221,7 +1210,6 @@ describe('codec', () => { }); it('should return null values as null', () => { - (GrpcService.decodeValue_ as sinon.SinonStub).returns(null); const decoded = codec.decode(null, BYPASS_FIELD); assert.strictEqual(decoded, null); }); @@ -2078,16 +2066,17 @@ describe('codec', () => { }); describe('encode', () => { - it('should return the value from the common encoder', () => { - const value = {}; - const defaultEncodedValue = '{}'; + it('should encode NULL', () => { + assert.deepStrictEqual(codec.encode(null), {nullValue: 0}); + }); - (GrpcService.encodeValue_ as sinon.SinonStub) - .withArgs(value) - .returns(defaultEncodedValue); + it('should encode BOOL', () => { + assert.deepStrictEqual(codec.encode(true), {boolValue: true}); + assert.deepStrictEqual(codec.encode(false), {boolValue: false}); + }); - const encoded = codec.encode(value); - assert.strictEqual(encoded, defaultEncodedValue); + it('should encode STRING', () => { + assert.deepStrictEqual(codec.encode('hi'), {stringValue: 'hi'}); }); it('should encode BYTES', () => { @@ -2095,7 +2084,7 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.strictEqual(encoded, value.toString('base64')); + assert.deepStrictEqual(encoded, {stringValue: value.toString('base64')}); }); it('should encode ProtoMessage', () => { @@ -2115,10 +2104,11 @@ describe('codec', () => { const encoded = codec.encode(protoMessage); - assert.strictEqual( - encoded, - music.SingerInfo.encode(singerInfo).finish().toString('base64'), - ); + assert.deepStrictEqual(encoded, { + stringValue: music.SingerInfo.encode(singerInfo) + .finish() + .toString('base64'), + }); }); it('should encode ProtoEnum', () => { @@ -2131,19 +2121,87 @@ describe('codec', () => { const encoded = codec.encode(protoEnum); - assert.strictEqual(encoded, genre.toString()); + assert.deepStrictEqual(encoded, {stringValue: genre.toString()}); + }); + + it('should encode ProtoEnum with numeric value', () => { + const protoEnum = new codec.ProtoEnum({ + value: 3, + fullName: 'examples.spanner.music.Genre', + }); + + const encoded = codec.encode(protoEnum); + + assert.deepStrictEqual(encoded, {stringValue: '3'}); + }); + + it('should encode ProtoEnum with string enum resolving to numeric value', () => { + const protoEnum = new codec.ProtoEnum({ + value: 'ROCK', + enumObject: music.Genre, + fullName: 'examples.spanner.music.Genre', + }); + + const encoded = codec.encode(protoEnum); + + assert.deepStrictEqual(encoded, {numberValue: music.Genre.ROCK}); }); it('should encode structs', () => { const value = codec.Struct.fromJSON({a: 'b', c: 'd'}); + const encoded = codec.encode(value); - assert.deepStrictEqual(encoded, ['b', 'd']); + + assert.deepStrictEqual(encoded, { + listValue: {values: [{stringValue: 'b'}, {stringValue: 'd'}]}, + }); + }); + + it('should encode nested structs', () => { + const value = codec.Struct.fromJSON({ + a: codec.Struct.fromJSON({b: 5}), + }); + + const encoded = codec.encode(value); + + assert.deepStrictEqual(encoded, { + listValue: { + values: [{listValue: {values: [{stringValue: '5'}]}}], + }, + }); + }); + + it('should encode an empty struct', () => { + const encoded = codec.encode(codec.Struct.fromJSON({})); + + assert.deepStrictEqual(encoded, {listValue: {values: []}}); + }); + + it('should encode an array containing structs', () => { + const value = [codec.Struct.fromJSON({a: 'b'})]; + + const encoded = codec.encode(value); + + assert.deepStrictEqual(encoded, { + listValue: { + values: [{listValue: {values: [{stringValue: 'b'}]}}], + }, + }); + }); + + it('should throw if a struct field value is undefined', () => { + assert.throws( + () => codec.encode(codec.Struct.fromJSON({a: undefined})), + /Value of type undefined not recognized\./, + ); }); it('should stringify Infinity', () => { const value = Infinity; + const encoded = codec.encode(value); - assert.strictEqual(encoded, value.toString()); + + assert.deepStrictEqual(encoded, {stringValue: value.toString()}); }); it('should stringify -Infinity', () => { @@ -2151,7 +2209,7 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.strictEqual(encoded, value.toString()); + assert.deepStrictEqual(encoded, {stringValue: value.toString()}); }); it('should stringify NaN', () => { @@ -2159,7 +2217,7 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.strictEqual(encoded, value.toString()); + assert.deepStrictEqual(encoded, {stringValue: value.toString()}); }); it('should stringify INT64', () => { @@ -2167,7 +2225,34 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.strictEqual(encoded, value.toString()); + assert.deepStrictEqual(encoded, {stringValue: value.toString()}); + }); + + it('should stringify -0', () => { + const encoded = codec.encode(-0); + + assert.deepStrictEqual(encoded, {stringValue: '0'}); + }); + + it('should encode a non-integer number as a number', () => { + const encoded = codec.encode(3.14); + + assert.deepStrictEqual(encoded, {numberValue: 3.14}); + }); + + it('should encode Float with NaN and Infinity as numbers', () => { + assert.deepStrictEqual(codec.encode(new codec.Float(NaN)), { + numberValue: NaN, + }); + assert.deepStrictEqual(codec.encode(new codec.Float(Infinity)), { + numberValue: Infinity, + }); + assert.deepStrictEqual(codec.encode(new codec.Float(-Infinity)), { + numberValue: -Infinity, + }); + assert.deepStrictEqual(codec.encode(new codec.Float32(NaN)), { + numberValue: NaN, + }); }); it('should stringify NUMERIC', () => { @@ -2175,7 +2260,7 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.strictEqual(encoded, value.value); + assert.deepStrictEqual(encoded, {stringValue: value.value}); }); it('should stringify PG NUMERIC', () => { @@ -2183,7 +2268,7 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.strictEqual(encoded, value.value); + assert.deepStrictEqual(encoded, {stringValue: value.value}); }); it('should encode ARRAY and inner members', () => { @@ -2191,9 +2276,48 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.deepStrictEqual(encoded, [ - value.toString(), // (tests that it is stringified) - ]); + assert.deepStrictEqual(encoded, { + listValue: { + // Tests that the inner member is stringified. + values: [{stringValue: '5'}], + }, + }); + }); + + it('should encode nested ARRAYs', () => { + const value = [['a'], [null]]; + + const encoded = codec.encode(value); + + assert.deepStrictEqual(encoded, { + listValue: { + values: [ + {listValue: {values: [{stringValue: 'a'}]}}, + {listValue: {values: [{nullValue: 0}]}}, + ], + }, + }); + }); + + it('should encode an empty ARRAY', () => { + const encoded = codec.encode([]); + + assert.deepStrictEqual(encoded, {listValue: {values: []}}); + }); + + it('should throw if an array has an undefined value or hole', () => { + const sparseArray: unknown[] = []; + sparseArray[2] = 1; + + assert.throws( + () => codec.encode(sparseArray), + /Value of type undefined not recognized\./, + ); + + assert.throws( + () => codec.encode([undefined]), + /Value of type undefined not recognized\./, + ); }); it('should encode TIMESTAMP', () => { @@ -2201,7 +2325,7 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.strictEqual(encoded, value.toJSON()); + assert.deepStrictEqual(encoded, {stringValue: value.toJSON()}); }); it('should encode DATE', () => { @@ -2209,13 +2333,37 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.strictEqual(encoded, value.toJSON()); + assert.deepStrictEqual(encoded, {stringValue: value.toJSON()}); + }); + + it('should encode an invalid date as NULL', () => { + const encoded = codec.encode(new Date('not-a-date')); + + assert.deepStrictEqual(encoded, {nullValue: 0}); + }); + + it('should encode cross-realm Date', () => { + const crossRealmDate = vm.runInNewContext('new Date(0)'); + const encoded = codec.encode(crossRealmDate); + + assert.deepStrictEqual(encoded, { + stringValue: '1970-01-01T00:00:00.000Z', + }); + }); + + it('should encode cross-realm invalid Date as NULL', () => { + const crossRealmInvalidDate = vm.runInNewContext('new Date("nope")'); + const encoded = codec.encode(crossRealmInvalidDate); + + assert.deepStrictEqual(encoded, {nullValue: 0}); }); it('should encode INTERVAL', () => { const value = new codec.Interval(17, -20, BigInt(30001)); + const encoded = codec.encode(value); - assert.strictEqual(encoded, 'P1Y5M-20DT0.000030001S'); + + assert.deepStrictEqual(encoded, {stringValue: 'P1Y5M-20DT0.000030001S'}); }); it('should encode INT64', () => { @@ -2223,7 +2371,7 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.strictEqual(encoded, '10'); + assert.deepStrictEqual(encoded, {stringValue: '10'}); }); it('should encode PG OID', () => { @@ -2231,7 +2379,7 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.strictEqual(encoded, '10'); + assert.deepStrictEqual(encoded, {stringValue: '10'}); }); it('should encode UUID', () => { @@ -2239,7 +2387,7 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.strictEqual(encoded, value); + assert.deepStrictEqual(encoded, {stringValue: value}); }); it('should encode FLOAT32', () => { @@ -2247,7 +2395,7 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.strictEqual(encoded, 10); + assert.deepStrictEqual(encoded, {numberValue: 10}); }); it('should encode FLOAT64', () => { @@ -2255,7 +2403,7 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.strictEqual(encoded, 10); + assert.deepStrictEqual(encoded, {numberValue: 10}); }); it('should encode JSON', () => { @@ -2264,7 +2412,15 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.deepStrictEqual(encoded, expected); + assert.deepStrictEqual(encoded, {stringValue: expected}); + }); + + it('should encode PG JSONB', () => { + const value = new codec.PGJsonb({result: true}); + + const encoded = codec.encode(value); + + assert.deepStrictEqual(encoded, {stringValue: '{"result":true}'}); }); it('should encode complex object as JSON', () => { @@ -2277,10 +2433,10 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.deepStrictEqual( - encoded, - '{"boolKey":true,"numberKey":3.14,"stringKey":"test","objectKey":{"innerKey":"inner-value"}}', - ); + assert.deepStrictEqual(encoded, { + stringValue: + '{"boolKey":true,"numberKey":3.14,"stringKey":"test","objectKey":{"innerKey":"inner-value"}}', + }); }); it('should encode deeply-nested object as JSON', () => { @@ -2294,9 +2450,66 @@ describe('codec', () => { const encoded = codec.encode(value); - assert.deepStrictEqual( - encoded, - '{"k":'.repeat(nesting).concat('"v"').concat('}'.repeat(nesting)), + assert.deepStrictEqual(encoded, { + stringValue: '{"k":' + .repeat(nesting) + .concat('"v"') + .concat('}'.repeat(nesting)), + }); + }); + + it('should encode boxed primitives', () => { + assert.deepStrictEqual(codec.encode(new Number(5)), {stringValue: '5'}); + assert.deepStrictEqual(codec.encode(new Number(3.14)), { + numberValue: 3.14, + }); + assert.deepStrictEqual(codec.encode(new String('hi')), { + stringValue: 'hi', + }); + assert.deepStrictEqual(codec.encode(new Boolean(true)), { + boolValue: true, + }); + assert.deepStrictEqual(codec.encode(new Boolean(false)), { + boolValue: false, + }); + }); + + it('should throw if an object toJSON method returns undefined', () => { + assert.throws( + () => codec.encode({toJSON: () => undefined}), + /Value of type object not recognized\./, + ); + }); + + it('should throw for unsupported values', () => { + assert.throws( + () => codec.encode(undefined), + /Value of type undefined not recognized\./, + ); + + assert.throws( + () => codec.encode(new Map()), + /Value of type object not recognized\./, + ); + + assert.throws( + () => codec.encode(new Set([1])), + /Value of type object not recognized\./, + ); + + assert.throws( + () => codec.encode(BigInt(1)), + /Value of type bigint not recognized\./, + ); + + assert.throws( + () => codec.encode(Symbol('s')), + /Value of type symbol not recognized\./, + ); + + assert.throws( + () => codec.encode(() => {}), + /Value of type function not recognized\./, ); });