Update ListingTable to use StatisticsConverter by xinlifoobar · Pull Request #11068 · apache/datafusion

xinlifoobar · 2024-06-22T13:08:10Z

Which issue does this PR close?

Closes #10923
Closes #10924

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

xinlifoobar · 2024-06-23T07:20:48Z

datafusion/core/src/datasource/file_format/parquet.rs

        let c1_stats = &stats.column_statistics[0];
        assert_eq!(c1_stats.null_count, Precision::Exact(1));
-        assert_eq!(c1_stats.max_value, Precision::Absent);
-        assert_eq!(c1_stats.min_value, Precision::Absent);


The test behaviour changes due to Utf8 is not supported in

datafusion/datafusion/core/src/datasource/file_format/parquet.rs

Lines 298 to 376 in ea21b08

fn summarize_min_max(

max_values: &mut [Option<MaxAccumulator>],

min_values: &mut [Option<MinAccumulator>],

fields: &Fields,

i: usize,

stat: &ParquetStatistics,

) {

if !stat.has_min_max_set() {

max_values[i] = None;

min_values[i] = None;

return;

}

match stat {

ParquetStatistics::Boolean(s) if DataType::Boolean == *fields[i].data_type() => {

if let Some(max_value) = &mut max_values[i] {

max_value

.update_batch(&[Arc::new(BooleanArray::from(vec![*s.max()]))])

.unwrap_or_else(|_| max_values[i] = None);

}

if let Some(min_value) = &mut min_values[i] {

min_value

.update_batch(&[Arc::new(BooleanArray::from(vec![*s.min()]))])

.unwrap_or_else(|_| min_values[i] = None);

}

}

ParquetStatistics::Int32(s) if DataType::Int32 == *fields[i].data_type() => {

if let Some(max_value) = &mut max_values[i] {

max_value

.update_batch(&[Arc::new(Int32Array::from_value(*s.max(), 1))])

.unwrap_or_else(|_| max_values[i] = None);

}

if let Some(min_value) = &mut min_values[i] {

min_value

.update_batch(&[Arc::new(Int32Array::from_value(*s.min(), 1))])

.unwrap_or_else(|_| min_values[i] = None);

}

}

ParquetStatistics::Int64(s) if DataType::Int64 == *fields[i].data_type() => {

if let Some(max_value) = &mut max_values[i] {

max_value

.update_batch(&[Arc::new(Int64Array::from_value(*s.max(), 1))])

.unwrap_or_else(|_| max_values[i] = None);

}

if let Some(min_value) = &mut min_values[i] {

min_value

.update_batch(&[Arc::new(Int64Array::from_value(*s.min(), 1))])

.unwrap_or_else(|_| min_values[i] = None);

}

}

ParquetStatistics::Float(s) if DataType::Float32 == *fields[i].data_type() => {

if let Some(max_value) = &mut max_values[i] {

max_value

.update_batch(&[Arc::new(Float32Array::from(vec![*s.max()]))])

.unwrap_or_else(|_| max_values[i] = None);

}

if let Some(min_value) = &mut min_values[i] {

min_value

.update_batch(&[Arc::new(Float32Array::from(vec![*s.min()]))])

.unwrap_or_else(|_| min_values[i] = None);

}

}

ParquetStatistics::Double(s) if DataType::Float64 == *fields[i].data_type() => {

if let Some(max_value) = &mut max_values[i] {

max_value

.update_batch(&[Arc::new(Float64Array::from(vec![*s.max()]))])

.unwrap_or_else(|_| max_values[i] = None);

}

if let Some(min_value) = &mut min_values[i] {

min_value

.update_batch(&[Arc::new(Float64Array::from(vec![*s.min()]))])

.unwrap_or_else(|_| min_values[i] = None);

}

}

_ => {

max_values[i] = None;

min_values[i] = None;

}

}

}

It is an improvement!

datafusion/core/src/datasource/physical_plan/parquet/statistics.rs

alamb

Thank you @xinlifoobar -- this looks like a great improvement. I really like how you structured the code

alamb · 2024-06-24T21:44:32Z

datafusion/core/src/datasource/file_format/parquet.rs

