gengliangwang commented on code in PR #36398:
URL: https://github.com/apache/spark/pull/36398#discussion_r861553530
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveDefaultColumns.scala:
##########
@@ -183,25 +192,24 @@ case class ResolveDefaultColumns(
// If necessary, return a more descriptive error message if the user tries
to nest the DEFAULT
// column reference inside some other expression, such as DEFAULT + 1
(this is not allowed).
//
- // Note that we don't need to check if
"SQLConf.get.useNullsForMissingDefaultColumnValues"
- // after this point because this method only takes responsibility to
replace *existing*
- // DEFAULT references. In contrast, the "getDefaultExpressions" method
will check that config
+ // Note that we don't need to check if
"SQLConf.get.useNullsForMissingDefaultColumnValues" after
+ // this point because this method only takes responsibility to replace
*existing* DEFAULT
+ // references. In contrast, the "getDefaultExpressionsForInsert" method
will check that config
// and add new NULLs if needed.
input match {
case table: UnresolvedInlineTable =>
- replaceExplicitDefaultColumnValues(defaultExpressions, table)
+ replaceExplicitDefaultValuesForInlineTable(defaultExpressions, table)
case project: Project =>
- replaceExplicitDefaultColumnValues(defaultExpressions, columnNames,
project)
+ replaceExplicitDefaultValuesForProject(defaultExpressions,
columnNames, project)
}
}
/**
* Replaces unresolved DEFAULT column references with corresponding values
in an inline table.
*/
- private def replaceExplicitDefaultColumnValues(
- defaultExpressions: Seq[Expression],
- table: UnresolvedInlineTable): Option[LogicalPlan] = {
- var replaced = false
Review Comment:
So basically you are using a global variable `updated` to replace the local
variable `replaced` in methods. I think using a local variable is more robust.
Sharing the status among operators might result in wrong results in future
development.
Also, returning optional results is super common in the Spark codebase.
--
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]
