dtenedor commented on code in PR #38663:
URL: https://github.com/apache/spark/pull/38663#discussion_r1023072531
##
sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##
@@ -769,7 +769,7 @@ inlineTable
;
functionTable
-: funcName=functionName LEFT_PAREN (expression (COMMA expression)*)?
RIGHT_PAREN tableAlias
+: funcName=functionName LEFT_PAREN (functionArgument (COMMA
functionArgument)*)? RIGHT_PAREN tableAlias
Review Comment:
here in the `.g4` file, we only change the syntax for table function calls,
but the PR title and description mention general named argument support. Can we
either update the PR description or else update the `functionCall` rule to
replace `argument+=expression` with `argument+=functionArgument`? The latter
would be nice since we can make all the parser changes in the same PR.
##
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/literals.scala:
##
@@ -324,6 +324,21 @@ object LiteralTreeBits {
val nullLiteralBits: BitSet = new ImmutableBitSet(TreePattern.maxId,
LITERAL.id, NULL_LITERAL.id)
}
+case class NamedArgumentExpression(key: String, value: Expression) extends
LeafExpression {
Review Comment:
this is in `literals.scala`, but this new expression is not really a literal
:) can we maybe put this into its own file with a class comment?
##
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/NamedArgumentFunction.scala:
##
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.plans.logical
+
+import java.util.Locale
+
+import scala.collection.mutable
+
+import org.apache.spark.sql.catalyst.expressions.{Expression,
NamedArgumentExpression}
+import
org.apache.spark.sql.errors.QueryCompilationErrors.{tableFunctionDuplicateNamedArguments,
tableFunctionUnexpectedArgument}
+import org.apache.spark.sql.types._
+
+/**
+ * A trait to define a named argument function:
+ * Usage: _FUNC_(arg0, arg1, arg2, arg5 => value5, arg8 => value8)
+ *
+ * - Arguments can be passed positionally or by name
+ * - Positional arguments cannot come after a named argument
Review Comment:
from the example, named arguments can come after positional arguments. write
that in the comment too? also maybe we can add other constraints:
* no function call may include two arguments with the same name
* case sensitivity follows the SQLConf.CASE_SENSITIVE boolean configuration
* the function signature must specify the argument names, and the provided
argument names must match the names in the function signature
##
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala:
##
@@ -852,6 +852,31 @@ class PlanParserSuite extends AnalysisTest {
stop = 43))
}
+ test("table valued function with named arguments") {
+// All named arguments
+assertEqual(
+ "select * from my_tvf(arg1 => 'value1', arg2 => true)",
+ UnresolvedTableValuedFunction("my_tvf",
+NamedArgumentExpression("arg1", Literal("value1")) ::
+ NamedArgumentExpression("arg2", Literal(true)) :: Nil,
Seq.empty).select(star()))
+
+// Unnamed and named arguments
+assertEqual(
+ "select * from my_tvf(2, arg1 => 'value1', arg2 => true)",
+ UnresolvedTableValuedFunction("my_tvf",
+Literal(2) ::
+ NamedArgumentExpression("arg1", Literal("value1")) ::
+ NamedArgumentExpression("arg2", Literal(true)) :: Nil,
Seq.empty).select(star()))
+
+// Mixed arguments
+assertEqual(
+ "select * from my_tvf(arg1 => 'value1', 2, arg2 => true)",
Review Comment:
thanks for adding these parser tests! can we also add some query tests in
e.g. `SQLQuerySuite` that show what happens if we try to analyze such function
calls with named arguments? do we get an error message, or does the whole
feature work end-to-end?
##
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/NamedArgumentFunction.scala:
##
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor licens
