Skip to content

Enable partition decorator syntax for Storage Write API #69

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Enable partition decorator syntax for Storage Write API #69

wants to merge 11 commits into from

Conversation

podgaietska
Copy link
Contributor

Overview

Storage Write API recently added support for partition decorator syntax for default streams. This PR enables that feature in the connector’s Storage Write API path.

What Changed

  • Refactored table resolution logic to unify it for insertAll and Storage Write API paths.
  • Storage Write Api JsonStreamWriters for default stream now constructed with decorated table name when using partition decorator syntax.
  • Added a validation check to prevent use of partition decorator syntax with Storage Write API batch load mode, since Application Streams do not yet support it. If implicitly enabled, flag is ignored.
  • Explicit fail-fast for writes to unpartitioned or incorrectly partitioned tables when using decorator syntax, as JsonStreamWriter provides limited error details in these cases.

@podgaietska podgaietska mentioned this pull request Jul 23, 2025
Merged
@podgaietska podgaietska force-pushed the storage-write-api-enable-partition-decorator branch from 2b987ed to f5809d8 Compare July 28, 2025 18:18
agrawal-siddharth
agrawal-siddharth suggested changes Jul 29, 2025
View reviewed changes
kcbq-connector/src/main/java/com/wepay/kafka/connect/bigquery/RecordTableResolver.java
@@ -0,0 +1,142 @@
/*
* Copyright 2024 Copyright 2022 Aiven Oy and
Copy link

Choose a reason for hiding this comment

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

You can change the date to 2025.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry. I missed that. Please change the date to be one Copyright date of 2025

Copy link
Contributor Author

@podgaietska podgaietska Aug 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@Claudenw It fails the license header check if I change it to 2025 because mvn license:remove license:format regenerates headers to the way they are now:
https://github.com/Aiven-Open/bigquery-connector-for-apache-kafka/actions/runs/16635421449/job/47096590208

Unless I am missing something.

Claudenw
Claudenw requested changes Jul 30, 2025
View reviewed changes
Copy link
Contributor

@Claudenw Claudenw left a comment

Choose a reason for hiding this comment

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

Please add boiler plate headers to the new files as @agrawal-siddharth has pointed out.

Once the code looks good, I'll run the tests and if they pass merge it.

@Claudenw
Copy link
Contributor

@podgaietska

Please copy the header from one of the other source files and do not modify it. The verification test looks for exact text and format. I hope to switch to a different tool in the future but for now we do not allow any changes to the license block.

Please fix the headers in the following files:

kcbq-connector/src/main/java/com/wepay/kafka/connect/bigquery/RecordTableResolver.java
kcbq-connector/src/test/java/com/wepay/kafka/connect/bigquery/RecordTableResolverTest.java
kcbq-connector/src/test/java/com/wepay/kafka/connect/bigquery/integration/utils/TimePartitioningTestUtils.java

Claudenw
Claudenw requested changes Aug 1, 2025
View reviewed changes
Copy link
Contributor

@Claudenw Claudenw left a comment

Choose a reason for hiding this comment

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

Please change the date to be one Copyright date of 2025 in the 3 new files. It should only have the original date.

kcbq-connector/src/main/java/com/wepay/kafka/connect/bigquery/RecordTableResolver.java
@@ -0,0 +1,142 @@
/*
* Copyright 2024 Copyright 2022 Aiven Oy and
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry. I missed that. Please change the date to be one Copyright date of 2025

@podgaietska
Copy link
Contributor Author

@Claudenw could you please re-run the tests? Not sure why the failure. It passes locally fine

@Claudenw
Copy link
Contributor

Claudenw commented Aug 8, 2025

@podgaietska please rebase

@podgaietska podgaietska force-pushed the storage-write-api-enable-partition-decorator branch from 9c4d93c to 4329d7a Compare August 8, 2025 23:26
@podgaietska podgaietska requested a review from Claudenw August 13, 2025 16:57
@podgaietska podgaietska force-pushed the storage-write-api-enable-partition-decorator branch from 4329d7a to 906d562 Compare August 15, 2025 20:12
@podgaietska
Copy link
Contributor Author

Hi @Claudenw. Just curious what is happening with this PR? Is there anything else needed from me?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Claudenw Claudenw Awaiting requested review from Claudenw

1 more reviewer

@agrawal-siddharth agrawal-siddharth agrawal-siddharth approved these changes

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@podgaietska @Claudenw @agrawal-siddharth