GetRowsOptions Type Error for startIndex: Expects a string when it should expect a number

[arjunmehta]

Environment details

• OS: macOS 14.4.1
• Node.js version: 20.11
• npm version: 10.2.4
• @google-cloud/bigquery version: 7.5.2

Steps to reproduce

1. Try to pass startIndex as an integer option to getRows. eg.table.getRows({ startIndex: 31241 }).
2. Observe typescript error. Expects a string when this should be a number.
3. Passing a number in as "string" allows it to compile.

Expected behaviour

startIndex should work with an integer. As it is right now I need to convert the integer to a string eg. 31241.toString()

Relevant fixes in the code

https://github.com/googleapis/nodejs-bigquery/blob/74aa1501452c36af7969bb4a46b996485d9ca91b/src/types.d.ts#L5903 https://github.com/googleapis/nodejs-bigquery/blob/74aa1501452c36af7969bb4a46b996485d9ca91b/src/types.d.ts#L6055

[alvarowolfx]

The types.d.ts file is auto generated from the BigQuery Discovery file. On the service side, the startIndex field is declared as a string with uint64 format.

[alvarowolfx]

Copying the comments from PR #1351

The Discovery document represents the backend/API definition, so it can only be changed by the service side itself. In this case, would be a change on the BigQuery backend. What can be done is to change the generated code to take into consideration the format field of the Discovery doc, not just the type field. But in the end what the service side accepts is a string, so some conversion would have to happen. Another thing to consider is that the startIndex attribute expects an uint64 and that can't be represented with a integer in Javascript, which holds at most 2^53 - 1. So users would have to use a BigInt and convert to string anyway.

So in summary, it still makes sense to accept a string here because of the format, if we support number, some conversion code would have to be added.

[arjunmehta]

@alvarowolfx Oh I see. Seems like a mistake to me, especially because maxResults, though similar but oddly uint32, is defined as an integer.

From https://www.googleapis.com/discovery/v1/apis/bigquery/v2/rest

                        "maxResults":
                        {
                            "description": "Maximum number of results to read.",
                            "format": "uint32",
                            "location": "query",
                            "type": "integer"
                        },

On the Typescript side, would you agree it seems like a bug to have to adapt an integer index to be a string in order to pass through?

Obviously this is beyond an issue with this module, but not sure where the relevant upstream issue should be tracked.

[alvarowolfx]

the key difference is the format, some programming languages doesn't have native support for 64 bit integers, that's why the service side has to accept it in string format. The example that you have with maxResults is limited to uint32 and 32 bit integers are a common thing in programming languages. Under the hood the services are defined using Protobuf, so that behavior and handling of data formats comes from that.

[arjunmehta]

@alvarowolfx Got it, reading https://developers.google.com/discovery/v1/type-format a bit more and I see the reason why Uint64 is a string more clearly. Though from a TypeScript standpoint still seems strange, as I could pass a string "HEY THIS IS CRAZY" to startIndex and would not get any type errors.

But to continue the discussion, when not using TypeScript, the module already works by passing in a literal integer.

Typescript can be very adaptable to support integers AND strings as a type definition. I would propose the issue is actually on the upstream https://github.com/callmehiphop/discovery-tsd/blob/master/src/converter.js

Personally for Typescript, I would convert the following schema to a number | string:

"startIndex": {
  "description": "Zero-based index of the starting row.",
  "format": "uint64",
  "location": "query",
  "type": "string"
},

So if (schema.type === 'string' && (schema.format === 'uint64' || schema.format === 'int64')) then string | number

I could propose in a PR on https://github.com/callmehiphop/discovery-tsd

Some background on my end, this issue arose when converting an existing project to use Typescript, and I was surprised by the definition.