[SPARK-44059] Add analyzer support of named arguments for built-in functions #41864

learningchess2003 · 2023-07-05T15:16:56Z

What changes were proposed in this pull request?

Add analyzer support for named function arguments.

Why are the changes needed?

Part of the project needed for general named function argument support.

Does this PR introduce any user-facing change?

We added support for named arguments for the CountMinSketchAgg and Mask SQL functions.

How was this patch tested?

A new suite was added for this test called NamedArgumentFunctionSuite.

learningchess2003 · 2023-07-05T15:35:02Z

@dtenedor @anchovYu Can you leave some review comments on this new PR! I rebased it and it should be in a more compact form ready for review!

anchovYu

Thanks for the effort 👍!

anchovYu · 2023-07-05T17:28:06Z

common/utils/src/main/resources/error/error-classes.json

+    "message" : [
+      "Call to function <functionName> is invalid because it includes multiple argument assignments to the same name <parameterName>."
+    ],
+    "sqlState" : "42704K"


I feel that some of my previous comments are naturally collapsed so you can't see them without expanding. I'll copy those to this new PR.

The error code should be 5 characters according to https://github.com/apache/spark/tree/master/common/utils/src/main/resources/error.

common/utils/src/main/resources/error/error-classes.json

...lyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/SupportsNamedArguments.scala

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/maskExpressions.scala

anchovYu · 2023-07-05T17:34:01Z

...lyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/SupportsNamedArguments.scala

