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-911238: Add WithOriginalTimestamp context switch #916

Closed
wants to merge 2 commits into from

Conversation

sfc-gh-pfus
Copy link
Collaborator

@sfc-gh-pfus sfc-gh-pfus commented Sep 15, 2023

Description

Add WithOriginalTimestamp context switch.

Checklist

  • Code compiles correctly
  • Run make fmt to fix inconsistent formats
  • Run make lint to get lint errors and fix all of them
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary

@sfc-gh-dstempniak sfc-gh-dstempniak force-pushed the SNOW-911238-timestamp-overflow branch 2 times, most recently from 0eaf5dc to b89711e Compare September 25, 2023 07:52
@sfc-gh-dstempniak sfc-gh-dstempniak force-pushed the SNOW-911238-timestamp-overflow branch from b89711e to dd1f4c7 Compare September 25, 2023 08:49
@sfc-gh-dstempniak sfc-gh-dstempniak changed the title SNOW-911238: Check timestamp overflow SNOW-911238: Add WithOriginalTimestamp context switch Sep 25, 2023
arrow_chunk.go Outdated Show resolved Hide resolved
cmd/arrow/batches/arrow_batches.go Outdated Show resolved Hide resolved
converter.go Outdated Show resolved Hide resolved
converter_test.go Show resolved Hide resolved
converter_test.go Outdated Show resolved Hide resolved
converter_test.go Outdated Show resolved Hide resolved
util.go Show resolved Hide resolved
// ArrowSnowflakeTimestampToTime converts original timestamp returned by Snowflake to time.TIme
func ArrowSnowflakeTimestampToTime(rec arrow.Record, rb *ArrowBatch, colIdx int, recIdx int) *time.Time {
scale := int(rb.scd.RowSet.RowType[colIdx].Scale)
dbType := strings.ToUpper(rb.scd.RowSet.RowType[colIdx].Type)
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps we can encapsulate scale, dbtype and loc in a new structure? that way we wouldn't have to pass the whole ArrowBatch to the method? Just an idea

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it is a good option. In such case a driver user will have to extract data from ArrowBatch by himself and create such object to pass. Moreover scd and loc aren't public.

converter.go Show resolved Hide resolved
converter.go Show resolved Hide resolved
converter.go Outdated Show resolved Hide resolved
@github-actions github-actions bot locked and limited conversation to collaborators Oct 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants