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

GH-36568: [Go] Include Timestamp Zone in ValueStr #36569

Merged
merged 2 commits into from
Jul 10, 2023

Conversation

zeroshade
Copy link
Member

@zeroshade zeroshade commented Jul 8, 2023

Rationale for this change

While trying to fix an issue with Snowflake ADBC timestamp handling, I came across this as part of the problem.

What changes are included in this PR?

Adds the timezone string indicator to the output of ValueStr for a timestamp array.

@zeroshade zeroshade requested a review from lidavidm July 8, 2023 17:35
@github-actions
Copy link

github-actions bot commented Jul 8, 2023

⚠️ GitHub issue #36568 has been automatically assigned in GitHub to PR creator.

Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

Is there a test case we can add?

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Jul 8, 2023
@zeroshade
Copy link
Member Author

@lidavidm test case added

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jul 10, 2023
@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Jul 10, 2023
@zeroshade zeroshade merged commit 70f4476 into apache:main Jul 10, 2023
24 checks passed
@zeroshade zeroshade removed the awaiting merge Awaiting merge label Jul 10, 2023
@zeroshade zeroshade deleted the timestamp-zone-handling branch July 10, 2023 20:25
lidavidm pushed a commit to apache/arrow-adbc that referenced this pull request Jul 11, 2023
raulcd pushed a commit that referenced this pull request Jul 11, 2023
### Rationale for this change
While trying to fix an issue with Snowflake ADBC timestamp handling, I came across this as part of the problem.

### What changes are included in this PR?
Adds the timezone string indicator to the output of `ValueStr` for a timestamp array.

* Closes: #36568

Authored-by: Matt Topol <zotthewizard@gmail.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 70f4476.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Go] Include Timestamp Zone in ValueStr output
2 participants