Skip to content
Closed
Original file line number Diff line number Diff line change
Expand Up @@ -6222,7 +6222,7 @@
"Detected implicit cartesian product for <joinType> join between logical plans",
"<leftPlan>",
"and",
"rightPlan",
"<rightPlan>",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of this PR is to detect mistakes like this.

"Join condition is missing or trivial.",
"Either: use the CROSS JOIN syntax to allow cartesian products between these relations, or: enable implicit cartesian products by setting the configuration variable spark.sql.crossJoin.enabled=true."
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package org.apache.spark

import java.net.URL

import scala.collection.immutable.Map
import scala.jdk.CollectionConverters._

import com.fasterxml.jackson.annotation.JsonIgnore
Expand Down Expand Up @@ -52,7 +51,7 @@ class ErrorClassesJsonReader(jsonFileURLs: Seq[URL]) {
val sub = new StringSubstitutor(sanitizedParameters.asJava)
sub.setEnableUndefinedVariableException(true)
sub.setDisableSubstitutionInValues(true)
try {
val errorMessage = try {
sub.replace(ErrorClassesJsonReader.TEMPLATE_REGEX.replaceAllIn(
messageTemplate, "\\$\\{$1\\}"))
} catch {
Expand All @@ -61,6 +60,17 @@ class ErrorClassesJsonReader(jsonFileURLs: Seq[URL]) {
s"MessageTemplate: $messageTemplate, " +
s"Parameters: $messageParameters", i)
}
if (util.SparkEnvUtils.isTesting) {
val placeHoldersNum = ErrorClassesJsonReader.TEMPLATE_REGEX.findAllIn(messageTemplate).length
if (placeHoldersNum < sanitizedParameters.size) {
throw SparkException.internalError(
s"Found unused message parameters for the error class $errorClass. " +
s"Its error message format has $placeHoldersNum place holders, " +
s"but the passed message parameters map has ${sanitizedParameters.size} items. " +
"Consider to add place holders to the error format or remove unused message parameters.")
}
}
errorMessage
}

def getMessageParameters(errorClass: String): Seq[String] = {
Expand Down
33 changes: 25 additions & 8 deletions core/src/test/scala/org/apache/spark/SparkThrowableSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -207,13 +207,6 @@ class SparkThrowableSuite extends SparkFunSuite {
}
assert(e.getErrorClass === "INTERNAL_ERROR")
assert(e.getMessageParameters().get("message").contains("Undefined error message parameter"))

// Does not fail with too many args (expects 0 args)
assert(getMessage("DIVIDE_BY_ZERO", Map("config" -> "foo", "a" -> "bar")) ==
"[DIVIDE_BY_ZERO] Division by zero. " +
"Use `try_divide` to tolerate divisor being 0 and return NULL instead. " +
"If necessary set foo to \"false\" " +
"to bypass this error. SQLSTATE: 22012")
}

test("Error message is formatted") {
Expand Down Expand Up @@ -504,7 +497,7 @@ class SparkThrowableSuite extends SparkFunSuite {
|{
| "MISSING_PARAMETER" : {
| "message" : [
| "Parameter ${param} is missing."
| "Parameter <param> is missing."
| ]
| }
|}
Expand All @@ -517,4 +510,28 @@ class SparkThrowableSuite extends SparkFunSuite {
assert(errorMessage.contains("Parameter null is missing."))
}
}

test("detect unused message parameters") {
checkError(
exception = intercept[SparkException] {
SparkThrowableHelper.getMessage(
errorClass = "CANNOT_UP_CAST_DATATYPE",
messageParameters = Map(
"expression" -> "CAST('aaa' AS LONG)",
"sourceType" -> "STRING",
"targetType" -> "LONG",
"op" -> "CAST", // unused parameter
"details" -> "implicit cast"
))
},
errorClass = "INTERNAL_ERROR",
parameters = Map(
"message" ->
("Found unused message parameters for the error class CANNOT_UP_CAST_DATATYPE. " +
"Its error message format has 4 place holders, but the passed message parameters map " +
"has 5 items. Consider to add place holders to the error format or " +
"remove unused message parameters.")
)
)
}
}