You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "candiduslynx (via GitHub)" <gi...@apache.org> on 2023/06/04 06:02:03 UTC
[GitHub] [arrow] candiduslynx opened a new pull request, #35899: MINOR: [Go] Add `ElemField() Field` to `arrow.ListLikeType` interface
candiduslynx opened a new pull request, #35899:
URL: https://github.com/apache/arrow/pull/35899
### Rationale for this change
Follow-up for #35885
### What changes are included in this PR?
* Added `ElemField() Field` to `arrow.ListLikeType` interface
* Added `ElemField() Field` to `arrow.MapType` implementation (just calls `arrow.ValueField`)
* To consider: remove `arrow.MapType.ValueField` & `arrow.MapType.ValueType`?
### Are these changes tested?
Compile-time assertion for corresponding types.
### Are there any user-facing changes?
* Added `ElemField() Field` to `arrow.ListLikeType` interface
* Added `ElemField() Field` to `arrow.MapType` implementation (just calls `arrow.ValueField`)
<!--
If there are any breaking changes to public APIs, please uncomment the line below and explain which changes are breaking.
-->
<!-- **This PR includes breaking changes to public APIs.** -->
<!--
Please uncomment the line below (and provide explanation) if the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld). We use this to highlight fixes to issues that may affect users without their knowledge. For this reason, fixing bugs that cause errors don't count, since those are usually obvious.
-->
<!-- **This PR contains a "Critical Fix".** -->
--
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] candiduslynx commented on a diff in pull request #35899: GH-35909: [Go] Deprecate `arrow.MapType.ValueField` & `arrow.MapType.ValueType` methods
Posted by "candiduslynx (via GitHub)" <gi...@apache.org>.
candiduslynx commented on code in PR #35899:
URL: https://github.com/apache/arrow/pull/35899#discussion_r1220061910
##########
go/parquet/pqarrow/encode_arrow.go:
##########
@@ -36,19 +36,17 @@ import (
// get the count of the number of leaf arrays for the type
func calcLeafCount(dt arrow.DataType) int {
+ if dt, ok := dt.(arrow.ListLikeType); ok {
+ return calcLeafCount(dt.Elem())
+ }
Review Comment:
Done in 9ce73c4a0e8ccbc0c1f0fdc632666df4dc947750.
I removed the panic on the union types then (as they are nested, BTW).
--
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] ursabot commented on pull request #35899: GH-35909: [Go] Deprecate `arrow.MapType.ValueField` & `arrow.MapType.ValueType` methods
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #35899:
URL: https://github.com/apache/arrow/pull/35899#issuecomment-1581697127
Benchmark runs are scheduled for baseline = 77d8bc59fbbd44b285d4f1abf02f215ec33d8ad8 and contender = daacbcc4c5f0e435b2158896584a4385dbf38986. daacbcc4c5f0e435b2158896584a4385dbf38986 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/74338effeb1249c1a036173263869e02...9772be5bc18a475288fa82031e08c0cb/)
[Finished :arrow_down:0.44% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/ba4bad8764c7437892ed3e9fbf141bae...81b3700a36094495b666a23412397055/)
[Finished :arrow_down:0.33% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/9d77f658cbee488b934148a8498d8c47...6391f0fec46244429e873c6d52b30928/)
[Finished :arrow_down:0.3% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/6635abc577c34c4284beffc738b32097...f4fede89bbda458d88fbb675a8f1623e/)
Buildkite builds:
[Finished] [`daacbcc4` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2994)
[Finished] [`daacbcc4` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/3030)
[Finished] [`daacbcc4` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2995)
[Finished] [`daacbcc4` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/3020)
[Finished] [`77d8bc59` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2993)
[Finished] [`77d8bc59` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/3029)
[Finished] [`77d8bc59` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2994)
[Finished] [`77d8bc59` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/3019)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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 #35899: GH-35909: [Go] Deprecate `arrow.MapType.ValueField` & `arrow.MapType.ValueType` methods
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35899:
URL: https://github.com/apache/arrow/pull/35899#issuecomment-1576533346
:warning: GitHub issue #35909 **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
[GitHub] [arrow] candiduslynx commented on pull request #35899: GH-35909: [Go] Deprecate `arrow.MapType.ValueField` & `arrow.MapType.ValueType` methods
Posted by "candiduslynx (via GitHub)" <gi...@apache.org>.
candiduslynx commented on PR #35899:
URL: https://github.com/apache/arrow/pull/35899#issuecomment-1577233336
@zeroshade any chance to get an initial review today?
--
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] candiduslynx commented on pull request #35899: MINOR: [Go] Add `ElemField() Field` to `arrow.ListLikeType` interface
Posted by "candiduslynx (via GitHub)" <gi...@apache.org>.
candiduslynx commented on PR #35899:
URL: https://github.com/apache/arrow/pull/35899#issuecomment-1575413307
@zeroshade I wonder if you'll be open to removing the `ValueType` & `ValueField` for `arrow.MapType` (so that it doesn't have the duplicate methods with the same results)?
--
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] candiduslynx commented on pull request #35899: GH-35909: [Go] Deprecate `arrow.MapType.ValueField` & `arrow.MapType.ValueType` methods
Posted by "candiduslynx (via GitHub)" <gi...@apache.org>.
candiduslynx commented on PR #35899:
URL: https://github.com/apache/arrow/pull/35899#issuecomment-1579250375
@zeroshade I think the tests have passed.
Any additional issues in the PR?
--
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 #35899: GH-35909: [Go] Deprecate `arrow.MapType.ValueField` & `arrow.MapType.ValueType` methods
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #35899:
URL: https://github.com/apache/arrow/pull/35899#discussion_r1219920936
##########
go/parquet/pqarrow/encode_arrow.go:
##########
@@ -36,19 +36,17 @@ import (
// get the count of the number of leaf arrays for the type
func calcLeafCount(dt arrow.DataType) int {
+ if dt, ok := dt.(arrow.ListLikeType); ok {
+ return calcLeafCount(dt.Elem())
+ }
Review Comment:
could we further simply this by doing
```go
if dt, ok := dt.(arrow.NestedType); ok {
nleaves := 0
for _, f := range dt.Fields() {
nleaves += calcLeafCount(f.Type)
}
return nleaves
}
```
Which will cover struct types in addition to the list/map types?
--
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 #35899: GH-35909: [Go] Deprecate `arrow.MapType.ValueField` & `arrow.MapType.ValueType` methods
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #35899:
URL: https://github.com/apache/arrow/pull/35899
--
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