You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "thorfour (via GitHub)" <gi...@apache.org> on 2023/06/28 18:40:50 UTC
[GitHub] [arrow] thorfour opened a new pull request, #36365: MINOR: [Go] Schema: NumFields
thorfour opened a new pull request, #36365:
URL: https://github.com/apache/arrow/pull/36365
Expose the number of fields in a schema for iteration over fields.
<!--
Thanks for opening a pull request!
If this is your first pull request you can find detailed information on how
to contribute here:
* [New Contributor's Guide](https://arrow.apache.org/docs/dev/developers/guide/step_by_step/pr_lifecycle.html#reviews-and-merge-of-the-pull-request)
* [Contributing Overview](https://arrow.apache.org/docs/dev/developers/overview.html)
If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
Then could you also rename the pull request title in the following format?
GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
In the case of PARQUET issues on JIRA the title also supports:
PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
-->
### Rationale for this change
Previously if one wanted to iterate over the fields in the schema you would call the `Fields()` function and just iterate over the slice. However, due to [this commit](https://github.com/rtpsw/arrow/commit/802674b73c94c84388a6775b424ebe4f6e04274e) there is now an allocation and copy that happens when that's called. So to iterate over the fields without allocations one now must use the `Field(i int)` method; however that means a user must already know exactly how many fields are in the schema which isn't possible today.
This adds a simple `NumFields() int` method that returns the number of fields in a schema to allow a user to iterate over all the fields without having to copy them.
### What changes are included in this PR?
Single function added `NumFields() int` to schema
### Are these changes tested?
N/A
### Are there any user-facing changes?
Yes this is a new API
<!--
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] github-actions[bot] commented on pull request #36365: GH-36384: [Go] Schema: NumFields
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36365:
URL: https://github.com/apache/arrow/pull/36365#issuecomment-1613301560
:warning: GitHub issue #36384 **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] thorfour commented on pull request #36365: MINOR: [Go] Schema: NumFields
Posted by "thorfour (via GitHub)" <gi...@apache.org>.
thorfour commented on PR #36365:
URL: https://github.com/apache/arrow/pull/36365#issuecomment-1613198904
> Could you open a new issue for non-MINOR change instead of using `MINOR: ` title? See also our MINOR definition: https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes
Ah my bad, missed that. Will open
--
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] thorfour commented on pull request #36365: [Go] Schema: NumFields
Posted by "thorfour (via GitHub)" <gi...@apache.org>.
thorfour commented on PR #36365:
URL: https://github.com/apache/arrow/pull/36365#issuecomment-1613201632
This addresses #36384
--
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] kou commented on pull request #36365: MINOR: [Go] Schema: NumFields
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #36365:
URL: https://github.com/apache/arrow/pull/36365#issuecomment-1612125591
Could you open a new issue for non-MINOR change instead of using `MINOR: ` title?
See also our MINOR definition: https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes
--
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 #36365: GH-36384: [Go] Schema: NumFields
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #36365:
URL: https://github.com/apache/arrow/pull/36365
--
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] conbench-apache-arrow[bot] commented on pull request #36365: GH-36384: [Go] Schema: NumFields
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #36365:
URL: https://github.com/apache/arrow/pull/36365#issuecomment-1624388127
Conbench analyzed the 6 benchmark runs on commit `575b0957`.
There were 8 benchmark results indicating a performance regression:
- Commit Run on `arm64-m6g-linux-compute` at [2023-07-02 18:39:30Z](http://conbench.ursa.dev/compare/runs/7fc09a863ce547058db73f7a7afb0d60...4cd8f10aeefc482da4f33113dd50789e/)
- [params=<STATIC_VECTOR(int)>, source=cpp-micro, suite=arrow-small-vector-benchmark](http://conbench.ursa.dev/compare/benchmarks/064a0be0217d73ad8000dbf9c04355b3...064a1c4a661371578000ad8e41867ef4)
- Commit Run on `arm64-t4g-linux-compute` at [2023-07-02 18:46:34Z](http://conbench.ursa.dev/compare/runs/4b5cefb5c07044d6a1da455c2caafe0b...0b89bcf3acf649a690ebc0a787693f02/)
- [params=<UInt32Type>, source=cpp-micro, suite=arrow-value-parsing-benchmark](http://conbench.ursa.dev/compare/benchmarks/064a0bfad482734c80000fee6dffa616...064a1c6444847a0880008714872f0a0c)
- and 6 more (see the report linked below)
The [full Conbench report](https://github.com/apache/arrow/runs/14842492256) has more details.
--
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 #36365: GH-36384: [Go] Schema: NumFields
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #36365:
URL: https://github.com/apache/arrow/pull/36365#discussion_r1246859678
##########
go/arrow/schema.go:
##########
@@ -194,12 +194,13 @@ func (sc *Schema) WithEndianness(e endian.Endianness) *Schema {
func (sc *Schema) Endianness() endian.Endianness { return sc.endianness }
func (sc *Schema) IsNativeEndian() bool { return sc.endianness == endian.NativeEndian }
func (sc *Schema) Metadata() Metadata { return sc.meta }
-func (sc *Schema) Fields() []Field {
+func (sc *Schema) Fields() []Field {
fields := make([]Field, len(sc.fields))
copy(fields, sc.fields)
return fields
}
-func (sc *Schema) Field(i int) Field { return sc.fields[i] }
+func (sc *Schema) Field(i int) Field { return sc.fields[i] }
+func (sc *Schema) NumFields() int { return len(sc.fields) }
Review Comment:
Can you add a small unit test for 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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] thorfour commented on a diff in pull request #36365: GH-36384: [Go] Schema: NumFields
Posted by "thorfour (via GitHub)" <gi...@apache.org>.
thorfour commented on code in PR #36365:
URL: https://github.com/apache/arrow/pull/36365#discussion_r1247163853
##########
go/arrow/schema.go:
##########
@@ -194,12 +194,13 @@ func (sc *Schema) WithEndianness(e endian.Endianness) *Schema {
func (sc *Schema) Endianness() endian.Endianness { return sc.endianness }
func (sc *Schema) IsNativeEndian() bool { return sc.endianness == endian.NativeEndian }
func (sc *Schema) Metadata() Metadata { return sc.meta }
-func (sc *Schema) Fields() []Field {
+func (sc *Schema) Fields() []Field {
fields := make([]Field, len(sc.fields))
copy(fields, sc.fields)
return fields
}
-func (sc *Schema) Field(i int) Field { return sc.fields[i] }
+func (sc *Schema) Field(i int) Field { return sc.fields[i] }
+func (sc *Schema) NumFields() int { return len(sc.fields) }
Review Comment:
added
--
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