+                    &file_schema,
+                    file_metadata.schema_descr(),
+                ) {
+                    Ok(stats_converter) => {


this is a very nice formulation

* Update ListingTable to use StatisticsConverter * complete support for all types parquet * Fix misc * Fix misc * fix test * fix misc * fix none

xinlifoobar added 4 commits June 22, 2024 00:23

Update ListingTable to use StatisticsConverter

6671986

complete support for all types parquet

5dbfbd6

Fix misc

28eb6c1

Fix misc

e77f1bd

github-actions bot added the core Core DataFusion crate label Jun 22, 2024

xinlifoobar marked this pull request as draft June 22, 2024 13:35

fix test

ab96a17

xinlifoobar marked this pull request as ready for review June 23, 2024 07:17

xinlifoobar commented Jun 23, 2024

View reviewed changes

xinlifoobar marked this pull request as draft June 23, 2024 08:18

xinlifoobar added 2 commits June 23, 2024 16:18

fix misc

8e69e62

fix none

42fcbbd

xinlifoobar marked this pull request as ready for review June 23, 2024 11:01

alamb mentioned this pull request Jun 24, 2024

Update ListingTable to use StatisticsConverter, remove redundant statistics extraction code #10924

Closed

alamb approved these changes Jun 24, 2024

View reviewed changes

alamb merged commit 459afbb into apache:main Jun 24, 2024

appletreeisyellow mentioned this pull request Jun 26, 2024

WIP(iox-10893): patched df upgrade 2024-06-26 influxdata/arrow-datafusion#30

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Update ListingTable to use StatisticsConverter#11068

Update ListingTable to use StatisticsConverter#11068
alamb merged 7 commits intoapache:mainfrom
xinlifoobar:dev/xinli1/list-table

xinlifoobar commented Jun 22, 2024 •

edited by alamb

Loading

Uh oh!

xinlifoobar Jun 23, 2024

Uh oh!

alamb Jun 24, 2024 •

edited

Loading

Uh oh!

Uh oh!

alamb left a comment

Uh oh!

alamb Jun 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	fn summarize_min_max(
	max_values: &mut [Option<MaxAccumulator>],
	min_values: &mut [Option<MinAccumulator>],
	fields: &Fields,
	i: usize,
	stat: &ParquetStatistics,
	) {
	if !stat.has_min_max_set() {
	max_values[i] = None;
	min_values[i] = None;
	return;
	}
	match stat {
	ParquetStatistics::Boolean(s) if DataType::Boolean == *fields[i].data_type() => {
	if let Some(max_value) = &mut max_values[i] {
	max_value
	.update_batch(&[Arc::new(BooleanArray::from(vec![*s.max()]))])
	.unwrap_or_else(\|_\| max_values[i] = None);
	}
	if let Some(min_value) = &mut min_values[i] {
	min_value
	.update_batch(&[Arc::new(BooleanArray::from(vec![*s.min()]))])
	.unwrap_or_else(\|_\| min_values[i] = None);
	}
	}
	ParquetStatistics::Int32(s) if DataType::Int32 == *fields[i].data_type() => {
	if let Some(max_value) = &mut max_values[i] {
	max_value
	.update_batch(&[Arc::new(Int32Array::from_value(*s.max(), 1))])
	.unwrap_or_else(\|_\| max_values[i] = None);
	}
	if let Some(min_value) = &mut min_values[i] {
	min_value
	.update_batch(&[Arc::new(Int32Array::from_value(*s.min(), 1))])
	.unwrap_or_else(\|_\| min_values[i] = None);
	}
	}
	ParquetStatistics::Int64(s) if DataType::Int64 == *fields[i].data_type() => {
	if let Some(max_value) = &mut max_values[i] {
	max_value
	.update_batch(&[Arc::new(Int64Array::from_value(*s.max(), 1))])
	.unwrap_or_else(\|_\| max_values[i] = None);
	}
	if let Some(min_value) = &mut min_values[i] {
	min_value
	.update_batch(&[Arc::new(Int64Array::from_value(*s.min(), 1))])
	.unwrap_or_else(\|_\| min_values[i] = None);
	}
	}
	ParquetStatistics::Float(s) if DataType::Float32 == *fields[i].data_type() => {
	if let Some(max_value) = &mut max_values[i] {
	max_value
	.update_batch(&[Arc::new(Float32Array::from(vec![*s.max()]))])
	.unwrap_or_else(\|_\| max_values[i] = None);
	}
	if let Some(min_value) = &mut min_values[i] {
	min_value
	.update_batch(&[Arc::new(Float32Array::from(vec![*s.min()]))])
	.unwrap_or_else(\|_\| min_values[i] = None);
	}
	}
	ParquetStatistics::Double(s) if DataType::Float64 == *fields[i].data_type() => {
	if let Some(max_value) = &mut max_values[i] {
	max_value
	.update_batch(&[Arc::new(Float64Array::from(vec![*s.max()]))])
	.unwrap_or_else(\|_\| max_values[i] = None);
	}
	if let Some(min_value) = &mut min_values[i] {
	min_value
	.update_batch(&[Arc::new(Float64Array::from(vec![*s.min()]))])
	.unwrap_or_else(\|_\| min_values[i] = None);
	}
	}
	_ => {
	max_values[i] = None;
	min_values[i] = None;
	}
	}
	}

Comments

Conversation

xinlifoobar commented Jun 22, 2024 • edited by alamb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

xinlifoobar Jun 23, 2024

Choose a reason for hiding this comment

Uh oh!

alamb Jun 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb Jun 24, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xinlifoobar commented Jun 22, 2024 •

edited by alamb

Loading

alamb Jun 24, 2024 •

edited

Loading