ARROW-10783: [Rust] [DataFusion] Implement row count statistics for Parquet TableProviderParquet statistics [WIP] by XiaokunDing · Pull Request #8860 · apache/arrow

XiaokunDing · 2020-12-07T12:54:51Z

No description provided.

…a source

… Grove

… Seddon

github-actions · 2020-12-07T13:09:25Z

https://issues.apache.org/jira/browse/ARROW-10783

…arquet TableProvider

seddonm1

FYI I have independently prepared a PR but was waiting for #8831

seddonm1 · 2020-12-07T19:59:49Z

    }
+
+    /// Get the statistics from parquet file format
+    pub fn statistics(&self) -> Option<Statistics> {


By doing this you are parsing the parquet file headers a second time when they have already been parsed.

Sorry, for clarity, the ParquetReader already performs this code. You could expose the metadata produced in the first pass and then you don't need to reimplement this logic (which has to read all the files again).

seddonm1 · 2020-12-07T20:01:47Z


+/// The table statistics
+#[derive(Clone, Debug)]
+pub struct Statistics {


This is repeating what you have done in #8831

Thanks Mike, this is just a draft code for WIP, when #8831 have been merged I'll recommit it.

seddonm1 · 2020-12-08T08:00:17Z

Here is a link to the WIP i was doing: https://github.com/seddonm1/arrow/compare/master...seddonm1:parquet-statistics?expand=1

andygrove · 2020-12-09T14:55:44Z

+            }
+        }
+
+        if num_rows >= i64::MAX {


nit: you could use std::cmp::min and std::cmp::max instead of these if statements to find min/max values

Dandandan · 2020-12-10T16:57:27Z

@XiaokunDing what about changing the statistics to be optional per statistic, but not the statistics itself?
I was looking at adding it e.g. for a MemTable where it is easy to add a num_rows statistic, but (currently) not so trivial to add a total_byte_size implementation. Having the num_rows available could e.g. allow more efficient joins.
I guess this would be the same for other statistics that could be added later on.

Dandandan · 2020-12-10T17:01:01Z

So the proposal would be to change the statistics struct to

pub struct Statistics {
    /// The number of table rows
    pub num_rows: Option<usize>,
    /// total byte of the table rows
    pub total_byte_size: Option<usize>,
}

and the Statistics doesn't need to be wrapped as Option<Statistics> anymore.

Dandandan · 2020-12-10T17:20:02Z

Added a PR for that here: #8889

Dandandan · 2020-12-13T11:19:42Z

@XiaokunDing do you plan updating this PR?

…eProvider This is an implementation of Statistics for the Parquet datasource as this PR seems to have gone quiet: #8860 This PR exposes the ParquetMetaData produced by the file reader as even though initially we only consume `row_count` and `total_byte_size` there is column level metadata that may be very useful in future for potentially large performance gains. Closes #8944 from seddonm1/parquet-statistics Authored-by: Mike Seddon <seddonm1@gmail.com> Signed-off-by: Andrew Lamb <andrew@nerdnetworks.org>

alamb · 2020-12-17T23:41:19Z

FYI this PR was partially superceded by #8944 -- unless @XiaokunDing objects, I think we should close this PR.

jorgecarleitao · 2020-12-21T06:48:37Z

Closing as duplicate. Thanks @XiaokunDing for this initial implementation and for your interest in the Arrow project!

Let us know if there was anything on our side that led you to not continue this work, so that we can also improve our process. If not, I will assume that you did not have the time, which is perfectly understandable :)

…eProvider This is an implementation of Statistics for the Parquet datasource as this PR seems to have gone quiet: apache/arrow#8860 This PR exposes the ParquetMetaData produced by the file reader as even though initially we only consume `row_count` and `total_byte_size` there is column level metadata that may be very useful in future for potentially large performance gains. Closes #8944 from seddonm1/parquet-statistics Authored-by: Mike Seddon <seddonm1@gmail.com> Signed-off-by: Andrew Lamb <andrew@nerdnetworks.org>

shawnding(丁晓坤) added 3 commits December 4, 2020 18:05

ARROW-10781 [Rust] [DataFusion] add the 'Statistics' interface in dat…

114f83c

…a source

ARROW-10781 [Rust] [DataFusion] Fix the code address comments by Andy…

d2e892b

… Grove

