SEPURI-SAI-KRISHNA commented on code in PR #19646:
URL: https://github.com/apache/hudi/pull/19646#discussion_r3820558141


##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/DataSkippingUtils.scala:
##########
@@ -479,6 +482,16 @@ object ColumnStatsExpressionUtils {
   @inline def genColNumNullsExpr(colName: String): Expression = 
sparkAdapter.getExpressionFromColumn(col(getNullCountColumnNameFor(colName)))
   @inline def genColValueCountExpr: Expression = 
sparkAdapter.getExpressionFromColumn(col(getValueCountColumnNameFor))
 
+  @inline def genColumnIsNullExpression(colName: String): Expression = {

Review Comment:
   The convention in this object is split by what the helper does, rather than 
uniform, so I'd like to keep the current name:
   
   * `genCol…Expr` — `genColMinValueExpr`, `genColMaxValueExpr`, 
`genColNumNullsExpr`, `genColValueCountExpr`. These fetch a single column-stats 
column.
   * `genColumn…Expression` — `genColumnValuesEqualToExpression`, 
`genColumnOnlyValuesEqualToExpression`. These compose a whole predicate out of 
the accessors above.
   
   `genColumnIsNullExpression` is in the second group: it builds 
`Or(IsNull(numNullsExpr), GreaterThan(numNullsExpr, Literal(0)))` on top of 
`genColNumNullsExpr`, exactly as `genColumnValuesEqualToExpression` does. 
Renaming it to `genColumnIsNullExpr` would pair the `genColumn` prefix with the 
`Expr` suffix and match neither group.
   
   Happy to rename if a committer prefers otherwise.
   



-- 
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]

Reply via email to