Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error parsing STRUCT with JSON "keys" field #1774

Closed
andrao opened this issue Jan 18, 2023 · 0 comments · Fixed by #1775 or #1779
Closed

Error parsing STRUCT with JSON "keys" field #1774

andrao opened this issue Jan 18, 2023 · 0 comments · Fixed by #1775 or #1779
Labels
api: spanner Issues related to the googleapis/nodejs-spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@andrao
Copy link
Contributor

andrao commented Jan 18, 2023

Environment details

  • OS: Mac OS Ventura 13.1
  • Node.js version: 14.17.6
  • npm version: 6.14.15
  • @google-cloud/spanner version: 6.7.0

Error

Steps to reproduce

The error occurs when the queried column either is or includes a STRUCT with a child field of type: "JSON" and a name value equal to one of the JS Array method names, e.g. "keys".

This condition is satisfied by a change-stream query that returns data change records. Change stream queries that don't return data change records are unaffected.

In my case I'm using database.runStream() to run the query.

See also

The error case is reproduced by this test case in the issue-associated PR: https://github.com/googleapis/nodejs-spanner/pull/1775/files#diff-bd3702694080cbb1cc59ec4e3021374f22a2fd665617a80bd3c29630b9dbe7b7R675-R695.

Details

I'm hitting an error when running a change-stream query via the database.runStream() method:

SELECT ChangeRecord FROM READ_${MyStream}
(
    start_timestamp => @start_timestamp,
    end_timestamp => NULL,
    partition_token => @partition_token,
    heartbeat_milliseconds => 5000
)

In the case where the query returns data change records, an error is thrown:

function keys() { [native code] }
 ^

SyntaxError: Unexpected token u in JSON at position 1
    at JSON.parse (<anonymous>)
    at decode (<dir>/node_modules/@google-cloud/spanner/build/src/codec.js:321:28)
    at <dir>/node_modules/@google-cloud/spanner/build/src/codec.js:332:31
    at Array.map (<anonymous>)
    at decode (<dir>/node_modules/@google-cloud/spanner/build/src/codec.js:331:45)
    at <dir>/node_modules/@google-cloud/spanner/build/src/codec.js:326:24
    at Array.map (<anonymous>)
    at decode (<dir>/node_modules/@google-cloud/spanner/build/src/codec.js:325:31)
    at <dir>/node_modules/@google-cloud/spanner/build/src/codec.js:332:31
    at Array.map (<anonymous>)
    at decode (<dir>/node_modules/@google-cloud/spanner/build/src/codec.js:331:45)
    at <dir>/node_modules/@google-cloud/spanner/build/src/codec.js:326:24
    at Array.map (<anonymous>)
    at decode (<dir>/node_modules/@google-cloud/spanner/build/src/codec.js:325:31)
    at <dir>/node_modules/@google-cloud/spanner/build/src/codec.js:332:31
    at Array.map (<anonymous>)
    at decode (<dir>/node_modules/@google-cloud/spanner/build/src/codec.js:331:45)
    at <dir>/node_modules/@google-cloud/spanner/build/src/codec.js:326:24
    at Array.map (<anonymous>)
    at Object.decode (<dir>/node_modules/@google-cloud/spanner/build/src/codec.js:325:31)
    at <dir>/node_modules/@google-cloud/spanner/build/src/partial-result-stream.js:202:49
    at Array.map (<anonymous>)

codec.js:321 – the initial source of the error – is part of the decode() function; this line specifically covers the decoding of JSON values [1]:

320   case 'JSON':
321       decoded = JSON.parse(decoded);
322       break;

Line 332 – the next level down in the stack – handles STRUCT parsing [2]:

330   case 'STRUCT':
331       fields = type.structType.fields.map(({ name, type }, index) => {
332           const value = decode(decoded[name] || decoded[index], type);
333           return { name, value };
334       });
335       decoded = Struct.fromArray(fields);
336       break;

I added some logging to the file and found that the error occurs when parsing the data-change record .mods column. .mods is an array with the following format:

{
    "name": "mods",
    "type": {
        "code": "ARRAY",
        "arrayElementType": {
            "code": "STRUCT",
            "arrayElementType": null,
            "structType": {
                "fields": [
                    {
                        "name": "keys",
                        "type": {
                            "code": "JSON",
                            "arrayElementType": null,
                            "structType": null,
                            "typeAnnotation": "TYPE_ANNOTATION_CODE_UNSPECIFIED"
                        }
                    },
                    {
                        "name": "new_values",
                        "type": {
                            "code": "JSON",
                            "arrayElementType": null,
                            "structType": null,
                            "typeAnnotation": "TYPE_ANNOTATION_CODE_UNSPECIFIED"
                        }
                    },
                    {
                        "name": "old_values",
                        "type": {
                            "code": "JSON",
                            "arrayElementType": null,
                            "structType": null,
                            "typeAnnotation": "TYPE_ANNOTATION_CODE_UNSPECIFIED"
                        }
                    }
                ]
            },
            "typeAnnotation": "TYPE_ANNOTATION_CODE_UNSPECIFIED"
        },
        "structType": null,
        "typeAnnotation": "TYPE_ANNOTATION_CODE_UNSPECIFIED"
    }
}

A sample input/not-yet-decoded .mods value:

[
    [
        "{\"entity_id\":\"d4374939-3f52-4dfe-a074-d2ca202aa5e5\"}",
        "{\"updated_at\":\"2023-01-18T08:01:40.402463Z\"}",
        "{}"
    ]
]
  • (Note that the value here for the STRUCT is an array and not an object. The existing unit tests for running decode() with a STRUCT type use an object value, and not an array. Unsure why both cases are possible.)

When we decode the first .mods array item, a STRUCT, we land at codec.js:331. The first field provided by type.structType.fields has { name: "keys", type: "JSON" }. We, at this point, have a decoded value of:

[
    "{\"entity_id\":\"d4374939-3f52-4dfe-a074-d2ca202aa5e5\"}",
    "{\"updated_at\":\"2023-01-18T08:01:40.402463Z\"}",
    "{}"
]

At codec.js:332 we should be passing over decoded[name] and using decoded[index], since decoded is an array; however since name == "keys" and Array.prototype.keys is a method, decoded[name] registers as truthy: decoded.keys is a function.

We therefore pass the Array.prototype.keys function back into decode() with type: "JSON", land at codec.js:321, and JSON.parse() fails with a function as an argument.


Solution

The apparent solution: when decoding a STRUCT, only prefer decoded[name] when decoded is not an array:

330   case 'STRUCT':
331       fields = type.structType.fields.map(({ name, type }, index) => {
332           const value = decode((!Array.isArray(decoded) && decoded[name!]) || decoded[index], type);
333           return { name, value };
334       });
335       decoded = Struct.fromArray(fields);
336       break;

Or, in the src/ Typescript code:

case 'STRUCT':
  fields = type.structType!.fields!.map(({name, type}, index) => {
    const value = decode(
      (!Array.isArray(decoded) && decoded[name!]) || decoded[index],
      type as spannerClient.spanner.v1.Type
    );
    return {name, value};
  });
  decoded = Struct.fromArray(fields as Field[]);
  break;

Indeed when this change is made, the aforementioned query works correctly.


Footnotes

  1. nodejs-spanner/src/codec.ts

    Lines 390 to 400 in ba7029a

    case 'JSON':
    if (
    type.typeAnnotation ===
    spannerClient.spanner.v1.TypeAnnotationCode.PG_JSONB ||
    type.typeAnnotation === 'PG_JSONB'
    ) {
    decoded = new PGJsonb(decoded);
    break;
    }
    decoded = JSON.parse(decoded);
    break;
  2. nodejs-spanner/src/codec.ts

    Lines 411 to 420 in ba7029a

    case 'STRUCT':
    fields = type.structType!.fields!.map(({name, type}, index) => {
    const value = decode(
    decoded[name!] || decoded[index],
    type as spannerClient.spanner.v1.Type
    );
    return {name, value};
    });
    decoded = Struct.fromArray(fields as Field[]);
    break;
@andrao andrao added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Jan 18, 2023
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/nodejs-spanner API. label Jan 18, 2023
andrao added a commit to andrao/nodejs-spanner that referenced this issue Jan 18, 2023
Where the STRUCT value is an array, always determine inner member value using field index.

STRUCT value is represented by var 'decoded'.

Refs: googleapis#1774
andrao added a commit to andrao/nodejs-spanner that referenced this issue Jan 19, 2023
Where the STRUCT value is an array, always determine inner member value using field index.

STRUCT value is represented by var 'decoded'.

Refs: googleapis#1774
surbhigarg92 pushed a commit that referenced this issue Jan 23, 2023
Where the STRUCT value is an array, always determine inner member value using field index.

STRUCT value is represented by var 'decoded'.

Refs: #1774
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/nodejs-spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
1 participant