You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "hermanschaaf (via GitHub)" <gi...@apache.org> on 2023/04/17 14:38:11 UTC
[GitHub] [arrow] hermanschaaf opened a new pull request, #35189: GH-35188: [Go] Use AppendValueFromString for extension types in CSV Reader
hermanschaaf opened a new pull request, #35189:
URL: https://github.com/apache/arrow/pull/35189
Rather than adding quotes and using `json.NewDecoder` to decode extension types, we should use `AppendValueFromString` which provides the opposite operation to `ValueStr`.
Closes https://github.com/apache/arrow/issues/35188
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] hermanschaaf commented on a diff in pull request #35189: GH-35188: [Go] Use AppendValueFromString for extension types in CSV Reader
Posted by "hermanschaaf (via GitHub)" <gi...@apache.org>.
hermanschaaf commented on code in PR #35189:
URL: https://github.com/apache/arrow/pull/35189#discussion_r1168873398
##########
go/internal/types/extension_types.go:
##########
@@ -117,6 +117,14 @@ func (b *UUIDBuilder) UnmarshalJSON(data []byte) error {
return b.Unmarshal(dec)
}
+func (b *UUIDBuilder) AppendValueFromString(s string) error {
+ if s == array.NullValueStr {
+ b.AppendNull()
+ return nil
+ }
+ return b.UnmarshalOne(json.NewDecoder(strings.NewReader(`"` + s + `"`)))
Review Comment:
Good call, updated 👍
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #35189: GH-35188: [Go] Use AppendValueFromString for extension types in CSV Reader
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35189:
URL: https://github.com/apache/arrow/pull/35189#issuecomment-1511497954
* Closes: #35188
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] zeroshade merged pull request #35189: GH-35188: [Go] Use AppendValueFromString for extension types in CSV Reader
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #35189:
URL: https://github.com/apache/arrow/pull/35189
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] hermanschaaf commented on a diff in pull request #35189: GH-35188: [Go] Use AppendValueFromString for extension types in CSV Reader
Posted by "hermanschaaf (via GitHub)" <gi...@apache.org>.
hermanschaaf commented on code in PR #35189:
URL: https://github.com/apache/arrow/pull/35189#discussion_r1168858264
##########
go/internal/types/extension_types.go:
##########
@@ -117,6 +117,14 @@ func (b *UUIDBuilder) UnmarshalJSON(data []byte) error {
return b.Unmarshal(dec)
}
+func (b *UUIDBuilder) AppendValueFromString(s string) error {
+ if s == array.NullValueStr {
+ b.AppendNull()
+ return nil
+ }
+ return b.UnmarshalOne(json.NewDecoder(strings.NewReader(`"` + s + `"`)))
Review Comment:
Good point, I think we can do that; will give it a try!
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] zeroshade commented on a diff in pull request #35189: GH-35188: [Go] Use AppendValueFromString for extension types in CSV Reader
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #35189:
URL: https://github.com/apache/arrow/pull/35189#discussion_r1168838344
##########
go/internal/types/extension_types.go:
##########
@@ -117,6 +117,14 @@ func (b *UUIDBuilder) UnmarshalJSON(data []byte) error {
return b.Unmarshal(dec)
}
+func (b *UUIDBuilder) AppendValueFromString(s string) error {
+ if s == array.NullValueStr {
+ b.AppendNull()
+ return nil
+ }
+ return b.UnmarshalOne(json.NewDecoder(strings.NewReader(`"` + s + `"`)))
Review Comment:
why not just call `uuid.Parse` directly here?
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #35189: GH-35188: [Go] Use AppendValueFromString for extension types in CSV Reader
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35189:
URL: https://github.com/apache/arrow/pull/35189#issuecomment-1511498008
:warning: GitHub issue #35188 **has been automatically assigned in GitHub** to PR creator.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org