-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-32646][SQL] ORC predicate pushdown should work with case-insensitive analysis #29457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
65b1e7f
e3949f1
a3d4bc8
7104666
2f010de
090747d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -39,6 +39,8 @@ trait OrcFiltersBase { | |||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| case class OrcPrimitiveField(fieldName: String, fieldType: DataType) | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||
| * This method returns a map which contains ORC field name and data type. Each key | ||||||||||||||||||||||||||||||||||||||||||||
| * represents a column; `dots` are used as separators for nested columns. If any part | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -49,19 +51,21 @@ trait OrcFiltersBase { | |||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||
| protected[sql] def getSearchableTypeMap( | ||||||||||||||||||||||||||||||||||||||||||||
| schema: StructType, | ||||||||||||||||||||||||||||||||||||||||||||
| caseSensitive: Boolean): Map[String, DataType] = { | ||||||||||||||||||||||||||||||||||||||||||||
| caseSensitive: Boolean): Map[String, OrcPrimitiveField] = { | ||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.spark.sql.connector.catalog.CatalogV2Implicits.MultipartIdentifierHelper | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| def getPrimitiveFields( | ||||||||||||||||||||||||||||||||||||||||||||
| fields: Seq[StructField], | ||||||||||||||||||||||||||||||||||||||||||||
| parentFieldNames: Seq[String] = Seq.empty): Seq[(String, DataType)] = { | ||||||||||||||||||||||||||||||||||||||||||||
| parentFieldNames: Seq[String] = Seq.empty): Seq[(String, OrcPrimitiveField)] = { | ||||||||||||||||||||||||||||||||||||||||||||
| fields.flatMap { f => | ||||||||||||||||||||||||||||||||||||||||||||
| f.dataType match { | ||||||||||||||||||||||||||||||||||||||||||||
| case st: StructType => | ||||||||||||||||||||||||||||||||||||||||||||
| getPrimitiveFields(st.fields, parentFieldNames :+ f.name) | ||||||||||||||||||||||||||||||||||||||||||||
| case BinaryType => None | ||||||||||||||||||||||||||||||||||||||||||||
| case _: AtomicType => | ||||||||||||||||||||||||||||||||||||||||||||
| Some(((parentFieldNames :+ f.name).quoted, f.dataType)) | ||||||||||||||||||||||||||||||||||||||||||||
| val fieldName = (parentFieldNames :+ f.name).quoted | ||||||||||||||||||||||||||||||||||||||||||||
| val orcField = OrcPrimitiveField(fieldName, f.dataType) | ||||||||||||||||||||||||||||||||||||||||||||
| Some((fieldName, orcField)) | ||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because we look up attribute name of pushed down predicates in the map E,g, If ORC file has 'ABC It is similar to Parquet: Lines 56 to 76 in d6a68e0
|
||||||||||||||||||||||||||||||||||||||||||||
| case _ => None | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK the input
schemais only used to build thedataTypeMap, can we directly pass the physical ORC schema here? Then we don't need to convert the ORC schema to catalyst schema which is consistent with the parquet side.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our ORC pushdown code, e.g.,
buildLeafSearchArgument, uses catalyst schema in many places. I think it is still possible to use ORC's schema there and remove catalyst schema, but it looks like a big change. If we want to do it, I'd suggest to do it in another PR. This diff is already not small now.