Skip to content

Commit 77eaff3

Browse files
gengliangwanggengliangwang
committed
address comments
1 parent ce08f04 commit 77eaff3

3 files changed

Lines changed: 80 additions & 24 deletions

File tree

  • sql

‎sql/core/v1.2.1/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcFilters.scala‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ private[sql] object OrcFilters extends OrcFiltersBase {
124124
val childResultOptional = convertibleFiltersHelper(pred, canPartialPushDown = false)
125125
childResultOptional.map(Not)
126126
case other =>
127-
for (_ <- buildSearchArgument(dataTypeMap, other, newBuilder())) yield other
127+
for (_ <- buildLeafSearchArgument(dataTypeMap, other, newBuilder())) yield other
128128
}
129129
filters.flatMap { filter =>
130130
convertibleFiltersHelper(filter, true)
@@ -173,9 +173,6 @@ private[sql] object OrcFilters extends OrcFiltersBase {
173173
dataTypeMap: Map[String, DataType],
174174
expression: Filter,
175175
builder: Builder): Option[Builder] = {
176-
def getType(attribute: String): PredicateLeaf.Type =
177-
getPredicateLeafType(dataTypeMap(attribute))
178-
179176
import org.apache.spark.sql.sources._
180177

181178
expression match {
@@ -196,10 +193,30 @@ private[sql] object OrcFilters extends OrcFiltersBase {
196193
negate <- buildSearchArgument(dataTypeMap, child, builder.startNot())
197194
} yield negate.end()
198195

199-
// NOTE: For all case branches dealing with leaf predicates below, the additional `startAnd()`
200-
// call is mandatory. ORC `SearchArgument` builder requires that all leaf predicates must be
201-
// wrapped by a "parent" predicate (`And`, `Or`, or `Not`).
196+
case other => buildLeafSearchArgument(dataTypeMap, other, builder)
197+
}
198+
}
199+
200+
/**
201+
* Build a SearchArgument for a leaf predicate and return the builder so far.
202+
*
203+
* @param dataTypeMap a map from the attribute name to its data type.
204+
* @param expression the input filter predicates.
205+
* @param builder the input SearchArgument.Builder.
206+
* @return the builder so far.
207+
*/
208+
private def buildLeafSearchArgument(
209+
dataTypeMap: Map[String, DataType],
210+
expression: Filter,
211+
builder: Builder): Option[Builder] = {
212+
def getType(attribute: String): PredicateLeaf.Type =
213+
getPredicateLeafType(dataTypeMap(attribute))
202214

215+
import org.apache.spark.sql.sources._
216+
// NOTE: For all case branches dealing with leaf predicates below, the additional `startAnd()`
217+
// call is mandatory. ORC `SearchArgument` builder requires that all leaf predicates must be
218+
// wrapped by a "parent" predicate (`And`, `Or`, or `Not`).
219+
expression match {
203220
case EqualTo(attribute, value) if isSearchableType(dataTypeMap(attribute)) =>
204221
val quotedName = quoteAttributeNameIfNeeded(attribute)
205222
val castedValue = castLiteralValue(value, dataTypeMap(attribute))

‎sql/core/v2.3.5/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcFilters.scala‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ private[sql] object OrcFilters extends OrcFiltersBase {
124124
val childResultOptional = convertibleFiltersHelper(pred, canPartialPushDown = false)
125125
childResultOptional.map(Not)
126126
case other =>
127-
for (_ <- buildSearchArgument(dataTypeMap, other, newBuilder())) yield other
127+
for (_ <- buildLeafSearchArgument(dataTypeMap, other, newBuilder())) yield other
128128
}
129129
filters.flatMap { filter =>
130130
convertibleFiltersHelper(filter, true)
@@ -173,9 +173,6 @@ private[sql] object OrcFilters extends OrcFiltersBase {
173173
dataTypeMap: Map[String, DataType],
174174
expression: Filter,
175175
builder: Builder): Option[Builder] = {
176-
def getType(attribute: String): PredicateLeaf.Type =
177-
getPredicateLeafType(dataTypeMap(attribute))
178-
179176
import org.apache.spark.sql.sources._
180177

181178
expression match {
@@ -196,10 +193,30 @@ private[sql] object OrcFilters extends OrcFiltersBase {
196193
negate <- buildSearchArgument(dataTypeMap, child, builder.startNot())
197194
} yield negate.end()
198195

199-
// NOTE: For all case branches dealing with leaf predicates below, the additional `startAnd()`
200-
// call is mandatory. ORC `SearchArgument` builder requires that all leaf predicates must be
201-
// wrapped by a "parent" predicate (`And`, `Or`, or `Not`).
196+
case other => buildLeafSearchArgument(dataTypeMap, other, builder)
197+
}
198+
}
199+
200+
/**
201+
* Build a SearchArgument for a leaf predicate and return the builder so far.
202+
*
203+
* @param dataTypeMap a map from the attribute name to its data type.
204+
* @param expression the input filter predicates.
205+
* @param builder the input SearchArgument.Builder.
206+
* @return the builder so far.
207+
*/
208+
private def buildLeafSearchArgument(
209+
dataTypeMap: Map[String, DataType],
210+
expression: Filter,
211+
builder: Builder): Option[Builder] = {
212+
def getType(attribute: String): PredicateLeaf.Type =
213+
getPredicateLeafType(dataTypeMap(attribute))
202214

215+
import org.apache.spark.sql.sources._
216+
// NOTE: For all case branches dealing with leaf predicates below, the additional `startAnd()`
217+
// call is mandatory. ORC `SearchArgument` builder requires that all leaf predicates must be
218+
// wrapped by a "parent" predicate (`And`, `Or`, or `Not`).
219+
expression match {
203220
case EqualTo(attribute, value) if isSearchableType(dataTypeMap(attribute)) =>
204221
val quotedName = quoteAttributeNameIfNeeded(attribute)
205222
val castedValue = castLiteralValue(value, dataTypeMap(attribute))

‎sql/hive/src/main/scala/org/apache/spark/sql/hive/orc/OrcFilters.scala‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,16 @@ private[orc] object OrcFilters extends Logging {
134134
val childResultOptional = convertibleFiltersHelper(pred, canPartialPushDown = false)
135135
childResultOptional.map(Not)
136136
case other =>
137-
for (_ <- buildSearchArgument(dataTypeMap, other, newBuilder())) yield other
137+
for (_ <- buildLeafSearchArgument(dataTypeMap, other, newBuilder())) yield other
138138
}
139139
filters.flatMap { filter =>
140140
convertibleFiltersHelper(filter, true)
141141
}
142142
}
143143

144144
/**
145+
* Build a SearchArgument and return the builder so far.
146+
*
145147
* @param dataTypeMap a map from the attribute name to its data type.
146148
* @param expression the input filter predicates.
147149
* @param builder the input SearchArgument.Builder.
@@ -151,15 +153,6 @@ private[orc] object OrcFilters extends Logging {
151153
dataTypeMap: Map[String, DataType],
152154
expression: Filter,
153155
builder: Builder): Option[Builder] = {
154-
def isSearchableType(dataType: DataType): Boolean = dataType match {
155-
// Only the values in the Spark types below can be recognized by
156-
// the `SearchArgumentImpl.BuilderImpl.boxLiteral()` method.
157-
case ByteType | ShortType | FloatType | DoubleType => true
158-
case IntegerType | LongType | StringType | BooleanType => true
159-
case TimestampType | _: DecimalType => true
160-
case _ => false
161-
}
162-
163156
expression match {
164157
case And(left, right) =>
165158
for {
@@ -178,6 +171,35 @@ private[orc] object OrcFilters extends Logging {
178171
negate <- buildSearchArgument(dataTypeMap, child, builder.startNot())
179172
} yield negate.end()
180173

174+
case other => buildLeafSearchArgument(dataTypeMap, other, builder)
175+
}
176+
}
177+
178+
/**
179+
* Build a SearchArgument for a leaf predicate and return the builder so far.
180+
*
181+
* @param dataTypeMap a map from the attribute name to its data type.
182+
* @param expression the input filter predicates.
183+
* @param builder the input SearchArgument.Builder.
184+
* @return the builder so far.
185+
*/
186+
private def buildLeafSearchArgument(
187+
dataTypeMap: Map[String, DataType],
188+
expression: Filter,
189+
builder: Builder): Option[Builder] = {
190+
def isSearchableType(dataType: DataType): Boolean = dataType match {
191+
// Only the values in the Spark types below can be recognized by
192+
// the `SearchArgumentImpl.BuilderImpl.boxLiteral()` method.
193+
case ByteType | ShortType | FloatType | DoubleType => true
194+
case IntegerType | LongType | StringType | BooleanType => true
195+
case TimestampType | _: DecimalType => true
196+
case _ => false
197+
}
198+
import org.apache.spark.sql.sources._
199+
// NOTE: For all case branches dealing with leaf predicates below, the additional `startAnd()`
200+
// call is mandatory. ORC `SearchArgument` builder requires that all leaf predicates must be
201+
// wrapped by a "parent" predicate (`And`, `Or`, or `Not`).
202+
expression match {
181203
// NOTE: For all case branches dealing with leaf predicates below, the additional `startAnd()`
182204
// call is mandatory. ORC `SearchArgument` builder requires that all leaf predicates must be
183205
// wrapped by a "parent" predicate (`And`, `Or`, or `Not`).

0 commit comments

Comments
 (0)