You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/04/22 05:12:20 UTC
[GitHub] [arrow] houqp opened a new pull request #7009: ARROW-8552: [Rust] support iterate parquet row columns
houqp opened a new pull request #7009:
URL: https://github.com/apache/arrow/pull/7009
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on issue #7009: ARROW-8552: [Rust] support iterate parquet row columns
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on issue #7009:
URL: https://github.com/apache/arrow/pull/7009#issuecomment-617555901
https://issues.apache.org/jira/browse/ARROW-8552
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on a change in pull request #7009: ARROW-8552: [Rust] support iterate parquet row columns
Posted by GitBox <gi...@apache.org>.
andygrove commented on a change in pull request #7009:
URL: https://github.com/apache/arrow/pull/7009#discussion_r413857080
##########
File path: rust/parquet/src/record/api.rs
##########
@@ -50,6 +50,33 @@ impl Row {
pub fn len(&self) -> usize {
self.fields.len()
}
+
+ pub fn get_column_iter(&self) -> RowColumnIter {
+ RowColumnIter {
+ fields: &self.fields,
+ curr: 0,
+ count: self.fields.len(),
+ }
+ }
+}
+
+pub struct RowColumnIter<'a> {
+ fields: &'a Vec<(String, Field)>,
+ curr: usize,
+ count: usize,
+}
+
+impl<'a> Iterator for RowColumnIter<'a> {
+ type Item = (usize, &'a String);
Review comment:
Why do we need the index included in the iterator results? The user could use `enumerate` to get those? Also, would it make more sense for the iterator be over the `Field` rather than just field name?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nevi-me commented on issue #7009: ARROW-8552: [Rust] support iterate parquet row columns
Posted by GitBox <gi...@apache.org>.
nevi-me commented on issue #7009:
URL: https://github.com/apache/arrow/pull/7009#issuecomment-617678597
> looks like the windows CI is failing with error not related to my change:
>
> ```
> "error: \'rustfmt.exe\' is not installed for the toolchain \'nightly-2019-11-14-x86_64-pc-windows-msvc\'\nTo install
> ```
This will be fixed by #7010. May you please kindly rebase once it's merged
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] houqp commented on issue #7009: ARROW-8552: [Rust] support iterate parquet row columns
Posted by GitBox <gi...@apache.org>.
houqp commented on issue #7009:
URL: https://github.com/apache/arrow/pull/7009#issuecomment-618158794
@nevi-me rebased and tests are passing 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] houqp commented on a change in pull request #7009: ARROW-8552: [Rust] support iterate parquet row columns
Posted by GitBox <gi...@apache.org>.
houqp commented on a change in pull request #7009:
URL: https://github.com/apache/arrow/pull/7009#discussion_r414007919
##########
File path: rust/parquet/src/record/api.rs
##########
@@ -50,6 +50,33 @@ impl Row {
pub fn len(&self) -> usize {
self.fields.len()
}
+
+ pub fn get_column_iter(&self) -> RowColumnIter {
+ RowColumnIter {
+ fields: &self.fields,
+ curr: 0,
+ count: self.fields.len(),
+ }
+ }
+}
+
+pub struct RowColumnIter<'a> {
+ fields: &'a Vec<(String, Field)>,
+ curr: usize,
+ count: usize,
+}
+
+impl<'a> Iterator for RowColumnIter<'a> {
+ type Item = (usize, &'a String);
Review comment:
Thanks for the suggestions, I have changed it to return column name and field as a tuple.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] houqp commented on issue #7009: ARROW-8552: [Rust] support iterate parquet row columns
Posted by GitBox <gi...@apache.org>.
houqp commented on issue #7009:
URL: https://github.com/apache/arrow/pull/7009#issuecomment-617600824
looks like the windows CI is failing with error not related to my change:
```
"error: \'rustfmt.exe\' is not installed for the toolchain \'nightly-2019-11-14-x86_64-pc-windows-msvc\'\nTo install
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] houqp commented on issue #7009: ARROW-8552: [Rust] support iterate parquet row columns
Posted by GitBox <gi...@apache.org>.
houqp commented on issue #7009:
URL: https://github.com/apache/arrow/pull/7009#issuecomment-617914513
Thanks @nevi-me for the fix, looks like your PR has been approved. I will wait for the merge.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org