Skip to content

Commit d5f7bdf

Browse files
authored
[Spark] Make DeltaDMLTestUtils agnostic to name/path-based access (delta-io#4738)
<!-- 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://github.com/delta-io/delta/blob/master/CONTRIBUTING.md 2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP] Your PR title ...'. 3. Be sure to keep the PR description updated to reflect all changes. 4. Please write your PR title to summarize what this PR proposes. 5. If possible, provide a concise example to reproduce the issue for a faster review. 6. If applicable, include the corresponding issue number in the PR title and link it in the body. --> #### Which Delta project/connector is this regarding? <!-- Please add the component selected below to the beginning of the pull request title For example: [Spark] Title of my pull request --> - [x] Spark - [ ] Standalone - [ ] Flink - [ ] Kernel - [ ] Other (fill in here) ## Description This PR is one of a series of PRs aiming to refactor the DML suites such that we can extend the existing suites to test with name-based table access. Currently, all DML suites are centered around the `DeltaDMLTestUtils` trait that creates a test table stored in `tempPath`. In this PR, all path-related elements in this trait will be abstracted out and put into a new specialized trait called `DeltaDMLByPathTestUtils`. ## How was this patch tested? Existing UTs ## Does this PR introduce _any_ user-facing changes? No
1 parent 1cdf004 commit d5f7bdf

File tree

6 files changed

+54
-32
lines changed

6 files changed

+54
-32
lines changed

spark/src/test/scala/org/apache/spark/sql/delta/DeleteSuiteBase.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import org.apache.spark.sql.types.StructType
2727

2828
abstract class DeleteSuiteBase extends QueryTest
2929
with SharedSparkSession
30-
with DeltaDMLTestUtils
30+
with DeltaDMLByPathTestUtils
3131
with DeltaTestUtilsForTempViews
3232
with DeltaExcludedBySparkVersionTestMixinShims {
3333

spark/src/test/scala/org/apache/spark/sql/delta/DeltaInsertIntoTest.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ import org.apache.spark.sql.types.StructType
3737
* Each take a unique path through analysis. The abstractions below captures these different
3838
* inserts to allow more easily running tests with all or a subset of them.
3939
*/
40-
trait DeltaInsertIntoTest extends QueryTest with DeltaDMLTestUtils with DeltaSQLCommandTest {
40+
trait DeltaInsertIntoTest
41+
extends QueryTest
42+
with DeltaDMLByPathTestUtils
43+
with DeltaSQLCommandTest {
4144

4245
val catalogName = "spark_catalog"
4346

spark/src/test/scala/org/apache/spark/sql/delta/DeltaTestUtils.scala

Lines changed: 44 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -568,34 +568,25 @@ trait DeltaDMLTestUtils
568568

569569
import testImplicits._
570570

571-
protected var tempDir: File = _
572-
573571
protected var deltaLog: DeltaLog = _
574572

575-
protected def tempPath: String = tempDir.getCanonicalPath
576-
577-
override protected def beforeEach(): Unit = {
578-
super.beforeEach()
579-
// Using a space in path to provide coverage for special characters.
580-
tempDir = Utils.createTempDir(namePrefix = "spark test")
581-
deltaLog = DeltaLog.forTable(spark, new Path(tempPath))
582-
}
583-
584-
override protected def afterEach(): Unit = {
585-
try {
586-
Utils.deleteRecursively(tempDir)
587-
DeltaLog.clearCache()
588-
} finally {
589-
super.afterEach()
590-
}
591-
}
573+
protected def tableSQLIdentifier: String
592574

593575
protected def append(df: DataFrame, partitionBy: Seq[String] = Nil): Unit = {
576+
import DeltaTestUtils.TableIdentifierOrPath
577+
594578
val dfw = df.write.format("delta").mode("append")
595579
if (partitionBy.nonEmpty) {
596580
dfw.partitionBy(partitionBy: _*)
597581
}
598-
dfw.save(tempPath)
582+
getTableIdentifierOrPath(tableSQLIdentifier) match {
583+
case TableIdentifierOrPath.Identifier(id, _) => dfw.saveAsTable(id.toString)
584+
// A cleaner way to write this is to just use `saveAsTable` where the
585+
// table name is delta.`path`. However, it will throw an error when
586+
// we use "append" mode and the table does not exist, so we use `save`
587+
// here instead.
588+
case TableIdentifierOrPath.Path(path, _) => dfw.save(path)
589+
}
599590
}
600591

601592
protected def withKeyValueData(
@@ -612,7 +603,7 @@ trait DeltaDMLTestUtils
612603
if (isKeyPartitioned) Seq(targetKeyValueNames._1) else Nil)
613604
withTempView("source") {
614605
source.toDF(sourceKeyValueNames._1, sourceKeyValueNames._2).createOrReplaceTempView("source")
615-
thunk("source", s"delta.`$tempPath`")
606+
thunk("source", tableSQLIdentifier)
616607
}
617608
}
618609

@@ -633,12 +624,6 @@ trait DeltaDMLTestUtils
633624
}
634625
}
635626

636-
protected def readDeltaTable(path: String): DataFrame = {
637-
spark.read.format("delta").load(path)
638-
}
639-
640-
protected def getDeltaFileStmt(path: String): String = s"SELECT * FROM delta.`$path`"
641-
642627
/**
643628
* Finds the latest operation of the given type that ran on the test table and returns the
644629
* dataframe with the changes of the corresponding table version.
@@ -665,3 +650,35 @@ trait DeltaDMLTestUtils
665650
.drop(CDCReader.CDC_COMMIT_VERSION)
666651
}
667652
}
653+
654+
trait DeltaDMLByPathTestUtils extends DeltaDMLTestUtils {
655+
self: SharedSparkSession =>
656+
657+
protected var tempDir: File = _
658+
659+
protected def tempPath: String = tempDir.getCanonicalPath
660+
661+
override protected def beforeEach(): Unit = {
662+
super.beforeEach()
663+
// Using a space in path to provide coverage for special characters.
664+
tempDir = Utils.createTempDir(namePrefix = "spark test")
665+
deltaLog = DeltaLog.forTable(spark, tempPath)
666+
}
667+
668+
override protected def afterEach(): Unit = {
669+
try {
670+
Utils.deleteRecursively(tempDir)
671+
DeltaLog.clearCache()
672+
} finally {
673+
super.afterEach()
674+
}
675+
}
676+
677+
override protected def tableSQLIdentifier: String = s"delta.`$tempPath`"
678+
679+
protected def readDeltaTable(path: String): DataFrame = {
680+
spark.read.format("delta").load(path)
681+
}
682+
683+
protected def getDeltaFileStmt(path: String): String = s"SELECT * FROM delta.`$path`"
684+
}

spark/src/test/scala/org/apache/spark/sql/delta/MergeIntoTestUtils.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import org.apache.spark.sql.test.SharedSparkSession
2828
* either [[MergeIntoSQLTestUtils]] or [[MergeIntoScalaTestUtils]] to run merge tests using the SQL
2929
* or Scala API resp.
3030
*/
31-
trait MergeIntoTestUtils extends DeltaDMLTestUtils with MergeHelpers {
31+
trait MergeIntoTestUtils extends DeltaDMLByPathTestUtils with MergeHelpers {
3232
self: SharedSparkSession =>
3333

3434
protected def executeMerge(

spark/src/test/scala/org/apache/spark/sql/delta/UpdateSuiteBase.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import org.apache.spark.sql.types._
3636
abstract class UpdateSuiteBase
3737
extends QueryTest
3838
with SharedSparkSession
39-
with DeltaDMLTestUtils
39+
with DeltaDMLByPathTestUtils
4040
with DeltaSQLTestUtils
4141
with DeltaTestUtilsForTempViews
4242
with DeltaExcludedBySparkVersionTestMixinShims {

spark/src/test/scala/org/apache/spark/sql/delta/typewidening/TypeWideningTestMixin.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ import org.apache.spark.sql.types._
3535
/**
3636
* Test mixin that enables type widening by default for all tests in the suite.
3737
*/
38-
trait TypeWideningTestMixin extends DeltaSQLCommandTest with DeltaDMLTestUtils { self: QueryTest =>
38+
trait TypeWideningTestMixin
39+
extends DeltaSQLCommandTest
40+
with DeltaDMLByPathTestUtils { self: QueryTest =>
3941

4042
import testImplicits._
4143

0 commit comments

Comments
 (0)