Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for streaming Arrow results from explicit destination tables in createQueryArrowStream using the BigQuery Storage Read API. The review feedback identifies three robustness issues: a potential unhandled exception if getStorageReadClient fails, a missing validation check for parentProjectId which could result in an invalid resource path, and a potential TypeError if query is null during the destination check.
f2edf85 to
e70ca28
Compare
a83abe3 to
566de0a
Compare
04e682f to
a13292a
Compare
a13292a to
a8e1848
Compare
a8e1848 to
508a4a2
Compare
b4cc0d4 to
4dd87b7
Compare
bccdb5b to
3e8968a
Compare
4d56cd1 to
3e8968a
Compare
| callback: (err: Error | null, schema?: Uint8Array | string) => void, | ||
| ): void { | ||
| const {bq, storage} = ctx; | ||
| const {bq} = ctx; |
There was a problem hiding this comment.
I'm not sure if destructuring the bigquery client is necessary here
There was a problem hiding this comment.
Removed; it uses ctx.bq directly.
|
|
||
| const sessionSchema = session?.arrowSchema?.serializedSchema || undefined; | ||
| const readStream = session?.streams?.[0]?.name; | ||
| if (!readStream) { |
There was a problem hiding this comment.
Can we also include a check that session.streams.length === 0?
There was a problem hiding this comment.
Added. It checks streams.length === 0 (or a missing stream name) before reading streams[0], and emits the 0-row batch with the session schema.
| ctx: ArrowStreamContext, | ||
| parentProjectId: string | undefined, | ||
| table: bigquery.ITableReference, | ||
| callback: (err: Error | null, session?: IReadSession) => void, |
There was a problem hiding this comment.
Nit: this is a lot of callback nesting. This function could be refactored to be a Promise instead
There was a problem hiding this comment.
Done. createArrowReadSession returns a Promise now, and streamDestinationTable/readDestinationSchema await it, so there's no callback nesting left.
Read results of jobs.insert queries with an explicit destination table through BigQueryReadClient.createReadSession (dataFormat: ARROW, maxStreamCount: 1) once the job completes, instead of the job's default stream, which only serves anonymous destination tables. An empty destination table returns no streams, so emit one 0-row RecordBatch with the session's Arrow schema.
3e8968a to
a32866f
Compare
…ow-destination-table
…ow-destination-table
Throw a clear error instead of creating a read session with parent "projects/undefined" when no project ID can be resolved. Note in the createQueryArrowStream JSDoc that an explicit destination table is read in full through a read session after the query finishes.
A query with an explicit
destinationtable writes its results there instead of to an anonymous table, so there's nostreams/_defaultto read. This reads the destination table through a read session once the job finishes.createReadSessionwithdataFormat: 'ARROW'andmaxStreamCount: 1(one stream keeps rows in the query's order) and readsstreams[0]fromoffset = 0.RecordBatchwith the session's schema.projects/undefined.