You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "srielau (via GitHub)" <gi...@apache.org> on 2023/09/18 17:38:54 UTC
[GitHub] [spark] srielau opened a new pull request, #42985: [SPARK-44838][SQL] raise_error improvement
srielau opened a new pull request, #42985:
URL: https://github.com/apache/spark/pull/42985
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'core/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
### Was this patch authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] itholic commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1336491031
##########
python/pyspark/sql/tests/test_functions.py:
##########
@@ -1031,10 +1031,10 @@ def check_assert_true(self, tpe):
[Row(val=None), Row(val=None), Row(val=None)],
)
- with self.assertRaisesRegex(tpe, "too big"):
+ with self.assertRaisesRegex(tpe, "\[USER_RAISED_EXCEPTION\] too big"):
Review Comment:
```suggestion
with self.assertRaisesRegex(tpe, r"\[USER_RAISED_EXCEPTION\] too big"):
```
##########
python/pyspark/sql/tests/test_functions.py:
##########
@@ -1031,10 +1031,10 @@ def check_assert_true(self, tpe):
[Row(val=None), Row(val=None), Row(val=None)],
)
- with self.assertRaisesRegex(tpe, "too big"):
+ with self.assertRaisesRegex(tpe, "\[USER_RAISED_EXCEPTION\] too big"):
df.select(F.assert_true(df.id < 2, "too big")).toDF("val").collect()
- with self.assertRaisesRegex(tpe, "2000000"):
+ with self.assertRaisesRegex(tpe, "\[USER_RAISED_EXCEPTION\] 2000000.0"):
Review Comment:
```suggestion
with self.assertRaisesRegex(tpe, r"\[USER_RAISED_EXCEPTION\] 2000000.0"):
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] itholic commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1335371923
##########
python/pyspark/sql/tests/test_functions.py:
##########
@@ -1032,9 +1032,11 @@ def check_assert_true(self, tpe):
)
with self.assertRaisesRegex(tpe, "too big"):
- df.select(F.assert_true(df.id < 2, "too big")).toDF("val").collect()
+ df.select(F.assert_true(df.id < 2, "[USER_RAISED_EXCEPTION] too big")).toDF(
+ "val"
+ ).collect()
- with self.assertRaisesRegex(tpe, "2000000"):
+ with self.assertRaisesRegex(tpe, "[USER_RAISED_EXCEPTION] 2000000.0"):
Review Comment:
```suggestion
with self.assertRaisesRegex(tpe, "\[USER_RAISED_EXCEPTION\] 2000000.0"):
```
Maybe this should work? `[` is treated as a special character for regex, so we should escape it.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1333970780
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala:
##########
@@ -61,68 +62,97 @@ case class PrintToStderr(child: Expression) extends UnaryExpression {
/**
* Throw with the result of an expression (used for debugging).
*/
+// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr) - Throws an exception with `expr`.",
+ usage = "_FUNC_( expr [, errorParams ]) - Throws a USER_RAISED_EXCEPTION with `expr` as message, or a defined error class in `expr` with a parameter map.",
examples = """
Examples:
> SELECT _FUNC_('custom error message');
- java.lang.RuntimeException
- custom error message
+ [USER_RAISED_EXCEPTION] custom error message
+
+ > SELECT _FUNC_('VIEW_NOT_FOUND', Map('relationName' -> '`V1`'));
+ [VIEW_NOT_FOUND] The view `V1` cannot be found. ...
""",
since = "3.1.0",
group = "misc_funcs")
-case class RaiseError(child: Expression, dataType: DataType)
- extends UnaryExpression with ImplicitCastInputTypes {
+// scalastyle:on line.size.limit
+case class RaiseError(errorClass: Expression, errorParms: Expression, dataType: DataType)
+ extends BinaryExpression with ImplicitCastInputTypes {
+
+ def this(str: Expression) = {
+ this(Literal(
+ if (SQLConf.get.legacyNegativeIndexInArrayInsert) {
Review Comment:
what's this?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srielau commented on a diff in pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1338009883
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -3305,6 +3305,14 @@ object functions {
*/
def raise_error(c: Column): Column = Column.fn("raise_error", c)
+ /**
+ * Throws an exception with the provided error message.
+ *
+ * @group misc_funcs
+ * @since 3.5.0
Review Comment:
```suggestion
* @since 4.0.0
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #42985:
URL: https://github.com/apache/spark/pull/42985#issuecomment-1736965187
unfortunately, there are conflicts now...
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1336566184
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -5310,6 +5321,9 @@ class SQLConf extends Serializable with Logging with SqlApiConf {
getConf(SQLConf.LEGACY_NEGATIVE_INDEX_IN_ARRAY_INSERT)
}
+ def legacyRaiseErrorWithoutErrorClass: Boolean =
Review Comment:
where do we read this conf?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srielau commented on pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on PR #42985:
URL: https://github.com/apache/spark/pull/42985#issuecomment-1735899527
@cloud-fan I've added QA and addressed your initial comments. This is ready for final review.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1338012200
##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -3250,6 +3250,16 @@ object functions {
*/
def raise_error(c: Column): Column = Column.fn("raise_error", c)
+ /**
+ * Throws an exception with the provided error class and parameter map.
+ *
+ * @group misc_funcs
+ * @since 3.5.0
+ */
+ def raise_error(c: Column, e: Column): Column = withExpr {
+ RaiseError(c.expr, e.expr)
Review Comment:
should call `Column.fn`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srielau commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1334513626
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -4432,6 +4432,17 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val LEGACY_RAISE_ERROR_WITHOUT_ERROR_CLASS =
+ buildConf("spark.sql.legacy.raiseErrorWithoutErrorClass")
+ .internal()
+ .doc("When set to true, restores the legacy behavior of `raise_error` and `assert_true` to " +
+ "not return the `[USER_RAISED_EXCEPTION]` prefix." +
+ "For example, `raise_error('error!')` returns `error!` instead of " +
+ "`[[USER_RAISED_EXCEPTION] Error!`.")
Review Comment:
I spoke to Xiao, and he also recommended a config. There are two things at play here:
The exception changed away from RuntimeException, and we got the prefix.
It smells like we have a decent chance of breaking anyone who wants to catch these exceptions.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1334552099
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala:
##########
@@ -61,68 +62,92 @@ case class PrintToStderr(child: Expression) extends UnaryExpression {
/**
* Throw with the result of an expression (used for debugging).
*/
+// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr) - Throws an exception with `expr`.",
+ usage = "_FUNC_( expr [, errorParams ]) - Throws a USER_RAISED_EXCEPTION with `expr` as message, or a defined error class in `expr` with a parameter map.",
examples = """
Examples:
> SELECT _FUNC_('custom error message');
- java.lang.RuntimeException
- custom error message
+ [USER_RAISED_EXCEPTION] custom error message
+
+ > SELECT _FUNC_('VIEW_NOT_FOUND', Map('relationName' -> '`V1`'));
+ [VIEW_NOT_FOUND] The view `V1` cannot be found. ...
""",
since = "3.1.0",
group = "misc_funcs")
-case class RaiseError(child: Expression, dataType: DataType)
- extends UnaryExpression with ImplicitCastInputTypes {
+// scalastyle:on line.size.limit
+case class RaiseError(errorClass: Expression, errorParms: Expression, dataType: DataType)
+ extends BinaryExpression with ImplicitCastInputTypes {
- def this(child: Expression) = this(child, NullType)
+ def this(str: Expression) = {
+ this(Literal("USER_RAISED_EXCEPTION"),
+ CreateMap(Seq(Literal("errorMessage"), str)), NullType)
+ }
+
+ def this(errorClass: Expression, errorParms: Expression) = {
+ this(errorClass, errorParms, NullType)
+ }
override def foldable: Boolean = false
override def nullable: Boolean = true
- override def inputTypes: Seq[AbstractDataType] = Seq(StringType)
+ override def inputTypes: Seq[AbstractDataType] =
+ Seq(StringType, MapType(StringType, StringType))
+
+ override def left: Expression = errorClass
+ override def right: Expression = errorParms
override def prettyName: String = "raise_error"
override def eval(input: InternalRow): Any = {
- val value = child.eval(input)
- if (value == null) {
- throw new RuntimeException()
- }
- throw new RuntimeException(value.toString)
+ val error = errorClass.eval(input).asInstanceOf[UTF8String]
+ val parms: MapData = errorParms.eval(input).asInstanceOf[MapData]
+ throw raiseError(error, parms)
}
// if (true) is to avoid codegen compilation exception that statement is unreachable
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val eval = child.genCode(ctx)
+ val error = errorClass.genCode(ctx)
+ val parms = errorParms.genCode(ctx)
ExprCode(
- code = code"""${eval.code}
+ code = code"""${error.code}
Review Comment:
```suggestion
code = code"""${error.code}
|${parms.code}
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1336565053
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala:
##########
@@ -61,64 +62,83 @@ case class PrintToStderr(child: Expression) extends UnaryExpression {
/**
* Throw with the result of an expression (used for debugging).
*/
+// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr) - Throws an exception with `expr`.",
+ usage = "_FUNC_( expr [, errorParams ]) - Throws a USER_RAISED_EXCEPTION with `expr` as message, or a defined error class in `expr` with a parameter map.",
examples = """
Examples:
> SELECT _FUNC_('custom error message');
- java.lang.RuntimeException
- custom error message
+ [USER_RAISED_EXCEPTION] custom error message
+
+ > SELECT _FUNC_('VIEW_NOT_FOUND', Map('relationName' -> '`V1`'));
Review Comment:
This looks a bit ambiguous. What if the error class has no parameters? If we call `raise_error("MY_ERROR_CLASS")`, the error class is treated as the error message.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1333974507
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -4432,6 +4432,17 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val LEGACY_RAISE_ERROR_WITHOUT_ERROR_CLASS =
+ buildConf("spark.sql.legacy.raiseErrorWithoutErrorClass")
+ .internal()
+ .doc("When set to true, restores the legacy behavior of `raise_error` and `assert_true` to " +
+ "not return the `[USER_RAISED_EXCEPTION]` prefix." +
+ "For example, `raise_error('error!')` returns `error!` instead of " +
+ "`[[USER_RAISED_EXCEPTION] Error!`.")
Review Comment:
do we really need a config? who cares if we put an error class name in the beginning of error message...
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1336560079
##########
common/utils/src/main/scala/org/apache/spark/ErrorClassesJSONReader.scala:
##########
@@ -85,6 +92,18 @@ class ErrorClassesJsonReader(jsonFileURLs: Seq[URL]) {
.flatMap(_.sqlState)
.orNull
}
+
+ def validateErrorClass(errorClass: String): Boolean = {
+ val errorClasses = errorClass.split("\\.")
+ val result = errorClasses match {
+ case Array(mainClass) => errorInfoMap.contains(mainClass)
+ case Array(mainClass, subClass) => errorInfoMap.get(mainClass).map { info =>
+ info.subClass.get.contains(subClass)
+ }.getOrElse(false)
+ case _ => false
+ }
+ result
Review Comment:
```suggestion
errorClasses match {
case Array(mainClass) => errorInfoMap.contains(mainClass)
case Array(mainClass, subClass) => errorInfoMap.get(mainClass).map { info =>
info.subClass.get.contains(subClass)
}.getOrElse(false)
case _ => false
}
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1338009252
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -3305,6 +3305,14 @@ object functions {
*/
def raise_error(c: Column): Column = Column.fn("raise_error", c)
+ /**
+ * Throws an exception with the provided error message.
+ *
+ * @group misc_funcs
+ * @since 3.5.0
Review Comment:
it should be `4.0.0`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] itholic commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1336508758
##########
python/pyspark/sql/tests/test_functions.py:
##########
@@ -1031,10 +1031,10 @@ def check_assert_true(self, tpe):
[Row(val=None), Row(val=None), Row(val=None)],
)
- with self.assertRaisesRegex(tpe, "too big"):
+ with self.assertRaisesRegex(tpe, "\[USER_RAISED_EXCEPTION\] too big"):
Review Comment:
Oh, seems like this is missing. Could you also apply this suggestion?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srielau commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1337482091
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -5310,6 +5321,9 @@ class SQLConf extends Serializable with Logging with SqlApiConf {
getConf(SQLConf.LEGACY_NEGATIVE_INDEX_IN_ARRAY_INSERT)
}
+ def legacyRaiseErrorWithoutErrorClass: Boolean =
Review Comment:
Ah, I got interrupted and lost in the regression.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1338010035
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala:
##########
@@ -61,64 +62,88 @@ case class PrintToStderr(child: Expression) extends UnaryExpression {
/**
* Throw with the result of an expression (used for debugging).
*/
+// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr) - Throws an exception with `expr`.",
+ usage = "_FUNC_( expr [, errorParams ]) - Throws a USER_RAISED_EXCEPTION with `expr` as message, or a defined error class in `expr` with a parameter map.",
Review Comment:
should we highlight that null `errorParams` means no param? With an example like `raise_error("CANNOT_PARSE_DECIMAL", null)`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1336560281
##########
common/utils/src/main/scala/org/apache/spark/ErrorClassesJSONReader.scala:
##########
@@ -85,6 +92,18 @@ class ErrorClassesJsonReader(jsonFileURLs: Seq[URL]) {
.flatMap(_.sqlState)
.orNull
}
+
+ def validateErrorClass(errorClass: String): Boolean = {
Review Comment:
since it returns a boolean, a better name is `isValidErrorClass`
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang closed pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang closed pull request #42985: [SPARK-44838][SQL] raise_error improvement
URL: https://github.com/apache/spark/pull/42985
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srielau commented on pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on PR #42985:
URL: https://github.com/apache/spark/pull/42985#issuecomment-1736171673
@cloud-fan It all green! ... ship-it.... :-)
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1338011832
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -4443,6 +4443,17 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val LEGACY_RAISE_ERROR_WITHOUT_ERROR_CLASS =
+ buildConf("spark.sql.legacy.raiseErrorWithoutErrorClass")
+ .internal()
+ .doc("When set to true, restores the legacy behavior of `raise_error` and `assert_true` to " +
+ "not return the `[USER_RAISED_EXCEPTION]` prefix." +
+ "For example, `raise_error('error!')` returns `error!` instead of " +
+ "`[[USER_RAISED_EXCEPTION] Error!`.")
Review Comment:
```suggestion
"`[USER_RAISED_EXCEPTION] Error!`.")
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -4443,6 +4443,17 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val LEGACY_RAISE_ERROR_WITHOUT_ERROR_CLASS =
+ buildConf("spark.sql.legacy.raiseErrorWithoutErrorClass")
+ .internal()
+ .doc("When set to true, restores the legacy behavior of `raise_error` and `assert_true` to " +
+ "not return the `[USER_RAISED_EXCEPTION]` prefix." +
+ "For example, `raise_error('error!')` returns `error!` instead of " +
+ "`[[USER_RAISED_EXCEPTION] Error!`.")
+ .version("3.5.0")
Review Comment:
```suggestion
.version("4.0.0")
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1338010923
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala:
##########
@@ -2728,4 +2729,49 @@ private[sql] object QueryExecutionErrors extends QueryErrorsBase with ExecutionE
errorClass = "UNSUPPORTED_FEATURE.PURGE_TABLE",
messageParameters = Map.empty)
}
+
+ def raiseError(errorClass: UTF8String,
+ errorParms: MapData): RuntimeException = {
Review Comment:
```suggestion
def raiseError(
errorClass: UTF8String,
errorParms: MapData): RuntimeException = {
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1338012091
##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -3250,6 +3250,16 @@ object functions {
*/
def raise_error(c: Column): Column = Column.fn("raise_error", c)
+ /**
+ * Throws an exception with the provided error class and parameter map.
+ *
+ * @group misc_funcs
+ * @since 3.5.0
Review Comment:
```suggestion
* @since 4.0.0
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1336560622
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -6310,5 +6330,11 @@
"message" : [
"Failed to get block <blockId>, which is not a shuffle block"
]
+ },
+ "_LEGACY_ERROR_USER_RAISED_EXCEPTION" : {
Review Comment:
this is a new prefix. how is it different from `_LEGACY_ERROR_TEMP`?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1334552910
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala:
##########
@@ -61,68 +62,92 @@ case class PrintToStderr(child: Expression) extends UnaryExpression {
/**
* Throw with the result of an expression (used for debugging).
*/
+// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr) - Throws an exception with `expr`.",
+ usage = "_FUNC_( expr [, errorParams ]) - Throws a USER_RAISED_EXCEPTION with `expr` as message, or a defined error class in `expr` with a parameter map.",
examples = """
Examples:
> SELECT _FUNC_('custom error message');
- java.lang.RuntimeException
- custom error message
+ [USER_RAISED_EXCEPTION] custom error message
+
+ > SELECT _FUNC_('VIEW_NOT_FOUND', Map('relationName' -> '`V1`'));
+ [VIEW_NOT_FOUND] The view `V1` cannot be found. ...
""",
since = "3.1.0",
group = "misc_funcs")
-case class RaiseError(child: Expression, dataType: DataType)
- extends UnaryExpression with ImplicitCastInputTypes {
+// scalastyle:on line.size.limit
+case class RaiseError(errorClass: Expression, errorParms: Expression, dataType: DataType)
+ extends BinaryExpression with ImplicitCastInputTypes {
- def this(child: Expression) = this(child, NullType)
+ def this(str: Expression) = {
+ this(Literal("USER_RAISED_EXCEPTION"),
+ CreateMap(Seq(Literal("errorMessage"), str)), NullType)
+ }
+
+ def this(errorClass: Expression, errorParms: Expression) = {
+ this(errorClass, errorParms, NullType)
+ }
override def foldable: Boolean = false
override def nullable: Boolean = true
- override def inputTypes: Seq[AbstractDataType] = Seq(StringType)
+ override def inputTypes: Seq[AbstractDataType] =
+ Seq(StringType, MapType(StringType, StringType))
+
+ override def left: Expression = errorClass
+ override def right: Expression = errorParms
override def prettyName: String = "raise_error"
override def eval(input: InternalRow): Any = {
- val value = child.eval(input)
- if (value == null) {
- throw new RuntimeException()
- }
- throw new RuntimeException(value.toString)
+ val error = errorClass.eval(input).asInstanceOf[UTF8String]
+ val parms: MapData = errorParms.eval(input).asInstanceOf[MapData]
+ throw raiseError(error, parms)
}
// if (true) is to avoid codegen compilation exception that statement is unreachable
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val eval = child.genCode(ctx)
+ val error = errorClass.genCode(ctx)
+ val parms = errorParms.genCode(ctx)
ExprCode(
- code = code"""${eval.code}
+ code = code"""${error.code}
Review Comment:
We may need to check the nullability of error and params as well.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srielau commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1334513626
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -4432,6 +4432,17 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val LEGACY_RAISE_ERROR_WITHOUT_ERROR_CLASS =
+ buildConf("spark.sql.legacy.raiseErrorWithoutErrorClass")
+ .internal()
+ .doc("When set to true, restores the legacy behavior of `raise_error` and `assert_true` to " +
+ "not return the `[USER_RAISED_EXCEPTION]` prefix." +
+ "For example, `raise_error('error!')` returns `error!` instead of " +
+ "`[[USER_RAISED_EXCEPTION] Error!`.")
Review Comment:
I spoke to @gatorsmile , and he also recommended a config. There are two things at play here:
The exception changed away from RuntimeException, and we got the prefix.
It smells like we have a decent chance of breaking anyone who wants to catch these exceptions.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1338011596
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala:
##########
@@ -2728,4 +2729,49 @@ private[sql] object QueryExecutionErrors extends QueryErrorsBase with ExecutionE
errorClass = "UNSUPPORTED_FEATURE.PURGE_TABLE",
messageParameters = Map.empty)
}
+
+ def raiseError(errorClass: UTF8String,
+ errorParms: MapData): RuntimeException = {
+ val errorClassStr = if (errorClass != null) {
+ errorClass.toString.toUpperCase(Locale.ROOT)
+ } else {
+ "null"
+ }
+ val errorParmsMap = if (errorParms != null) {
+ val errorParmsMutable = collection.mutable.Map[String, String]()
+ errorParms.foreach(StringType, StringType, { case (key, value) =>
+ errorParmsMutable += (key.toString ->
+ (if (value == null) { "null" } else { value.toString } ))
+ })
+ errorParmsMutable.toMap
+ } else {
+ Map.empty[String, String]
+ }
+
+ // Is the error class a known error class? If not raise an error
+ if (!SparkThrowableHelper.isValidErrorClass(errorClassStr)) {
+ new SparkRuntimeException(
+ errorClass = "USER_RAISED_EXCEPTION_UNKNOWN_ERROR_CLASS",
+ messageParameters = Map("errorClass" -> toSQLValue(errorClassStr)))
+ } else {
+ // Did the user provide all parameters? If not raise an error
+ val expectedParms = SparkThrowableHelper.getMessageParameters(errorClassStr).sorted
+ val providedParms = errorParmsMap.keys.toSeq.sorted
+ if (expectedParms != providedParms) {
+ new SparkRuntimeException(
+ errorClass = "USER_RAISED_EXCEPTION_PARAMETER_MISMATCH",
+ messageParameters = Map("errorClass" -> errorClassStr,
Review Comment:
shall we call `toSQLValue` like https://github.com/apache/spark/pull/42985/files#diff-fad3de9058d5ffc779a83700a6c6477d2d3caa03b00e167c4aa87413af4ba7cdR2755 ?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srielau commented on a diff in pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1338010959
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala:
##########
@@ -61,64 +62,88 @@ case class PrintToStderr(child: Expression) extends UnaryExpression {
/**
* Throw with the result of an expression (used for debugging).
*/
+// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr) - Throws an exception with `expr`.",
+ usage = "_FUNC_( expr [, errorParams ]) - Throws a USER_RAISED_EXCEPTION with `expr` as message, or a defined error class in `expr` with a parameter map.",
Review Comment:
I cannot add examples that fail because it blows up the doc sanity tests. bets I can do is verbalize it in the description
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #42985:
URL: https://github.com/apache/spark/pull/42985#issuecomment-1737772781
Thanks, merging to master
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srielau commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1334511376
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala:
##########
@@ -61,68 +62,97 @@ case class PrintToStderr(child: Expression) extends UnaryExpression {
/**
* Throw with the result of an expression (used for debugging).
*/
+// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr) - Throws an exception with `expr`.",
+ usage = "_FUNC_( expr [, errorParams ]) - Throws a USER_RAISED_EXCEPTION with `expr` as message, or a defined error class in `expr` with a parameter map.",
examples = """
Examples:
> SELECT _FUNC_('custom error message');
- java.lang.RuntimeException
- custom error message
+ [USER_RAISED_EXCEPTION] custom error message
+
+ > SELECT _FUNC_('VIEW_NOT_FOUND', Map('relationName' -> '`V1`'));
+ [VIEW_NOT_FOUND] The view `V1` cannot be found. ...
""",
since = "3.1.0",
group = "misc_funcs")
-case class RaiseError(child: Expression, dataType: DataType)
- extends UnaryExpression with ImplicitCastInputTypes {
+// scalastyle:on line.size.limit
+case class RaiseError(errorClass: Expression, errorParms: Expression, dataType: DataType)
+ extends BinaryExpression with ImplicitCastInputTypes {
+
+ def this(str: Expression) = {
+ this(Literal(
+ if (SQLConf.get.legacyNegativeIndexInArrayInsert) {
Review Comment:
Abandoned effort to put the logic in the wrong spot. Removed.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srielau commented on a diff in pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1338012537
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala:
##########
@@ -2728,4 +2729,49 @@ private[sql] object QueryExecutionErrors extends QueryErrorsBase with ExecutionE
errorClass = "UNSUPPORTED_FEATURE.PURGE_TABLE",
messageParameters = Map.empty)
}
+
+ def raiseError(errorClass: UTF8String,
+ errorParms: MapData): RuntimeException = {
+ val errorClassStr = if (errorClass != null) {
+ errorClass.toString.toUpperCase(Locale.ROOT)
+ } else {
+ "null"
+ }
+ val errorParmsMap = if (errorParms != null) {
+ val errorParmsMutable = collection.mutable.Map[String, String]()
+ errorParms.foreach(StringType, StringType, { case (key, value) =>
+ errorParmsMutable += (key.toString ->
+ (if (value == null) { "null" } else { value.toString } ))
+ })
+ errorParmsMutable.toMap
+ } else {
+ Map.empty[String, String]
+ }
+
+ // Is the error class a known error class? If not raise an error
+ if (!SparkThrowableHelper.isValidErrorClass(errorClassStr)) {
+ new SparkRuntimeException(
+ errorClass = "USER_RAISED_EXCEPTION_UNKNOWN_ERROR_CLASS",
+ messageParameters = Map("errorClass" -> toSQLValue(errorClassStr)))
+ } else {
+ // Did the user provide all parameters? If not raise an error
+ val expectedParms = SparkThrowableHelper.getMessageParameters(errorClassStr).sorted
+ val providedParms = errorParmsMap.keys.toSeq.sorted
+ if (expectedParms != providedParms) {
+ new SparkRuntimeException(
+ errorClass = "USER_RAISED_EXCEPTION_PARAMETER_MISMATCH",
+ messageParameters = Map("errorClass" -> errorClassStr,
Review Comment:
```suggestion
messageParameters = Map("errorClass" -> toSQLvalue(errorClassStr),
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srielau commented on a diff in pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1338011972
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala:
##########
@@ -61,64 +62,88 @@ case class PrintToStderr(child: Expression) extends UnaryExpression {
/**
* Throw with the result of an expression (used for debugging).
*/
+// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr) - Throws an exception with `expr`.",
+ usage = "_FUNC_( expr [, errorParams ]) - Throws a USER_RAISED_EXCEPTION with `expr` as message, or a defined error class in `expr` with a parameter map.",
Review Comment:
```suggestion
usage = "_FUNC_( expr [, errorParams ]) - Throws a USER_RAISED_EXCEPTION with `expr` as message, or a defined error class in `expr` with a parameter map. A `null` errorParms is equivalent to an empty map.",
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srielau commented on pull request #42985: [SPARK-44838][SQL] raise_error improvement
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on PR #42985:
URL: https://github.com/apache/spark/pull/42985#issuecomment-1736667508
@cloud-fan All comments addressed
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srielau commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1336596704
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala:
##########
@@ -61,64 +62,83 @@ case class PrintToStderr(child: Expression) extends UnaryExpression {
/**
* Throw with the result of an expression (used for debugging).
*/
+// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr) - Throws an exception with `expr`.",
+ usage = "_FUNC_( expr [, errorParams ]) - Throws a USER_RAISED_EXCEPTION with `expr` as message, or a defined error class in `expr` with a parameter map.",
examples = """
Examples:
> SELECT _FUNC_('custom error message');
- java.lang.RuntimeException
- custom error message
+ [USER_RAISED_EXCEPTION] custom error message
+
+ > SELECT _FUNC_('VIEW_NOT_FOUND', Map('relationName' -> '`V1`'));
Review Comment:
The only other choice I have is to treat it like an error class first, and if I do not find any fall back to the default behavior.
But that is hardly better since it will not catch typos...
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srielau commented on a diff in pull request #42985: [SPARK-44838][SQL][WIP] raise_error improvement
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #42985:
URL: https://github.com/apache/spark/pull/42985#discussion_r1336596068
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -6310,5 +6330,11 @@
"message" : [
"Failed to get block <blockId>, which is not a shuffle block"
]
+ },
+ "_LEGACY_ERROR_USER_RAISED_EXCEPTION" : {
Review Comment:
It's not temporary.... But I still want it to be filtered out.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org