Skip to content

Commit

Permalink
Minor refactor in test assertion to use ParseException
Browse files Browse the repository at this point in the history
GitOrigin-RevId: dbe29e3e8c4886c19586808e20a723180e259de1
  • Loading branch information
hvanhovell authored and allisonport-db committed Jul 20, 2023
1 parent b2b2bd7 commit 71e0a83
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ trait DeltaAlterTableTestBase
}

protected def assertNotSupported(command: String, messages: String*): Unit = {
val ex = intercept[AnalysisException] {
val ex = intercept[Exception] {
sql(command)
}.getMessage
assert(ex.contains("not supported") || ex.contains("Unsupported") || ex.contains("Cannot"))
Expand Down Expand Up @@ -1353,7 +1353,7 @@ trait DeltaAlterTableTests extends DeltaAlterTableTestBase {
assert(ex1.getMessage.contains("Missing field V1") ||
ex1.getMessage.contains("Cannot update missing field V1"))

val ex2 = intercept[AnalysisException] {
val ex2 = intercept[ParseException] {
sql(s"ALTER TABLE $tableName CHANGE COLUMN v1 V1 integer")
}
assert(ex2.getMessage.contains("Renaming column is not supported"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import org.scalatest.GivenWhenThen
import org.apache.spark.{SparkConf, SparkException}
import org.apache.spark.sql.{AnalysisException, DataFrame, QueryTest, Row}
import org.apache.spark.sql.catalyst.TableIdentifier
import org.apache.spark.sql.catalyst.parser.ParseException
import org.apache.spark.sql.catalyst.util.quietly
import org.apache.spark.sql.connector.catalog.CatalogManager
import org.apache.spark.sql.internal.SQLConf
Expand Down Expand Up @@ -524,19 +525,19 @@ abstract class DeltaHistoryManagerBase extends DeltaTimeTravelTests
generateCommits(tblName, start, start + 20.minutes)

// These all actually fail parsing
intercept[AnalysisException] {
intercept[ParseException] {
sql(s"insert into ${versionAsOf(tblName, 0)} values (11, 12, 13)")
}

intercept[AnalysisException] {
intercept[ParseException] {
sql(s"update ${versionAsOf(tblName, 0)} set id = id - 1 where id < 10")
}

intercept[AnalysisException] {
intercept[ParseException] {
sql(s"delete from ${versionAsOf(tblName, 0)} id < 10")
}

intercept[AnalysisException] {
intercept[ParseException] {
sql(s"""merge into ${versionAsOf(tblName, 0)} old
|using $tblName new
|on old.id = new.id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ trait DeltaTableCreationTests
""".stripMargin)
}

intercept[AnalysisException] {
intercept[ParseException] {
sql(
s"""CREATE TABLE delta_test(
| a string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ trait MergeIntoNotMatchedBySourceSuite extends MergeIntoSuiteBase {
}

// Test analysis errors with NOT MATCHED BY SOURCE clauses.
testAnalysisErrorsInUnlimitedClauses(
testErrorsInUnlimitedClauses(
"error on multiple not matched by source update clauses without condition")(
mergeOn = "s.key = t.key",
updateNotMatched(condition = "t.key == 3", set = "value = 2 * value"),
Expand All @@ -84,7 +84,7 @@ trait MergeIntoNotMatchedBySourceSuite extends MergeIntoSuiteBase {
errorStrs = "when there are more than one not matched by source clauses in a merge " +
"statement, only the last not matched by source clause can omit the condition" :: Nil)

testAnalysisErrorsInUnlimitedClauses(
testErrorsInUnlimitedClauses(
"error on multiple not matched by source update/delete clauses without condition")(
mergeOn = "s.key = t.key",
updateNotMatched(condition = "t.key == 3", set = "value = 2 * value"),
Expand All @@ -93,7 +93,7 @@ trait MergeIntoNotMatchedBySourceSuite extends MergeIntoSuiteBase {
errorStrs = "when there are more than one not matched by source clauses in a merge " +
"statement, only the last not matched by source clause can omit the condition" :: Nil)

testAnalysisErrorsInUnlimitedClauses(
testErrorsInUnlimitedClauses(
"error on non-empty condition following empty condition in not matched by source " +
"update clauses")(
mergeOn = "s.key = t.key",
Expand All @@ -102,7 +102,7 @@ trait MergeIntoNotMatchedBySourceSuite extends MergeIntoSuiteBase {
errorStrs = "when there are more than one not matched by source clauses in a merge " +
"statement, only the last not matched by source clause can omit the condition" :: Nil)

testAnalysisErrorsInUnlimitedClauses(
testErrorsInUnlimitedClauses(
"error on non-empty condition following empty condition in not matched by source " +
"delete clauses")(
mergeOn = "s.key = t.key",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ class MergeIntoSQLSuite extends MergeIntoSuiteBase with MergeIntoNotMatchedBySo
append(Seq((2, 2), (1, 4)).toDF("trgKey", "trgValue"))

// only the last NOT MATCHED clause can omit the condition
val e = intercept[AnalysisException](
val e = intercept[ParseException](
sql(s"""
|MERGE INTO delta.`$tempPath`
|USING source
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import org.scalatest.BeforeAndAfterEach
import org.apache.spark.sql.{functions, AnalysisException, DataFrame, QueryTest, Row}
import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.expressions.{GenericInternalRow, UnsafeArrayData}
import org.apache.spark.sql.catalyst.parser.ParseException
import org.apache.spark.sql.catalyst.util.FailFastMode
import org.apache.spark.sql.execution.adaptive.DisableAdaptiveExecution
import org.apache.spark.sql.functions._
Expand Down Expand Up @@ -4798,14 +4799,27 @@ abstract class MergeIntoSuiteBase
result: Seq[(Int, Int)]): Unit =
testExtendedMerge(name, "unlimited clauses")(source, target, mergeOn, mergeClauses : _*)(result)

protected def testAnalysisErrorsInUnlimitedClauses(
protected def testErrorsInUnlimitedClauses(
name: String)(
mergeOn: String,
mergeClauses: MergeClause*)(
errorStrs: Seq[String],
notErrorStrs: Seq[String] = Nil): Unit =
testAnalysisErrorsInExtendedMerge(name, "unlimited clauses")(mergeOn, mergeClauses : _*)(
errorStrs, notErrorStrs)
notErrorStrs: Seq[String] = Nil): Unit = {
test(s"unlimited clauses - analysis errors - $name") {
withKeyValueData(
source = Seq.empty,
target = Seq.empty,
sourceKeyValueNames = ("key", "srcValue"),
targetKeyValueNames = ("key", "tgtValue")
) { case (sourceName, targetName) =>
val errMsg = intercept[Exception] {
executeMerge(s"$targetName t", s"$sourceName s", mergeOn, mergeClauses: _*)
}.getMessage
errorStrs.foreach { s => errorContains(errMsg, s) }
notErrorStrs.foreach { s => errorNotContains(errMsg, s) }
}
}
}

testUnlimitedClauses("two conditional update + two conditional delete + insert")(
source = (0, 0) :: (1, 100) :: (3, 300) :: (4, 400) :: (5, 500) :: Nil,
Expand Down Expand Up @@ -4967,7 +4981,7 @@ abstract class MergeIntoSuiteBase
(9, 903) // (9, 900) inserted by notMatched_3
))

testAnalysisErrorsInUnlimitedClauses("error on multiple insert clauses without condition")(
testErrorsInUnlimitedClauses("error on multiple insert clauses without condition")(
mergeOn = "s.key = t.key",
update(condition = "s.key == 3", set = "key = s.key, value = 2 * srcValue"),
insert(condition = null, values = "(key, value) VALUES (s.key, srcValue)"),
Expand All @@ -4976,7 +4990,7 @@ abstract class MergeIntoSuiteBase
"clauses in a merge statement, only the last not matched" ::
"clause can omit the condition" :: Nil)

testAnalysisErrorsInUnlimitedClauses("error on multiple update clauses without condition")(
testErrorsInUnlimitedClauses("error on multiple update clauses without condition")(
mergeOn = "s.key = t.key",
update(condition = "s.key == 3", set = "key = s.key, value = 2 * srcValue"),
update(condition = null, set = "key = s.key, value = 3 * srcValue"),
Expand All @@ -4985,7 +4999,7 @@ abstract class MergeIntoSuiteBase
errorStrs = "when there are more than one matched clauses in a merge statement, " +
"only the last matched clause can omit the condition" :: Nil)

testAnalysisErrorsInUnlimitedClauses("error on multiple update/delete clauses without condition")(
testErrorsInUnlimitedClauses("error on multiple update/delete clauses without condition")(
mergeOn = "s.key = t.key",
update(condition = "s.key == 3", set = "key = s.key, value = 2 * srcValue"),
delete(condition = null),
Expand All @@ -4994,7 +5008,7 @@ abstract class MergeIntoSuiteBase
errorStrs = "when there are more than one matched clauses in a merge statement, " +
"only the last matched clause can omit the condition" :: Nil)

testAnalysisErrorsInUnlimitedClauses(
testErrorsInUnlimitedClauses(
"error on non-empty condition following empty condition for update clauses")(
mergeOn = "s.key = t.key",
update(condition = null, set = "key = s.key, value = 2 * srcValue"),
Expand All @@ -5003,7 +5017,7 @@ abstract class MergeIntoSuiteBase
errorStrs = "when there are more than one matched clauses in a merge statement, " +
"only the last matched clause can omit the condition" :: Nil)

testAnalysisErrorsInUnlimitedClauses(
testErrorsInUnlimitedClauses(
"error on non-empty condition following empty condition for insert clauses")(
mergeOn = "s.key = t.key",
update(condition = null, set = "key = s.key, value = srcValue"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class CheckConstraintsSuite extends QueryTest

test("can't add unparseable constraint") {
withTestTable { table =>
val e = intercept[AnalysisException] {
val e = intercept[ParseException] {
sql(s"ALTER TABLE $table\nADD CONSTRAINT lessThan5 CHECK (id <)")
}
// Make sure we're still getting a useful parse error, even though we do some complicated
Expand Down

0 comments on commit 71e0a83

Please sign in to comment.