You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/06/12 13:32:20 UTC
[GitHub] [spark] wangyum opened a new pull request, #36851: [WIP] Make `SchemaPruning` only pruning if it contains nested column
wangyum opened a new pull request, #36851:
URL: https://github.com/apache/spark/pull/36851
### What changes were proposed in this pull request?
This PR makes `SchemaPruning` only pruning when `HadoopFsRelation`'s schema contains nested column.
### Why are the changes needed?
Speed up `SchemaPruning`.
`TPCDSQueryWithStatsSuite` result:
Before this PR:
```
org.apache.spark.sql.execution.datasources.SchemaPruning 0 / 214653656 0 / 312
```
After this PR:
```
org.apache.spark.sql.execution.datasources.SchemaPruning 0 / 52967791 0 / 312
```
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
// TODO
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] github-actions[bot] closed pull request #36851: [SPARK-39535][SQL] Make `SchemaPruning` only pruning if it contains nested column
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #36851: [SPARK-39535][SQL] Make `SchemaPruning` only pruning if it contains nested column
URL: https://github.com/apache/spark/pull/36851
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a diff in pull request #36851: [SPARK-39535][SQL] Make `SchemaPruning` only pruning if it contains nested column
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #36851:
URL: https://github.com/apache/spark/pull/36851#discussion_r906374914
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/SchemaPruning.scala:
##########
@@ -103,8 +104,14 @@ object SchemaPruning extends Rule[LogicalPlan] {
}
}
+ private def containsNestedColumn(fsRelation: HadoopFsRelation): Boolean =
Review Comment:
`_metadata` is added by datasource based on if it is required in the query. It is not a physical column that we prune from Parquet by changing the required schema. I don't think it follows the same logic there.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #36851: [SPARK-39535][SQL] Make `SchemaPruning` only pruning if it contains nested column
Posted by GitBox <gi...@apache.org>.
wangyum commented on PR #36851:
URL: https://github.com/apache/spark/pull/36851#issuecomment-1165154168
cc @cloud-fan
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a diff in pull request #36851: [SPARK-39535][SQL] Make `SchemaPruning` only pruning if it contains nested column
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #36851:
URL: https://github.com/apache/spark/pull/36851#discussion_r906373236
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/SchemaPruning.scala:
##########
@@ -103,8 +104,14 @@ object SchemaPruning extends Rule[LogicalPlan] {
}
}
+ private def containsNestedColumn(fsRelation: HadoopFsRelation): Boolean =
Review Comment:
Are you sure?
I run `FileMetadataStructSuite` test with different pruning configurations:
`SQLConf.NESTED_SCHEMA_PRUNING_ENABLED` = `false`:
```
== Physical Plan ==
*(1) Project [name#1884, age#1885, info#1886.id AS id#1890L, _metadata#1891.file_name AS file_name#1892]
+- *(1) Project [name#1884, age#1885, info#1886, named_struct(file_name, file_name#1902) AS _metadata#1891]
+- *(1) ColumnarToRow
+- FileScan parquet [name#1884,age#1885,info#1886,file_name#1902] Batched: true, DataFilters: [], Format: Parquet, Location: InMemoryFileIndex(2 paths)[file:/spark/target/tmp/spark-9e425a7e-5b6c-46aa..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<name:string,age:int,info:struct<id:bigint,university:string>>
```
`SQLConf.NESTED_SCHEMA_PRUNING_ENABLED` = `true`:
```
== Physical Plan ==
*(1) Project [name#1884, age#1885, info#1886.id AS id#1890L, _metadata#1891.file_name AS file_name#1892]
+- *(1) Project [name#1884, age#1885, info#1886, named_struct(file_name, file_name#1902) AS _metadata#1891]
+- *(1) ColumnarToRow
+- FileScan parquet [name#1884,age#1885,info#1886,file_name#1902] Batched: true, DataFilters: [], Format: Parquet, Location: InMemoryFileIndex(2 paths)[file:/spark/target/tmp/spark-a0afa17b-52e7-4d1c..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<name:string,age:int,info:struct<id:bigint>>
```
You can see `info` is pruned. But for `_metadata` column, it is always pruned and no difference between pruning/non-pruning cases.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #36851: [SPARK-39535][SQL] Make `SchemaPruning` only pruning if it contains nested column
Posted by GitBox <gi...@apache.org>.
wangyum commented on code in PR #36851:
URL: https://github.com/apache/spark/pull/36851#discussion_r906791897
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/SchemaPruning.scala:
##########
@@ -103,8 +104,14 @@ object SchemaPruning extends Rule[LogicalPlan] {
}
}
+ private def containsNestedColumn(fsRelation: HadoopFsRelation): Boolean =
Review Comment:
Thank you all.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yaohua628 commented on a diff in pull request #36851: [SPARK-39535][SQL] Make `SchemaPruning` only pruning if it contains nested column
Posted by GitBox <gi...@apache.org>.
Yaohua628 commented on code in PR #36851:
URL: https://github.com/apache/spark/pull/36851#discussion_r906410700
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/SchemaPruning.scala:
##########
@@ -103,8 +104,14 @@ object SchemaPruning extends Rule[LogicalPlan] {
}
}
+ private def containsNestedColumn(fsRelation: HadoopFsRelation): Boolean =
Review Comment:
Yes, exactly!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #36851: [WIP] Make `SchemaPruning` only pruning if it contains nested column
Posted by GitBox <gi...@apache.org>.
wangyum commented on PR #36851:
URL: https://github.com/apache/spark/pull/36851#issuecomment-1153170221
cc @Yaohua628 @viirya
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yaohua628 commented on a diff in pull request #36851: [SPARK-39535][SQL] Make `SchemaPruning` only pruning if it contains nested column
Posted by GitBox <gi...@apache.org>.
Yaohua628 commented on code in PR #36851:
URL: https://github.com/apache/spark/pull/36851#discussion_r906392455
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/SchemaPruning.scala:
##########
@@ -103,8 +104,14 @@ object SchemaPruning extends Rule[LogicalPlan] {
}
}
+ private def containsNestedColumn(fsRelation: HadoopFsRelation): Boolean =
Review Comment:
We combined them in the same logic at `prunePhysicalColumns(...)`. And the flag `NESTED_SCHEMA_PRUNING_ENABLED` controls only data schema ([here](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/SchemaPruning.scala#L73)), but not `_metadata`.
Basically, we try to prune `_metadata` regardless of the flag and the file format.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yaohua628 commented on a diff in pull request #36851: [SPARK-39535][SQL] Make `SchemaPruning` only pruning if it contains nested column
Posted by GitBox <gi...@apache.org>.
Yaohua628 commented on code in PR #36851:
URL: https://github.com/apache/spark/pull/36851#discussion_r906283197
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/SchemaPruning.scala:
##########
@@ -103,8 +104,14 @@ object SchemaPruning extends Rule[LogicalPlan] {
}
}
+ private def containsNestedColumn(fsRelation: HadoopFsRelation): Boolean =
Review Comment:
Maybe you can have one more check here "if it contains `_metadata`" using `FileSourceMetadataAttribute(attr)` and add a test for it? That would be great!
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/SchemaPruning.scala:
##########
@@ -103,8 +104,14 @@ object SchemaPruning extends Rule[LogicalPlan] {
}
}
+ private def containsNestedColumn(fsRelation: HadoopFsRelation): Boolean =
Review Comment:
I think it will break the schema pruning for the hidden metadata struct (`_metadata` is not in `fsRelation.schema` but `fsRelation.output`).
We actually have tests for that ([here](https://github.com/apache/spark/blob/master/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/FileMetadataStructSuite.scala#L431)): The reason why they didn't break is the test data itself luckily has a nested column ([here](https://github.com/apache/spark/blob/master/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/FileMetadataStructSuite.scala#L37)).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a diff in pull request #36851: [SPARK-39535][SQL] Make `SchemaPruning` only pruning if it contains nested column
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #36851:
URL: https://github.com/apache/spark/pull/36851#discussion_r906408695
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/SchemaPruning.scala:
##########
@@ -103,8 +104,14 @@ object SchemaPruning extends Rule[LogicalPlan] {
}
}
+ private def containsNestedColumn(fsRelation: HadoopFsRelation): Boolean =
Review Comment:
Oh, I see. Operators will collect metadata column names. And during query execution, it will update into a row based on these column names. In `prunePhysicalColumns`, no matter the config value is, it always prune metadata columns.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a diff in pull request #36851: [SPARK-39535][SQL] Make `SchemaPruning` only pruning if it contains nested column
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #36851:
URL: https://github.com/apache/spark/pull/36851#discussion_r906323749
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/SchemaPruning.scala:
##########
@@ -103,8 +104,14 @@ object SchemaPruning extends Rule[LogicalPlan] {
}
}
+ private def containsNestedColumn(fsRelation: HadoopFsRelation): Boolean =
Review Comment:
Hmm, does nested column pruning work for metadata columns? Nested column pruning is basically to update required schema for file datasource like parquet.
I think metadata columns are directly reported from datasource to Spark. For example, `FileFormat` adds metadata columns separately during reading data. It is not covered by the nested column pruning logic.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yaohua628 commented on a diff in pull request #36851: [SPARK-39535][SQL] Make `SchemaPruning` only pruning if it contains nested column
Posted by GitBox <gi...@apache.org>.
Yaohua628 commented on code in PR #36851:
URL: https://github.com/apache/spark/pull/36851#discussion_r906361503
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/SchemaPruning.scala:
##########
@@ -103,8 +104,14 @@ object SchemaPruning extends Rule[LogicalPlan] {
}
}
+ private def containsNestedColumn(fsRelation: HadoopFsRelation): Boolean =
Review Comment:
> does nested column pruning work for metadata columns?
Yes, we combined them in this logic. And the metadata pruning should work for every file format: https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/SchemaPruning.scala#L92
(It's hidden file metadata for file-based V1 source)
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org