nuno-faria commented on code in PR #25638:
URL: https://github.com/apache/datafusion/pull/25638#discussion_r4238807909
##########
datafusion-cli/src/functions.rs:
##########
@@ -363,6 +363,15 @@ impl TableFunctionImpl for ParquetMetadataFunc {
Field::new("data_page_offset", DataType::Int64, true),
Field::new("total_compressed_size", DataType::Int64, true),
Field::new("total_uncompressed_size", DataType::Int64, true),
+ Field::new("bloom_filter_offset", DataType::Int64, true),
+ Field::new("bloom_filter_length", DataType::Int64, true),
+ Field::new("column_index_offset", DataType::Int64, true),
+ Field::new("column_index_length", DataType::Int64, true),
+ Field::new("offset_index_offset", DataType::Int64, true),
+ Field::new("offset_index_length", DataType::Int64, true),
+ Field::new("sorting_columns", DataType::Utf8, true),
+ Field::new("row_group_ordinal", DataType::Int64, true),
+ Field::new("row_group_file_offset", DataType::Int64, true),
Review Comment:
Since these metrics are based on the row group, it might be better to move
them near to the other row group metrics (i.e., after `row_group_bytes`).
##########
datafusion-cli/src/functions.rs:
##########
@@ -430,6 +449,19 @@ impl TableFunctionImpl for ParquetMetadataFunc {
data_page_offset_arr.push(column.data_page_offset());
total_compressed_size_arr.push(column.compressed_size());
total_uncompressed_size_arr.push(column.uncompressed_size());
+ bloom_filter_offset_arr.push(column.bloom_filter_offset());
+ bloom_filter_length_arr
+ .push(column.bloom_filter_length().map(|v| v as i64));
+ column_index_offset_arr.push(column.column_index_offset());
+ column_index_length_arr
+ .push(column.column_index_length().map(|v| v as i64));
+ offset_index_offset_arr.push(column.offset_index_offset());
+ offset_index_length_arr
+ .push(column.offset_index_length().map(|v| v as i64));
+ sorting_columns_arr
+ .push(row_group.sorting_columns().map(|cols|
format!("{cols:?}")));
+ row_group_ordinal_arr.push(row_group.ordinal().map(|v| v as
i64));
Review Comment:
Is there a difference between the ordinal and the existing row_group_id?
##########
datafusion-cli/src/main.rs:
##########
@@ -609,11 +609,51 @@ mod tests {
let rbs = df.collect().await?;
assert_snapshot!(batches_to_string(&rbs),@r#"
-
+-----------------------------------------------------------------+--------------+--------------------+-----------------------+-----------------+-----------+-------------+------------+----------------+------------+-----------+-----------+------------------+----------------------+-----------------+-----------------+--------------------+--------------------------+-------------------+------------------------+------------------+-----------------------+-------------------------+
- | filename |
row_group_id | row_group_num_rows | row_group_num_columns | row_group_bytes |
column_id | file_offset | num_values | path_in_schema | type | stats_min
| stats_max | stats_null_count | stats_distinct_count | stats_min_value |
stats_max_value | compression | encodings |
index_page_offset | dictionary_page_offset | data_page_offset |
total_compressed_size | total_uncompressed_size |
-
+-----------------------------------------------------------------+--------------+--------------------+-----------------------+-----------------+-----------+-------------+------------+----------------+------------+-----------+-----------+------------------+----------------------+-----------------+-----------------+--------------------+--------------------------+-------------------+------------------------+------------------+-----------------------+-------------------------+
- | ../parquet-testing/data/data_index_bloom_encoding_stats.parquet | 0
| 14 | 1 | 163 | 0
| 4 | 14 | "String" | BYTE_ARRAY | Hello |
today | 0 | | Hello | today
| GZIP(GzipLevel(6)) | [PLAIN, RLE, BIT_PACKED] | |
| 4 | 152 | 163
|
-
+-----------------------------------------------------------------+--------------+--------------------+-----------------------+-----------------+-----------+-------------+------------+----------------+------------+-----------+-----------+------------------+----------------------+-----------------+-----------------+--------------------+--------------------------+-------------------+------------------------+------------------+-----------------------+-------------------------+
+
+-----------------------------------------------------------------+--------------+--------------------+-----------------------+-----------------+-----------+-------------+------------+----------------+------------+-----------+-----------+------------------+----------------------+-----------------+-----------------+--------------------+--------------------------+-------------------+------------------------+------------------+-----------------------+-------------------------+---------------------+---------------------+---------------------+---------------------+---------------------+---------------------+-----------------+-------------------+-----------------------+
+ | filename |
row_group_id | row_group_num_rows | row_group_num_columns | row_group_bytes |
column_id | file_offset | num_values | path_in_schema | type | stats_min
| stats_max | stats_null_count | stats_distinct_count | stats_min_value |
stats_max_value | compression | encodings |
index_page_offset | dictionary_page_offset | data_page_offset |
total_compressed_size | total_uncompressed_size | bloom_filter_offset |
bloom_filter_length | column_index_offset | column_index_length |
offset_index_offset | offset_index_length | sorting_columns | row_group_ordinal
| row_group_file_offset |
+
+-----------------------------------------------------------------+--------------+--------------------+-----------------------+-----------------+-----------+-------------+------------+----------------+------------+-----------+-----------+------------------+----------------------+-----------------+-----------------+--------------------+--------------------------+-------------------+------------------------+------------------+-----------------------+-------------------------+---------------------+---------------------+---------------------+---------------------+---------------------+---------------------+-----------------+-------------------+-----------------------+
+ | ../parquet-testing/data/data_index_bloom_encoding_stats.parquet | 0
| 14 | 1 | 163 | 0
| 4 | 14 | "String" | BYTE_ARRAY | Hello |
today | 0 | | Hello | today
| GZIP(GzipLevel(6)) | [PLAIN, RLE, BIT_PACKED] | |
| 4 | 152 | 163
| 192 | | 156 |
25 | 181 | 11 |
| 0 | 4 |
+
+-----------------------------------------------------------------+--------------+--------------------+-----------------------+-----------------+-----------+-------------+------------+----------------+------------+-----------+-----------+------------------+----------------------+-----------------+-----------------+--------------------+--------------------------+-------------------+------------------------+------------------+-----------------------+-------------------------+---------------------+---------------------+---------------------+---------------------+---------------------+---------------------+-----------------+-------------------+-----------------------+
+ "#);
+
+ Ok(())
+ }
+
+ /// `bloom_filter_length` and `sorting_columns` are absent from the other
+ /// fixtures. These files populate them so a regression that always emits
+ /// NULL still fails.
+ #[tokio::test]
+ async fn test_parquet_metadata_bloom_length_and_sorting_columns()
+ -> Result<(), DataFusionError> {
+ let ctx = SessionContext::new();
+ ctx.register_udtf("parquet_metadata", Arc::new(ParquetMetadataFunc
{}));
+
+ let sql = "SELECT path_in_schema, bloom_filter_offset,
bloom_filter_length \
+ FROM
parquet_metadata('../parquet-testing/data/data_index_bloom_encoding_with_length.parquet')";
+ let df = ctx.sql(sql).await?;
+ let rbs = df.collect().await?;
+ assert_snapshot!(batches_to_string(&rbs), @r#"
+ +----------------+---------------------+---------------------+
+ | path_in_schema | bloom_filter_offset | bloom_filter_length |
+ +----------------+---------------------+---------------------+
+ | "String" | 253 | 2064 |
+ +----------------+---------------------+---------------------+
+ "#);
+
+ let sql = "SELECT row_group_id, column_id, path_in_schema,
sorting_columns \
+ FROM
parquet_metadata('../parquet-testing/data/sort_columns.parquet') \
+ ORDER BY row_group_id, column_id";
+ let df = ctx.sql(sql).await?;
+ let rbs = df.collect().await?;
+ assert_snapshot!(batches_to_string(&rbs), @r#"
+
+--------------+-----------+----------------+------------------------------------------------------------------------------------------------------------------------------------------------+
+ | row_group_id | column_id | path_in_schema | sorting_columns
|
+
+--------------+-----------+----------------+------------------------------------------------------------------------------------------------------------------------------------------------+
+ | 0 | 0 | "a" | [SortingColumn {
column_idx: 0, descending: true, nulls_first: true }, SortingColumn {
column_idx: 1, descending: false, nulls_first: false }] |
Review Comment:
I think the current `sorting_columns` is difficult to read. To me at least I
would prefer the column names instead. E.g.:
`[SortingColumn { column_idx: 0, descending: true, nulls_first: true },
SortingColumn { column_idx: 1, descending: false, nulls_first: false }]`
vs
`col1 DESC NULLS FIRST, col2 ASC`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