+ * @param default  The default value of the argument. If the default is none, then that means the
+ *                 argument is required. If no argument is provided, an exception is thrown.
+ */
+case class NamedArgument(name: String,


Suggested change

case class NamedArgument(name: String,

case class NamedArgument(

name: String,

anchovYu · 2023-07-05T17:36:25Z

sql/core/src/test/resources/sql-tests/inputs/mask-functions.sql

 SELECT mask(c1, replaceArg) from values('abcd-EFGH-8765-4321', 123) as t(c1, replaceArg);
 SELECT mask('abcd-EFGH-8765-4321', 'A', 'w', '');
-SELECT mask('abcd-EFGH-8765-4321', 'A', 'abc');
+SELECT mask('abcd-EFGH-8765-4321', 'A', 'abc');


Could you revert the changes of this file?

sql/core/src/test/resources/sql-tests/inputs/named-function-arguments.sql

sql/core/src/test/scala/org/apache/spark/sql/CountMinSketchAggQuerySuite.scala

learningchess2003 · 2023-07-05T18:39:39Z

@anchovYu Comments addressed!

MaxGekk · 2023-07-05T19:37:22Z

common/utils/src/main/resources/error/error-classes.json

  },
+  "UNRECOGNIZED_PARAMETER_NAME" : {
+    "message" : [
+      "Cannot invoke function <functionName> because the function call included a named argument reference for the argument named <argumentName>, but this function does not include any signature containing an argument with this name."


Could you help users and propose function parameters sorted by levenshtein distance? See for instance:

spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/StringUtils.scala

Line 80 in d9c2d38

private[spark] def orderSuggestedIdentifiersBySimilarity(

Sounds good! Added the feature.

common/utils/src/main/resources/error/error-classes.json

learningchess2003 · 2023-07-05T21:27:18Z

@MaxGekk Comments addressed!

dtenedor

I took another review pass, thanks again for working on this!

common/utils/src/main/resources/error/error-classes.json

...t/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/CountMinSketchAgg.scala

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/maskExpressions.scala

...lyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/SupportsNamedArguments.scala

sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala

sql/core/src/test/resources/sql-tests/inputs/named-function-arguments.sql

…ompilationErrors.scala Co-authored-by: Daniel Tenedorio <[email protected]>

Co-authored-by: Daniel Tenedorio <[email protected]>

…essions/aggregate/CountMinSketchAgg.scala Co-authored-by: Daniel Tenedorio <[email protected]>

…essions/maskExpressions.scala Co-authored-by: Daniel Tenedorio <[email protected]>

learningchess2003 · 2023-07-05T22:28:14Z

@dtenedor Addressed comments, let me know what you think!

anchovYu

Could you update the PR description with the doc and the previous parser PR linked? It can provide more context for other reviewers who are interested. Thanks!

anchovYu · 2023-07-05T22:35:44Z

common/utils/src/main/resources/error/error-classes.json

  },
+  "NAMED_ARGUMENTS_NOT_SUPPORTED" : {
+    "message" : [
+      "Named arguments are not supported for function <functionName>; please retry the query with positional arguments to the function call instead or enable the feature using the SQL config \"spark.sql.allowNamedFunctionArguments\"."


I don't think this error is triggered when the config is false right? It is only because the function itself doesn't implement the named argument trait.

Actually, you are right. There is a separate exception for this.

...lyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/SupportsNamedArguments.scala

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/maskExpressions.scala

common/utils/src/main/resources/error/error-classes.json

sql/core/src/test/resources/sql-tests/inputs/named-function-arguments.sql

…s/logical/SupportsNamedArguments.scala Co-authored-by: Xinyi <[email protected]>

common/utils/src/main/resources/error/error-classes.json

…essions/maskExpressions.scala Co-authored-by: Xinyi <[email protected]>

Co-authored-by: Xinyi <[email protected]>

cloud-fan · 2023-07-13T08:46:33Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

+      expressions: Seq[Expression]) : Seq[Expression] = {
+    val rearrangedExpressions = if (!builder.functionSignatures.isEmpty) {
+      val functionSignatures = builder.functionSignatures.get
+      if (functionSignatures.length != 1) {


If we don't have a plan to support function overload soon, shall we restrict it at compile time? def functionSignatures: Option[Seq[FunctionSignature]] = None can be def functionSignatures: Option[FunctionSignature] = None

Good question, I imagine we probably would not get around to prioritizing overloads soon with this framework. But if we did, would it become necessary to update all the places where these function signatures are defined, from an Option[FunctionSignature] to an Option[Seq[FunctionSignature]]? In contrast, if we go with the latter syntax of a Seq of just one function signature, we wouldn't have to update all those sites later.

However, I imagine such an update would be a fairly mechanical change, and in the interim, we get better compiler enforcement. It seems like this suggestion is better for now to switch the API to return just an Option[FunctionSignature] for now.

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/maskExpressions.scala

cloud-fan · 2023-07-13T08:52:28Z

sql/core/src/test/scala/org/apache/spark/sql/CountMinSketchAggQuerySuite.scala

    assert(sketch == reference)
  }
+
+  test("count-min sketch with named-arguments") {


nit: we can remove this end-to-end test if named-function-arguments.sql includes count_min_sketch

cloud-fan · 2023-07-13T08:53:22Z

sql/core/src/test/resources/sql-tests/results/table-valued-functions.sql.out

 {
-  "errorClass" : "WRONG_NUM_ARGS.WITHOUT_SUGGESTION",
-  "sqlState" : "42605",
+  "errorClass" : "DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE",


I feel the previous error is better. Shall we tune the named-parameter framework so that it always checks the num args first?

Agree. We shouldn't change the error messages for function invocations that do not use named arguments.

cloud-fan · 2023-07-13T08:56:05Z

...lyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/SupportsNamedArguments.scala

+    val parameters: Seq[NamedArgument] = functionSignature.parameters
+    val firstOptionalParamIndex: Int = parameters.indexWhere(_.default.isDefined)
+    if (firstOptionalParamIndex != -1 &&
+        parameters.drop(firstOptionalParamIndex).exists(_.default.isEmpty)) {


nit: parameters.dropWhile(_.default.isEmpty).exists(_.default.isEmpty) is simpler

cloud-fan · 2023-07-13T08:58:38Z

...lyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/SupportsNamedArguments.scala

+        functionName, functionSignature)
+    }
+
+    val firstNamedArgIdx: Int = args.indexWhere(_.isInstanceOf[NamedArgumentExpression])


nit: val (posArgs, namedArgs) = args.span(!_.isInstanceOf[NamedArgumentExpression])

cloud-fan · 2023-07-13T09:00:15Z

...lyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/SupportsNamedArguments.scala

+    // The following loop checks for the following:
+    // 1. Unrecognized parameter names
+    // 2. Duplicate routine parameter assignments
+    val allParameterNames: Seq[String] = parameters.map(_.name)


to confirm: the parameter name is case-sensitive. Is it also true in other systems or the SQL standard? cc @srielau

cloud-fan · 2023-07-13T09:02:22Z

...lyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/SupportsNamedArguments.scala

+import org.apache.spark.sql.catalyst.expressions.{Expression, NamedArgumentExpression}
+import org.apache.spark.sql.errors.QueryCompilationErrors
+
+object SupportsNamedArguments {


Do we have this object for ease of test? It looks to me that we can move the new trait Builder[T] to a new file and define the defaultRearrange function there.

MaxGekk · 2023-07-13T16:00:24Z

sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala

+      functionName: String, functionSignature: FunctionSignature) : Throwable = {
+    val errorMessage = s"Function $functionName has an unexpected required argument for" +
+      s" the provided function signature $functionSignature. All required arguments should" +
+      s" come before optional arguments."


s is not needed.

MaxGekk · 2023-07-13T16:01:48Z

sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala

+  def multipleFunctionSignatures(functionName: String,
+      functionSignatures: Seq[FunctionSignature]): Throwable = {
+    var errorMessage = s"Function $functionName cannot have multiple method signatures." +
+      s" The function signatures found were: \n"


s is not needed.

MaxGekk · 2023-07-13T16:02:41Z

sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala

+  def positionalAndNamedArgumentDoubleReference(
+      functionName: String, parameterName: String) : Throwable = {
+    val errorClass =
+      "DUPLICATE_ROUTINE_PARAMETER_ASSIGNMENT.POSITIONAL_AND_NAMED_ARGUMENT_DOUBLE_REFERENCE"


It is not long. Is it possible to make it shorter?

maybe DUPLICATE_ROUTINE_PARAMETER_ASSIGNMENT.BOTH_POSITIONAL_AND_NAMED?

MaxGekk · 2023-07-13T16:06:28Z

sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala

+    var candidatesString = ""
+    recommendations.foreach(candidatesString += _ + " ")


Could you just use mkString, please.

allisonwang-db · 2023-07-13T16:29:45Z

sql/core/src/test/resources/sql-tests/results/table-valued-functions.sql.out

+    "inputType" : "\"INT\"",
+    "paramIndex" : "1",
+    "requiredType" : "(\"ARRAY\" or \"MAP\")",
+    "sqlExpr" : "\"explode(1)\""


shouldn't this be explode (1,2)?

allisonwang-db · 2023-07-13T16:34:31Z

sql/core/src/test/resources/sql-tests/results/table-valued-functions.sql.out

+    "sqlExpr" : "\"explode(1)\""
  },
  "queryContext" : [ {
    "objectType" : "",


Can we add more named argument tests in this file (table-valued-functions.sql) when the function invocation throws exceptions? For example use named arguments for these tests:

spark/sql/core/src/test/resources/sql-tests/results/table-valued-functions.sql.out

Lines 349 to 355 in efed395

select * from explode(explode(array(1)))

-- !query schema

struct<>

-- !query output

org.apache.spark.sql.AnalysisException

{

"errorClass" : "UNSUPPORTED_GENERATOR.NESTED_IN_EXPRESSIONS",

I'd like to make sure the exceptions stay the same.

Sure, that's fine. I fixed the tests now so the errors remain consistent.

dtenedor

This is looking good, the only remaining changes left look like following Wenchen's suggestion to change the API to return an Option[FunctionSignature] instead of an Option[Seq[FunctionSignature]] for now (since we don't plan to support overloads in the short term), and some code commenting improvements.

dtenedor · 2023-07-13T17:03:23Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

+      expressions: Seq[Expression]) : Seq[Expression] = {
+    val rearrangedExpressions = if (!builder.functionSignatures.isEmpty) {
+      val functionSignatures = builder.functionSignatures.get
+      if (functionSignatures.length != 1) {


Good question, I imagine we probably would not get around to prioritizing overloads soon with this framework. But if we did, would it become necessary to update all the places where these function signatures are defined, from an Option[FunctionSignature] to an Option[Seq[FunctionSignature]]? In contrast, if we go with the latter syntax of a Seq of just one function signature, we wouldn't have to update all those sites later.

However, I imagine such an update would be a fairly mechanical change, and in the interim, we get better compiler enforcement. It seems like this suggestion is better for now to switch the API to return just an Option[FunctionSignature] for now.

dtenedor · 2023-07-13T17:03:44Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

    (name, (info, (expressions: Seq[Expression]) => builder(expressions)))
  }

+  def generatorBuilder[T <: GeneratorBuilder : ClassTag](


please add a comment for this method

dtenedor · 2023-07-13T17:04:12Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

+    val funcBuilder = (expressions: Seq[Expression]) => {
+      assert(expressions.forall(_.resolved), "function arguments must be resolved.")
+      val rearrangedExpressions = FunctionRegistry.rearrangeExpressions(name, builder, expressions)
+      val plan = builder.build(name, rearrangedExpressions)


you don't need the val plan here since you can just return builder.build(...)

dtenedor · 2023-07-13T17:05:50Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala


-trait ExpressionBuilder {
-  def build(funcName: String, expressions: Seq[Expression]): Expression
+trait Builder[T] {


please put some kind of short general comment for this Builder class mentioning that it is an interface for describing the expected parameters for SQL functions and takes responsibility for validating provided arguments for tham.

dtenedor · 2023-07-13T17:06:28Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

+}
+
+/**
+ * A trait used for scalar valued functions that defines how their expression representations


please change this to a complete sentence ending with a period (same for L1120 below)

dtenedor · 2023-07-13T17:07:24Z

sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala

+  def positionalAndNamedArgumentDoubleReference(
+      functionName: String, parameterName: String) : Throwable = {
+    val errorClass =
+      "DUPLICATE_ROUTINE_PARAMETER_ASSIGNMENT.POSITIONAL_AND_NAMED_ARGUMENT_DOUBLE_REFERENCE"


maybe DUPLICATE_ROUTINE_PARAMETER_ASSIGNMENT.BOTH_POSITIONAL_AND_NAMED?

dtenedor · 2023-07-13T17:08:31Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/FunctionBuilderBase.scala

+import org.apache.spark.sql.catalyst.expressions.{Expression, NamedArgumentExpression}
+import org.apache.spark.sql.errors.QueryCompilationErrors
+
+trait FunctionBuilderBase[T] {


please add a general comment describing what this object represents (using full sentences). Same for NamedArgumentsSupport below.

dtenedor · 2023-07-13T17:08:53Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/FunctionBuilderBase.scala

+   * compare a function call with provided arguments to determine if that function
+   * call is a match for the function signature.
+   *
+   * @return a list of function signatures


no need for this line since the above comment conveys that information.

dtenedor · 2023-07-13T17:09:44Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/FunctionBuilderBase.scala

+      }
+    }
+
+    // Check argument list size against provided parameter list length


Suggested change

// Check argument list size against provided parameter list length

// Check the argument list size against the provided parameter list length.

anchovYu · 2023-07-13T17:09:14Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/FunctionBuilderBase.scala

+        }
+      )
+    }
+    positionalArgs ++ rearrangedNamedArgs


Could you add an assertion that the size of returning results is equal to the parameter count in function signature? Meanwhile, build can also have similar assertion as well.

anchovYu · 2023-07-13T17:14:47Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/FunctionBuilderBase.scala

+   * Each function signature includes a list of parameters to which the analyzer can
+   * compare a function call with provided arguments to determine if that function
+   * call is a match for the function signature.
+   *


Could you add comment that we only support 1) one function signature 2) required should come before optional?

anchovYu · 2023-07-13T17:18:37Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/FunctionBuilderBase.scala

+    }
+
+    // Check argument list size against provided parameter list length
+    if (parameters.size < args.length) {


Could you add tests for this case? Is this exception only possible to hit when passing in all positional args?

I think we already have tests for these like in the context of explode for example.

Thanks. Could you add one for mask in the bunch of error cases in the e2e tests?

anchovYu · 2023-07-13T17:37:05Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/FunctionBuilderBase.scala

+   * This method is the default routine which rearranges the arguments in positional order according
+   * to the function signature provided. This will also fill in any default values that exists for
+   * optional arguments. This method will also be invoked even if there are no named arguments in
+   * the argument list.


Could you add more details:

keeps all positional args in the original order

rearranges named arguments according to the order defined in function signature, fill in missing optional arguments with default values

anchovYu · 2023-07-13T17:38:33Z

sql/core/src/test/resources/sql-tests/inputs/named-function-arguments.sql

@@ -1,5 +1,50 @@
+-- Test for named arguments for Mask


Could you add more tests on passing in all positional args and filling in the defaults?

I think the current mask-expression.sql test already handles this.

anchovYu · 2023-07-13T17:45:56Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

  private def expressionGeneratorOuter[T <: Generator : ClassTag](name: String)
    : (String, (ExpressionInfo, FunctionBuilder)) = {
-    val (_, (info, generatorBuilder)) = expression[T](name)
+    val (_, (info, builder)) = expression[T](name)


It this change necessary?

I felt it was because the name I created had the exact same name as the variable above and I didn't want to cause confusion. Also, in other example functions, builder is the variable name used for this kind of object, so I thought it be good to clear up the ambigiuity.

anchovYu · 2023-07-13T21:23:59Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/maskExpressions.scala

+    val upperCharArg = NamedArgument("upperChar", Some(Literal(Mask.MASKED_UPPERCASE)))
+    val lowerCharArg = NamedArgument("lowerChar", Some(Literal(Mask.MASKED_LOWERCASE)))
+    val digitCharArg = NamedArgument("digitChar", Some(Literal(Mask.MASKED_DIGIT)))
+    val otherCharArg = NamedArgument("otherChar", Some(Literal(Mask.MASKED_IGNORE)))


Existing constructors on mask adds a type information: Literal(Mask.MASKED_IGNORE, StringType)). Could you change it?

anchovYu · 2023-07-13T21:36:16Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/FunctionBuilderBase.scala

+    }
+
+    // Check argument list size against provided parameter list length
+    if (parameters.size < args.length) {


Thanks. Could you add one for mask in the bunch of error cases in the e2e tests?

srielau · 2023-07-13T23:49:50Z

common/utils/src/main/resources/error/error-classes.json

+  "NAMED_ARGUMENTS_NOT_SUPPORTED" : {
+    "message" : [
+      "Named arguments are not supported for function <functionName>; please retry the query with positional arguments to the function call instead."


Suggested change

"NAMED_ARGUMENTS_NOT_SUPPORTED" : {

"message" : [

"Named arguments are not supported for function <functionName>; please retry the query with positional arguments to the function call instead."

"NAMED_PARAMETERS_NOT_SUPPORTED" : {

"message" : [

"Named parameters are not supported for function <functionName>; please retry the query with positional arguments to the function call instead."

srielau · 2023-07-13T23:50:22Z

common/utils/src/main/resources/error/error-classes.json

+    ],
+    "sqlState" : "4274K"
+  },
  "NAMED_ARGUMENTS_SUPPORT_DISABLED" : {


Suggested change

"NAMED_ARGUMENTS_SUPPORT_DISABLED" : {

"NAMED_PARAMETER_SUPPORT_DISABLED" : {

dtenedor · 2023-07-14T00:06:50Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/FunctionBuilderBase.scala

+
+/**
+ * This is a base trait that is used for implementing builder classes that can be used to construct
+ * expressions or logical plans depending on if it is a table-valued or scala-valued function.


Suggested change

* expressions or logical plans depending on if it is a table-valued or scala-valued function.

* expressions or logical plans depending on if it is a table-valued or scalar function.

cloud-fan · 2023-07-14T08:08:52Z

seems ExpressionInfoSuite has test failures.

cloud-fan · 2023-07-14T08:10:12Z

common/utils/src/main/resources/error/error-classes.json

    ],
    "sqlState" : "428C4"
  },
+  "UNRECOGNIZED_PARAMETER_NAME" : {


Suggested change

"UNRECOGNIZED_PARAMETER_NAME" : {

"UNRECOGNIZED_ARGUMENT_NAME" : {

I asked Serge, and the original name is right. There's probably some confusion over the technical difference between argument and parameter. Typically, parameter is what each name is referring to.

cloud-fan

LGTM if all tests pass

learningchess2003 · 2023-07-15T23:33:35Z

Migrating to #42020 due to some problems with branches.

SPARK-44059

7b9d696

github-actions bot added the SQL label Jul 5, 2023

Regenerating golden files plus errors

c905b1e

anchovYu reviewed Jul 5, 2023

View reviewed changes

Addressing comments

be77e54

MaxGekk reviewed Jul 5, 2023

View reviewed changes

Adding suggestions

d88129d

learningchess2003 requested review from MaxGekk and anchovYu July 5, 2023 21:27

dtenedor reviewed Jul 5, 2023

View reviewed changes

learningchess2003 and others added 6 commits July 5, 2023 15:06

Update sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryC…

1c6717d

…ompilationErrors.scala Co-authored-by: Daniel Tenedorio <[email protected]>

Update common/utils/src/main/resources/error/error-classes.json

803a005

Co-authored-by: Daniel Tenedorio <[email protected]>

Update sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expr…

60a61c2

…essions/aggregate/CountMinSketchAgg.scala Co-authored-by: Daniel Tenedorio <[email protected]>

Update sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expr…

9728ce3

…essions/maskExpressions.scala Co-authored-by: Daniel Tenedorio <[email protected]>

Some changes

1f55065

Reversing declarations

f4fc8d1

learningchess2003 requested a review from dtenedor July 5, 2023 22:28

anchovYu reviewed Jul 5, 2023

View reviewed changes

learningchess2003 and others added 2 commits July 5, 2023 15:50

Fix failing test

67440d2

Update sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plan…

cc82b0e

…s/logical/SupportsNamedArguments.scala Co-authored-by: Xinyi <[email protected]>

anchovYu reviewed Jul 5, 2023

View reviewed changes

common/utils/src/main/resources/error/error-classes.json Show resolved Hide resolved

learningchess2003 and others added 4 commits July 5, 2023 16:17

Update sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expr…

a9cb890

…essions/maskExpressions.scala Co-authored-by: Xinyi <[email protected]>

Update common/utils/src/main/resources/error/error-classes.json

3706e34

Co-authored-by: Xinyi <[email protected]>

new count_min_sketch tests

1feb222

Add SQLSTATE documentation

61fb514

github-actions bot added the DOCS label Jul 6, 2023

learningchess2003 added 2 commits July 12, 2023 15:38

Adding some more support

53d1659

Adding working tests

d30fa3d

cloud-fan reviewed Jul 13, 2023

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/maskExpressions.scala Show resolved Hide resolved

cloud-fan reviewed Jul 13, 2023

View reviewed changes

MaxGekk requested changes Jul 13, 2023

View reviewed changes

allisonwang-db reviewed Jul 13, 2023

View reviewed changes

learningchess2003 added 2 commits July 13, 2023 09:35

Addressing comments and fixing error message

03efd52

Move builder to FunctionbuilderBase

5bd65f0

dtenedor reviewed Jul 13, 2023

View reviewed changes

anchovYu reviewed Jul 13, 2023

View reviewed changes

learningchess2003 added 2 commits July 13, 2023 11:10

Adding some stuff

6914a2b

Fixing random generation

9317f2f

anchovYu reviewed Jul 13, 2023

View reviewed changes

srielau approved these changes Jul 13, 2023

View reviewed changes

dtenedor approved these changes Jul 14, 2023

View reviewed changes

cloud-fan reviewed Jul 14, 2023

View reviewed changes

cloud-fan approved these changes Jul 14, 2023

View reviewed changes

learningchess2003 added 4 commits July 14, 2023 11:02

Some unexpected stufgf

4660881

Adding some more e2e tests

60d1f06

Regenerating documentation

8ac8e4c

Adding stuff

b6c3d8a

learningchess2003 closed this Jul 15, 2023

	case class NamedArgument(name: String,
	case class NamedArgument(
	name: String,

		var candidatesString = ""
		recommendations.foreach(candidatesString += _ + " ")

	select * from explode(explode(array(1)))
	-- !query schema
	struct<>
	-- !query output
	org.apache.spark.sql.AnalysisException
	{
	"errorClass" : "UNSUPPORTED_GENERATOR.NESTED_IN_EXPRESSIONS",

	// Check argument list size against provided parameter list length
	// Check the argument list size against the provided parameter list length.

	"NAMED_ARGUMENTS_SUPPORT_DISABLED" : {
	"NAMED_PARAMETER_SUPPORT_DISABLED" : {

	* expressions or logical plans depending on if it is a table-valued or scala-valued function.
	* expressions or logical plans depending on if it is a table-valued or scalar function.

	"UNRECOGNIZED_PARAMETER_NAME" : {
	"UNRECOGNIZED_ARGUMENT_NAME" : {

[SPARK-44059] Add analyzer support of named arguments for built-in functions #41864

[SPARK-44059] Add analyzer support of named arguments for built-in functions #41864

Uh oh!

Conversation

learningchess2003 commented Jul 5, 2023

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

learningchess2003 commented Jul 5, 2023

Uh oh!

anchovYu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

learningchess2003 commented Jul 5, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

learningchess2003 commented Jul 5, 2023

Uh oh!

dtenedor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

learningchess2003 commented Jul 5, 2023

Uh oh!

anchovYu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaxGekk Jul 13, 2023 •

edited

Loading