Skip to content

Commit a6c161d

Browse files
karenfengscottsand-db
authored andcommitted
Change default behavior of DROP CONSTRAINT to differ from IF EXISTS
Changes the behavior of DROP CONSTRAINT to throw an error by default if the constraint does not exist. The error will not be thrown if the user provides the argument IF EXISTS. Unit test GitOrigin-RevId: 969e7d41899ed911f596ee2c9221e6c73e2d444d
1 parent b664c7a commit a6c161d

File tree

9 files changed

+62
-10
lines changed

9 files changed

+62
-10
lines changed

core/src/main/resources/error/delta-error-classes.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@
2727
"message" : [ "There is a conflict from these SET columns: %s." ],
2828
"sqlState" : "42000"
2929
},
30+
"CONSTRAINT_DOES_NOT_EXIST" : {
31+
"message" : [ "Cannot drop nonexistent constraint %s from table %s. To avoid throwing an error, provide the parameter IF EXISTS or set the SQL session configuration %s to %s." ],
32+
"sqlState" : "42000"
33+
},
3034
"DELTA_CANNOT_CHANGE_DATA_TYPE" : {
3135
"message" : [ "Cannot change data type: %s" ],
3236
"sqlState" : "22000"

core/src/main/scala/io/delta/sql/parser/DeltaSqlParser.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,8 @@ class DeltaSqlAstBuilder extends DeltaSqlBaseBaseVisitor[AnyRef] {
297297
AlterTableDropConstraint(
298298
createUnresolvedTable(ctx.table.identifier.asScala.map(_.getText).toSeq,
299299
"ALTER TABLE ... DROP CONSTRAINT"),
300-
ctx.name.getText)
300+
ctx.name.getText,
301+
ifExists = ctx.EXISTS != null)
301302
}
302303

303304
protected def typedVisit[T](ctx: ParseTree): T = {

core/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/deltaConstraints.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ case class AlterTableAddConstraint(
3434
* The logical plan of the ALTER TABLE ... DROP CONSTRAINT command.
3535
*/
3636
case class AlterTableDropConstraint(
37-
table: LogicalPlan, constraintName: String) extends AlterTableCommand {
38-
override def changes: Seq[TableChange] = Seq(DropConstraint(constraintName))
37+
table: LogicalPlan, constraintName: String, ifExists: Boolean) extends AlterTableCommand {
38+
override def changes: Seq[TableChange] = Seq(DropConstraint(constraintName, ifExists))
3939

4040
protected def withNewChildInternal(newChild: LogicalPlan): LogicalPlan = copy(table = newChild)
4141
}

core/src/main/scala/org/apache/spark/sql/delta/DeltaErrors.scala

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,16 @@ object DeltaErrors
210210
new AnalysisException(s"Cannot use '$name' as the name of a CHECK constraint.")
211211
}
212212

213+
def nonexistentConstraint(constraintName: String, tableName: String): AnalysisException = {
214+
new DeltaAnalysisException(
215+
errorClass = "CONSTRAINT_DOES_NOT_EXIST",
216+
messageParameters = Array(
217+
constraintName,
218+
tableName,
219+
DeltaSQLConf.DELTA_ASSUMES_DROP_CONSTRAINT_IF_EXISTS.key,
220+
"true"))
221+
}
222+
213223
def checkConstraintNotBoolean(name: String, expr: String): AnalysisException = {
214224
new AnalysisException(s"CHECK constraint '$name' ($expr) should be a boolean expression.'")
215225
}

core/src/main/scala/org/apache/spark/sql/delta/catalog/DeltaCatalog.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ class DeltaCatalog extends DelegatingCatalogExtension
544544
case (t, constraints) if t == classOf[DropConstraint] =>
545545
constraints.foreach { constraint =>
546546
val c = constraint.asInstanceOf[DropConstraint]
547-
AlterTableDropConstraintDeltaCommand(table, c.constraintName).run(spark)
547+
AlterTableDropConstraintDeltaCommand(table, c.constraintName, c.ifExists).run(spark)
548548
}
549549
}
550550

