Skip to content

impl(bigquery): add internal proto custom stream writers - #6748

Merged
haphungw merged 2 commits into
googleapis:mainfrom
haphungw:feat-bigquery-proto-custom-writers
Sep 9, 2026
Merged

impl(bigquery): add internal proto custom stream writers#6748
haphungw merged 2 commits into
googleapis:mainfrom
haphungw:feat-bigquery-proto-custom-writers

Conversation

@haphungw

@haphungw haphungw commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Add internal protobuf custom stream writers (CommittedWriter, PendingWriter, BufferedWriter) and stream creation methods on WriterBuilder.

Towards #6598

@product-auto-label product-auto-label Bot added the api: bigquery Issues related to the BigQuery API. label Sep 9, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for buffered, committed, and pending stream writers in the BigQuery protobuf write client. It extracts shared logic into a new internal BaseWriter and implements BufferedWriter, CommittedWriter, and PendingWriter, while updating WriterBuilder to support constructing them. The feedback recommends avoiding an unnecessary string allocation in PendingWriter::commit by passing a borrowed &str directly to set_parent instead of calling .to_string(), which aligns with the repository's style guide.

Comment thread src/bigquery/src/write/proto/pending.rs Outdated
@haphungw
haphungw marked this pull request as ready for review September 9, 2026 06:05
@haphungw
haphungw requested a review from a team as a code owner September 9, 2026 06:05
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.95%. Comparing base (22ce0cc) to head (7a3abc8).

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #6748    +/-   ##
========================================
  Coverage   96.95%   96.95%            
========================================
  Files         319      323     +4     
  Lines      105346   105757   +411     
========================================
+ Hits       102137   102541   +404     
- Misses       3209     3216     +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dbolduc dbolduc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to continue down this path. I am not sure if it is easier to refactor now or later.

As an idea, we can make these types generic on the schema and rows:

use crate::write::generated::gapic_storage::model::append_rows_request;

pub(crate) struct BaseWriter<T>
where T: DataFormat {
  pub(crate) runner: Runner,
  pub(crate) write_stream: String,
  pub(crate) schema: T::Schema,
  pub(crate) client: BigQueryWrite,
}

impl<T> BaseWriter {
  pub(crate) fn append_request(&self, rows: T::Rows) -> AppendRowsRequest { ... }
}

trait DataFormat {
  type Schema;
  type Rows;

  fn to_rows(schema: Self::Schema, rows: Self::Rows) -> crate::write::generated::gapic_storage::model::append_rows_request::Rows;
}

struct Proto;
impl DataFormat for Proto {
  type Schema: ProtoSchema,
  type Rows: ProtoRows,

  fn to_rows(schema: Self::Schema, rows: Self::Rows) -> append_rows_request::Rows {
    append_rows_request::Rows::ProtoRows(Box::new(
        ProtoData::new().set_writer_schema(schema).set_rows(rows)))
  }
}

// and same for Arrow

then we test that implementation once.

And define types like:

pub mod arrow {
  pub use DefaultWriter = common::DefaultWriter<Arrow>;
  // etc.
}
pub mod proto {
  pub use DefaultWriter = common::DefaultWriter<Proto>;
  // etc.
}

The one thing to look out for is if the common:: types leak in our docs. If so that would be unfortunate and an argument against this kind of unification

@haphungw
haphungw merged commit 0f689d0 into googleapis:main Sep 9, 2026
42 checks passed
@haphungw
haphungw deleted the feat-bigquery-proto-custom-writers branch September 9, 2026 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigquery Issues related to the BigQuery API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants