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

SNOW-949967 Add an optional offset token parameter for openChannel #645

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

sfc-gh-asen
Copy link
Contributor

Adds an optional offsetToken field to OpenChannelRequest.java which will overwrite existing offset token if provided.

@@ -40,6 +40,9 @@ public enum OnErrorOption {
// Default timezone for TIMESTAMP_LTZ and TIMESTAMP_TZ columns
private final ZoneId defaultTimezone;

private final String offsetToken;
private final boolean isOffsetTokenProvided;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain why isOffsetTokenProvided is required?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is to distinguish null as an offset token vs. it not being set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, i want to support setting the offsetToken as null as well.

In order to distinguish between default null string vs user provided null, I am using this extra flag.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, could you add a IT test for NULL?

@sfc-gh-asen sfc-gh-asen merged commit 4450fc4 into master Oct 25, 2023
12 checks passed
@sfc-gh-asen sfc-gh-asen deleted the asen-SNOW-949967-offset-token branch October 25, 2023 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants