You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "lidavidm (via GitHub)" <gi...@apache.org> on 2023/02/02 18:20:55 UTC
[GitHub] [arrow-adbc] lidavidm opened a new pull request, #409: fix(go/adbc/driver/flightsql): guard against inconsistent schemas
lidavidm opened a new pull request, #409:
URL: https://github.com/apache/arrow-adbc/pull/409
In case the FlightInfo schema doesn't match the DoGet schema, return an error instead of allowing the client to misinterpret the result.
--
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-adbc] zeroshade commented on a diff in pull request #409: fix(go/adbc/driver/flightsql): guard against inconsistent schemas
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #409:
URL: https://github.com/apache/arrow-adbc/pull/409#discussion_r1094946334
##########
go/adbc/driver/flightsql/record_reader.go:
##########
@@ -201,3 +208,23 @@ func (r *reader) Schema() *arrow.Schema {
func (r *reader) Record() arrow.Record {
return r.rec
}
+
+func removeSchemaMetadata(schema *arrow.Schema) *arrow.Schema {
+ fields := make([]arrow.Field, len(schema.Fields()))
+ for i, field := range schema.Fields() {
+ fields[i] = removeFieldMetadata(&field)
+ }
+ return arrow.NewSchema(fields, nil)
+}
+
+func removeFieldMetadata(field *arrow.Field) arrow.Field {
+ // XXX: this should recurse into the child fields, but there's not
+ // an easy way to navigate this generically - we can improve this
+ // upstream
+ return arrow.Field{
+ Name: field.Name,
+ Type: field.Type,
+ Nullable: field.Nullable,
+ Metadata: arrow.Metadata{},
+ }
Review Comment:
Could do:
```go
if n, ok := field.Type.(arrow.NestedType); ok {
for i, f := range n.Fields() {
// stuff
}
}
```
?
Though upstream we should make an `EqualWithoutMetadata` for fields or something of the like....
--
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-adbc] lidavidm commented on a diff in pull request #409: fix(go/adbc/driver/flightsql): guard against inconsistent schemas
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #409:
URL: https://github.com/apache/arrow-adbc/pull/409#discussion_r1094948582
##########
go/adbc/driver/flightsql/record_reader.go:
##########
@@ -201,3 +208,23 @@ func (r *reader) Schema() *arrow.Schema {
func (r *reader) Record() arrow.Record {
return r.rec
}
+
+func removeSchemaMetadata(schema *arrow.Schema) *arrow.Schema {
+ fields := make([]arrow.Field, len(schema.Fields()))
+ for i, field := range schema.Fields() {
+ fields[i] = removeFieldMetadata(&field)
+ }
+ return arrow.NewSchema(fields, nil)
+}
+
+func removeFieldMetadata(field *arrow.Field) arrow.Field {
+ // XXX: this should recurse into the child fields, but there's not
+ // an easy way to navigate this generically - we can improve this
+ // upstream
+ return arrow.Field{
+ Name: field.Name,
+ Type: field.Type,
+ Nullable: field.Nullable,
+ Metadata: arrow.Metadata{},
+ }
Review Comment:
(that said: not like you can in C++ or Java either.)
--
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-adbc] lidavidm commented on a diff in pull request #409: fix(go/adbc/driver/flightsql): guard against inconsistent schemas
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #409:
URL: https://github.com/apache/arrow-adbc/pull/409#discussion_r1094947811
##########
go/adbc/driver/flightsql/record_reader.go:
##########
@@ -201,3 +208,23 @@ func (r *reader) Schema() *arrow.Schema {
func (r *reader) Record() arrow.Record {
return r.rec
}
+
+func removeSchemaMetadata(schema *arrow.Schema) *arrow.Schema {
+ fields := make([]arrow.Field, len(schema.Fields()))
+ for i, field := range schema.Fields() {
+ fields[i] = removeFieldMetadata(&field)
+ }
+ return arrow.NewSchema(fields, nil)
+}
+
+func removeFieldMetadata(field *arrow.Field) arrow.Field {
+ // XXX: this should recurse into the child fields, but there's not
+ // an easy way to navigate this generically - we can improve this
+ // upstream
+ return arrow.Field{
+ Name: field.Name,
+ Type: field.Type,
+ Nullable: field.Nullable,
+ Metadata: arrow.Metadata{},
+ }
Review Comment:
`// stuff` is the hard part because you can't generically reconstruct the type instance, right?
--
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-adbc] lidavidm commented on pull request #409: fix(go/adbc/driver/flightsql): guard against inconsistent schemas
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #409:
URL: https://github.com/apache/arrow-adbc/pull/409#issuecomment-1416069819
D'oh, staticcheck broke because it now needs go 1.19.
--
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-adbc] lidavidm merged pull request #409: fix(go/adbc/driver/flightsql): guard against inconsistent schemas
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #409:
URL: https://github.com/apache/arrow-adbc/pull/409
--
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-adbc] lidavidm commented on a diff in pull request #409: fix(go/adbc/driver/flightsql): guard against inconsistent schemas
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #409:
URL: https://github.com/apache/arrow-adbc/pull/409#discussion_r1095953940
##########
go/adbc/driver/flightsql/record_reader.go:
##########
@@ -201,3 +208,23 @@ func (r *reader) Schema() *arrow.Schema {
func (r *reader) Record() arrow.Record {
return r.rec
}
+
+func removeSchemaMetadata(schema *arrow.Schema) *arrow.Schema {
+ fields := make([]arrow.Field, len(schema.Fields()))
+ for i, field := range schema.Fields() {
+ fields[i] = removeFieldMetadata(&field)
+ }
+ return arrow.NewSchema(fields, nil)
+}
+
+func removeFieldMetadata(field *arrow.Field) arrow.Field {
+ // XXX: this should recurse into the child fields, but there's not
+ // an easy way to navigate this generically - we can improve this
+ // upstream
+ return arrow.Field{
+ Name: field.Name,
+ Type: field.Type,
+ Nullable: field.Nullable,
+ Metadata: arrow.Metadata{},
+ }
Review Comment:
I went ahead and implemented it.
--
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-adbc] lidavidm commented on pull request #409: fix(go/adbc/driver/flightsql): guard against inconsistent schemas
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #409:
URL: https://github.com/apache/arrow-adbc/pull/409#issuecomment-1416071311
Pinned to 0.3.3 for now.
--
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