ARROW-10781 [Rust] [DataFusion] Fix the code address comments by Mike…

ad27fd6

… Seddon

github-actions Bot added Component: Rust - DataFusion Component: Rust labels Dec 7, 2020

XiaokunDing changed the title ~~Implement row count statistics for Parquet TableProviderParquet statistics~~ ARROW-10783 Implement row count statistics for Parquet TableProviderParquet statistics Dec 7, 2020

XiaokunDing changed the title ~~ARROW-10783 Implement row count statistics for Parquet TableProviderParquet statistics~~ #ARROW-10783 Implement row count statistics for Parquet TableProviderParquet statistics Dec 7, 2020

XiaokunDing changed the title ~~#ARROW-10783 Implement row count statistics for Parquet TableProviderParquet statistics~~ ARROW-10783 [Rust] [DataFusion] Implement row count statistics for Parquet TableProviderParquet statistics Dec 7, 2020

nevi-me changed the title ~~ARROW-10783 [Rust] [DataFusion] Implement row count statistics for Parquet TableProviderParquet statistics~~ ARROW-10783: [Rust] [DataFusion] Implement row count statistics for Parquet TableProviderParquet statistics Dec 7, 2020

XiaokunDing force-pushed the parquet_statistics branch from e751344 to a025489 Compare December 7, 2020 13:08

XiaokunDing changed the title ~~ARROW-10783: [Rust] [DataFusion] Implement row count statistics for Parquet TableProviderParquet statistics~~ ARROW-10783: [Rust] [DataFusion] Implement row count statistics for Parquet TableProviderParquet statistics [ WIP] Dec 7, 2020

XiaokunDing changed the title ~~ARROW-10783: [Rust] [DataFusion] Implement row count statistics for Parquet TableProviderParquet statistics [ WIP]~~ ARROW-10783: [Rust] [DataFusion] Implement row count statistics for Parquet TableProviderParquet statistics Dec 7, 2020

XiaokunDing changed the title ~~ARROW-10783: [Rust] [DataFusion] Implement row count statistics for Parquet TableProviderParquet statistics~~ ARROW-10783: [Rust] [DataFusion] Implement row count statistics for Parquet TableProviderParquet statistics [WIP] Dec 7, 2020

ARROW-10783: [Rust] [DataFusion] Implement row count statistics for P…

239ebca

…arquet TableProvider

XiaokunDing force-pushed the parquet_statistics branch from a025489 to 239ebca Compare December 7, 2020 13:14

apache deleted a comment from github-actions Bot Dec 7, 2020

seddonm1 suggested changes Dec 8, 2020

View reviewed changes

XiaokunDing closed this Dec 8, 2020

XiaokunDing reopened this Dec 8, 2020

github-actions Bot added the needs-rebase A PR that needs to be rebased by the author label Dec 9, 2020

andygrove reviewed Dec 9, 2020

View reviewed changes

seddonm1 mentioned this pull request Dec 16, 2020

ARROW-10783: [Rust][DataFusion] Implement Statistics for Parquet TableProvider #8944

Closed

jorgecarleitao closed this Dec 21, 2020

asfimport mentioned this pull request Dec 21, 2020

[Rust] [DataFusion] Implement row count statistics for Parquet TableProvider #26724

Closed

Uh oh!

Conversation

XiaokunDing commented Dec 7, 2020

Uh oh!

github-actions Bot commented Dec 7, 2020

Uh oh!

seddonm1 left a comment

Choose a reason for hiding this comment

Uh oh!

seddonm1 Dec 7, 2020

Choose a reason for hiding this comment

Uh oh!

seddonm1 Dec 9, 2020

Choose a reason for hiding this comment

Uh oh!

seddonm1 Dec 7, 2020

Choose a reason for hiding this comment

Uh oh!

XiaokunDing Dec 8, 2020

Choose a reason for hiding this comment

Uh oh!

seddonm1 commented Dec 8, 2020

Uh oh!

andygrove Dec 9, 2020

Choose a reason for hiding this comment

Uh oh!

Dandandan commented Dec 10, 2020

Uh oh!

Dandandan commented Dec 10, 2020

Uh oh!

Dandandan commented Dec 10, 2020

Uh oh!

Dandandan commented Dec 13, 2020

Uh oh!

alamb commented Dec 17, 2020

Uh oh!

jorgecarleitao commented Dec 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants