learningchess2003 commented on code in PR #41864:
URL: https://github.com/apache/spark/pull/41864#discussion_r1258941506
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/maskExpressions.scala:
##########
@@ -344,4 +346,13 @@ object Mask extends SupportsNamedArguments {
strArg, upperCharArg, lowerCharArg, digitCharArg, otherCharArg))
Seq(functionSignature)
}
+
+ override def build(funcName: String, expressions: Seq[Expression]):
Expression = {
+ if (expressions.length < 1 || expressions.length > 5) {
+ throw QueryCompilationErrors.wrongNumArgsError(
+ funcName, Seq(1, 2, 3, 4, 5), expressions.length)
+ }
+ new Mask(expressions(0), expressions(1), expressions(2), expressions(3),
expressions(4))
Review Comment:
@anchovYu Thanks for the comments and the thoughts! About this: you are
right in that if we want to avoid reflection entirely, we will not be using the
```FunctionRegistryBase.build``` method since that heavily utilizes reflection
to find the constructor we want for building the function expression.
Therefore, it is true that in cases where we only provide positional arguments
that the old version of ```rearrange``` did not handle this case.
After some thoughts however, I realized that we can implement default
arguments as well in a cleaner fashion that what we had with the old way where
we would have multiple constructors defined for a single function expression.
The ```rearrange``` method would now also be responsible for filling in default
values for a function invocation where only positional arguments are provided
and if some optional parameter values are left unspecified. In another comment,
you were wondering why I checked if the index was -1 when splitting the
argument list into positional and named arguments, that check was done
precisely to allow the processing of default values for parameters in the case
where only positional arguments are provided. I thought this was a fairly
elegant way of doing it, although it can be quite confusing. I will definitely
need to clean up the code/add some comments to make this more clear.
--
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]
