joellubi commented on code in PR #1456:
URL: https://github.com/apache/arrow-adbc/pull/1456#discussion_r1459090880
##########
go/adbc/driver/snowflake/record_reader.go:
##########
@@ -389,7 +386,15 @@ func jsonDataToArrow(ctx context.Context, bldr
*array.RecordBuilder, ld gosnowfl
return nil, err
}
- if tz != time.UTC {
+ snowflakeType, ok :=
bldr.Schema().Field(i).Metadata.GetValue(MetadataKeySnowflakeType)
+ if !ok {
+ return nil, errToAdbcErr(
+ adbc.StatusInvalidData,
+ fmt.Errorf("key %s not found in
metadata for field %s", MetadataKeySnowflakeType, bldr.Schema().Field(i).Name),
+ )
+ }
Review Comment:
We do guarantee it because it is added in `rowTypesToArrowSchema`, which is
always called before `jsonDataToArrow`. If this method was used elsewhere in
the future without specifically producing the schema with
`rowTypesToArrowSchema` first then it could cause issues though. Perhaps it is
better to fail quickly in that case to ensure that kind of change isn't made in
the future. This does make me think about the case where the key is present but
has the default empty string value. The current behavior is to fall through to
the TIMESTAMP_TZ case, which I'm not sure is correct. I'll put up a revision to
this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