core/src/main/scala/org/apache/spark/sql/delta/commands/alterDeltaTableCommands.scala

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,8 @@ case class AlterTableAddConstraintDeltaCommand(
629629
*/
630630
case class AlterTableDropConstraintDeltaCommand(
631631
table: DeltaTableV2,
632-
name: String)
632+
name: String,
633+
ifExists: Boolean)
633634
extends LeafRunnableCommand with AlterDeltaTableCommand with IgnoreCachedData {
634635

635636
override def run(sparkSession: SparkSession): Seq[Row] = {
@@ -638,6 +639,14 @@ case class AlterTableDropConstraintDeltaCommand(
638639
val txn = startTransaction()
639640

640641
val oldExprText = Constraints.getExprTextByName(name, txn.metadata, sparkSession)
642+
if (oldExprText.isEmpty && !ifExists && !sparkSession.sessionState.conf.getConf(
643+
DeltaSQLConf.DELTA_ASSUMES_DROP_CONSTRAINT_IF_EXISTS)) {
644+
val quotedTableName = table.getTableIdentifierIfExists.map(_.quotedString)
645+
.orElse(table.catalogTable.map(_.identifier.quotedString))
646+
.getOrElse(table.name())
647+
throw DeltaErrors.nonexistentConstraint(name, quotedTableName)
648+
}
649+
641650
val newMetadata = txn.metadata.copy(
642651
configuration = txn.metadata.configuration - Constraints.checkConstraintPropertyName(name))
643652

core/src/main/scala/org/apache/spark/sql/delta/constraints/tableChanges.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,6 @@ case class AddConstraint(constraintName: String, expr: String) extends TableChan
3232
* will be thrown if the constraint doesn't exist.
3333
*
3434
* @param constraintName the name of the constraint to drop - case insensitive
35+
* @param ifExists if false, throws an error if the constraint to be dropped does not exist
3536
*/
36-
case class DropConstraint(constraintName: String) extends TableChange {}
37+
case class DropConstraint(constraintName: String, ifExists: Boolean) extends TableChange {}

core/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConf.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,14 @@ trait DeltaSQLConfBase {
284284
.booleanConf
285285
.createWithDefault(false)
286286

287+
val DELTA_ASSUMES_DROP_CONSTRAINT_IF_EXISTS =
288+
buildConf("constraints.assumesDropIfExists.enabled")
289+
.doc("""If true, DROP CONSTRAINT quietly drops nonexistent constraints even without
290+
|IF EXISTS.
291+
""")
292+
.booleanConf
293+
.createWithDefault(false)
294+
287295
val DELTA_STATE_CORRUPTION_IS_FATAL =
288296
buildConf("state.corruptionIsFatal")
289297
.internal()

core/src/test/scala/org/apache/spark/sql/delta/schema/CheckConstraintsSuite.scala

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,18 +136,37 @@ class CheckConstraintsSuite extends QueryTest
136136
}
137137
}
138138

139-
test("can drop constraint that doesn't exist") {
139+
test("drop constraint that doesn't exist throws an exception") {
140140
withTestTable { table =>
141-
sql(s"ALTER TABLE $table DROP CONSTRAINT IF EXISTS myConstraint")
141+
intercept[AnalysisException] {
142+
sql(s"ALTER TABLE $table DROP CONSTRAINT myConstraint")
143+
}
144+
}
145+
146+
withSQLConf((DeltaSQLConf.DELTA_ASSUMES_DROP_CONSTRAINT_IF_EXISTS.key, "false")) {
147+
withTestTable { table =>
148+
val e = intercept[AnalysisException] {
149+
sql(s"ALTER TABLE $table DROP CONSTRAINT myConstraint")
150+
}
151+
assert(e.getErrorClass == "CONSTRAINT_DOES_NOT_EXIST")
152+
errorContains(e.getMessage,
153+
"nonexistent constraint myconstraint from table `default`.`checkconstraintstest`")
154+
errorContains(e.getMessage,
155+
"databricks.spark.delta.constraints.assumesDropIfExists.enabled to true")
156+
}
142157
}
143158
}
144159

145-
// IF EXISTS is provided only for parallelism with existing DataSourceV2 commands that support it
146-
// as a stub. It doesn't change any behavior.
147160
test("can drop constraint that doesn't exist with IF EXISTS") {
148161
withTestTable { table =>
149162
sql(s"ALTER TABLE $table DROP CONSTRAINT IF EXISTS myConstraint")
150163
}
164+
165+
withSQLConf((DeltaSQLConf.DELTA_ASSUMES_DROP_CONSTRAINT_IF_EXISTS.key, "true")) {
166+
withTestTable { table =>
167+
sql(s"ALTER TABLE $table DROP CONSTRAINT myConstraint")
168+
}
169+
}
151170
}
152171

153172

0 commit comments

Comments
 (0)