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 2022/04/13 14:29:38 UTC

[GitHub] [arrow-rs] tustvold opened a new pull request, #1560: Support empty projection in ParquetRecordBatchReader

tustvold opened a new pull request, #1560:
URL: https://github.com/apache/arrow-rs/pull/1560

   # Which issue does this PR close?
   
   Closes #1537
   
   # Rationale for this change
    
   See ticket
   
   # What changes are included in this PR?
   
   See ticket
   
   # Are there any user-facing changes?
   
   No
   


-- 
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-rs] viirya commented on a diff in pull request #1560: Support empty projection in ParquetRecordBatchReader

Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #1560:
URL: https://github.com/apache/arrow-rs/pull/1560#discussion_r849739790


##########
parquet/src/arrow/array_reader/empty_array.rs:
##########
@@ -0,0 +1,75 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::arrow::array_reader::ArrayReader;
+use crate::errors::Result;
+use arrow::array::{ArrayDataBuilder, ArrayRef, StructArray};
+use arrow::datatypes::DataType as ArrowType;
+use std::any::Any;
+use std::sync::Arc;
+
+/// Returns an [`ArrayReader`] that yields [`StructArray`] with no columns
+/// but with row counts that correspond to the amount of data in the file
+///
+/// This is useful for when projection eliminates all columns within a collection
+pub fn make_empty_array_reader(row_count: usize) -> Box<dyn ArrayReader> {
+    Box::new(EmptyArrayReader::new(row_count))
+}
+
+struct EmptyArrayReader {
+    data_type: ArrowType,

Review Comment:
   `data_type` seems not necessary to keep here, it is always `ArrowType::Struct(vec![])`.



-- 
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-rs] tustvold commented on a diff in pull request #1560: Support empty projection in ParquetRecordBatchReader

Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #1560:
URL: https://github.com/apache/arrow-rs/pull/1560#discussion_r849553157


##########
parquet/src/arrow/array_reader/empty_array.rs:
##########
@@ -0,0 +1,58 @@
+use crate::arrow::array_reader::ArrayReader;
+use crate::errors::Result;
+use arrow::array::{ArrayDataBuilder, ArrayRef, StructArray};
+use arrow::datatypes::DataType as ArrowType;
+use std::any::Any;
+use std::sync::Arc;
+
+/// Returns an [`ArrayReader`] that yields [`StructArray`] with no columns
+/// but with row counts that correspond to the amount of data in the file
+///
+/// This is useful for when projection eliminates all columns within a collection
+pub fn make_empty_array_reader(row_count: usize) -> Box<dyn ArrayReader> {

Review Comment:
   I don't like this name, but it was the best I could come up with



-- 
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-rs] viirya commented on a diff in pull request #1560: Support empty projection in ParquetRecordBatchReader

Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #1560:
URL: https://github.com/apache/arrow-rs/pull/1560#discussion_r849744849


##########
parquet/src/arrow/array_reader/empty_array.rs:
##########
@@ -0,0 +1,75 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::arrow::array_reader::ArrayReader;
+use crate::errors::Result;
+use arrow::array::{ArrayDataBuilder, ArrayRef, StructArray};
+use arrow::datatypes::DataType as ArrowType;
+use std::any::Any;
+use std::sync::Arc;
+
+/// Returns an [`ArrayReader`] that yields [`StructArray`] with no columns
+/// but with row counts that correspond to the amount of data in the file
+///
+/// This is useful for when projection eliminates all columns within a collection
+pub fn make_empty_array_reader(row_count: usize) -> Box<dyn ArrayReader> {
+    Box::new(EmptyArrayReader::new(row_count))
+}
+
+struct EmptyArrayReader {
+    data_type: ArrowType,

Review Comment:
   yea, just found it and want to go to comment. you already replied. nvm.



-- 
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-rs] alamb merged pull request #1560: Support empty projection in ParquetRecordBatchReader

Posted by GitBox <gi...@apache.org>.
alamb merged PR #1560:
URL: https://github.com/apache/arrow-rs/pull/1560


-- 
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-rs] tustvold commented on a diff in pull request #1560: Support empty projection in ParquetRecordBatchReader

Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #1560:
URL: https://github.com/apache/arrow-rs/pull/1560#discussion_r849741967


##########
parquet/src/arrow/array_reader/empty_array.rs:
##########
@@ -0,0 +1,75 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::arrow::array_reader::ArrayReader;
+use crate::errors::Result;
+use arrow::array::{ArrayDataBuilder, ArrayRef, StructArray};
+use arrow::datatypes::DataType as ArrowType;
+use std::any::Any;
+use std::sync::Arc;
+
+/// Returns an [`ArrayReader`] that yields [`StructArray`] with no columns
+/// but with row counts that correspond to the amount of data in the file
+///
+/// This is useful for when projection eliminates all columns within a collection
+pub fn make_empty_array_reader(row_count: usize) -> Box<dyn ArrayReader> {
+    Box::new(EmptyArrayReader::new(row_count))
+}
+
+struct EmptyArrayReader {
+    data_type: ArrowType,

Review Comment:
   It's needed for `get_data_type(&self) -> &ArrowType` as it needs to return a reference with a lifetime coupled to the object itself.



-- 
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-rs] tustvold commented on a diff in pull request #1560: Support empty projection in ParquetRecordBatchReader

Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #1560:
URL: https://github.com/apache/arrow-rs/pull/1560#discussion_r849552832


##########
parquet/src/arrow/arrow_reader.rs:
##########
@@ -214,12 +205,6 @@ impl ParquetRecordBatchReader {
         batch_size: usize,
         array_reader: Box<dyn ArrayReader>,
     ) -> Result<Self> {
-        // Check that array reader is struct array reader
-        array_reader

Review Comment:
   This check does not seem necessary, all it cares about is that it yields StructArray



-- 
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-rs] codecov-commenter commented on pull request #1560: Support empty projection in ParquetRecordBatchReader

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1560:
URL: https://github.com/apache/arrow-rs/pull/1560#issuecomment-1100061431

   # [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1560?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1560](https://codecov.io/gh/apache/arrow-rs/pull/1560?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1a1dda8) into [master](https://codecov.io/gh/apache/arrow-rs/commit/0192872d86e04d17df576186673c858eed5c2e94?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0192872) will **decrease** coverage by `0.00%`.
   > The diff coverage is `73.80%`.
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #1560      +/-   ##
   ==========================================
   - Coverage   82.86%   82.85%   -0.01%     
   ==========================================
     Files         190      191       +1     
     Lines       55057    55085      +28     
   ==========================================
   + Hits        45622    45642      +20     
   - Misses       9435     9443       +8     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/1560?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [parquet/src/arrow/async\_reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/1560/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvYXJyb3cvYXN5bmNfcmVhZGVyLnJz) | `0.00% <0.00%> (ø)` | |
   | [parquet/src/arrow/array\_reader/empty\_array.rs](https://codecov.io/gh/apache/arrow-rs/pull/1560/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvYXJyb3cvYXJyYXlfcmVhZGVyL2VtcHR5X2FycmF5LnJz) | `55.55% <55.55%> (ø)` | |
   | [parquet/src/arrow/array\_reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/1560/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvYXJyb3cvYXJyYXlfcmVhZGVyLnJz) | `86.98% <100.00%> (-0.10%)` | :arrow_down: |
   | [parquet/src/arrow/array\_reader/builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/1560/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvYXJyb3cvYXJyYXlfcmVhZGVyL2J1aWxkZXIucnM=) | `68.01% <100.00%> (+0.09%)` | :arrow_up: |
   | [parquet/src/arrow/arrow\_reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/1560/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvYXJyb3cvYXJyb3dfcmVhZGVyLnJz) | `94.96% <100.00%> (+0.48%)` | :arrow_up: |
   | [arrow/src/array/array\_struct.rs](https://codecov.io/gh/apache/arrow-rs/pull/1560/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2FycmF5X3N0cnVjdC5ycw==) | `88.44% <0.00%> (-0.80%)` | :arrow_down: |
   | [arrow/src/array/transform/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/1560/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9tb2QucnM=) | `86.46% <0.00%> (ø)` | |
   | [parquet/src/arrow/schema.rs](https://codecov.io/gh/apache/arrow-rs/pull/1560/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvYXJyb3cvc2NoZW1hLnJz) | `85.83% <0.00%> (+0.11%)` | :arrow_up: |
   | [parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow-rs/pull/1560/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvZW5jb2RpbmdzL2VuY29kaW5nLnJz) | `93.56% <0.00%> (+0.18%)` | :arrow_up: |
   | ... and [1 more](https://codecov.io/gh/apache/arrow-rs/pull/1560/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1560?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1560?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [0192872...1a1dda8](https://codecov.io/gh/apache/arrow-rs/pull/1560?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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