[GitHub] spark pull request #23194: [MINOR][SQL] Combine the same codes in test cases
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/23194 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23194: [MINOR][SQL] Combine the same codes in test cases
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/23194#discussion_r238526892
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
---
@@ -377,41 +377,41 @@ abstract class DDLSuite extends QueryTest with
SQLTestUtils {
}
}
- test("CTAS a managed table with the existing empty directory") {
-val tableLoc = new
File(spark.sessionState.catalog.defaultTablePath(TableIdentifier("tab1")))
+ protected def withEmptyDirInTablePath(dirName: String)(f: File => Unit):
Unit = {
--- End diff --
private?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23194: [MINOR][SQL] Combine the same codes in test cases
Github user CarolinePeng commented on a diff in the pull request:
https://github.com/apache/spark/pull/23194#discussion_r238523807
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
---
@@ -377,41 +377,42 @@ abstract class DDLSuite extends QueryTest with
SQLTestUtils {
}
}
- test("CTAS a managed table with the existing empty directory") {
-val tableLoc = new
File(spark.sessionState.catalog.defaultTablePath(TableIdentifier("tab1")))
-try {
+ protected def withTableLocation(tableNames: String)(f: File => Unit):
Unit = {
--- End diff --
Ok,thank you !
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23194: [MINOR][SQL] Combine the same codes in test cases
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/23194#discussion_r238256775
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
---
@@ -439,31 +440,22 @@ abstract class DDLSuite extends QueryTest with
SQLTestUtils {
}.getMessage
assert(ex.contains(exMsgWithDefaultDB))
}
- } finally {
-waitForTasksToFinish()
-Utils.deleteRecursively(tableLoc)
}
}
}
test("rename a managed table with existing empty directory") {
-val tableLoc = new
File(spark.sessionState.catalog.defaultTablePath(TableIdentifier("tab2")))
-try {
+withTableLocation("tab2") { tableLoc =>
withTable("tab1") {
sql(s"CREATE TABLE tab1 USING $dataSource AS SELECT 1, 'a'")
-tableLoc.mkdir()
val ex = intercept[AnalysisException] {
sql("ALTER TABLE tab1 RENAME TO tab2")
}.getMessage
val expectedMsg = "Can not rename the managed table('`tab1`'). The
associated location"
assert(ex.contains(expectedMsg))
}
-} finally {
- waitForTasksToFinish()
- Utils.deleteRecursively(tableLoc)
}
}
-
--- End diff --
nit: revert this
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23194: [MINOR][SQL] Combine the same codes in test cases
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/23194#discussion_r238256611
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
---
@@ -377,41 +377,42 @@ abstract class DDLSuite extends QueryTest with
SQLTestUtils {
}
}
- test("CTAS a managed table with the existing empty directory") {
-val tableLoc = new
File(spark.sessionState.catalog.defaultTablePath(TableIdentifier("tab1")))
-try {
+ protected def withTableLocation(tableNames: String)(f: File => Unit):
Unit = {
+val tableLoc =
+ new
File(spark.sessionState.catalog.defaultTablePath(TableIdentifier(tableNames)))
+try
--- End diff --
nit `try {`
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23194: [MINOR][SQL] Combine the same codes in test cases
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/23194#discussion_r238256563
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
---
@@ -377,41 +377,42 @@ abstract class DDLSuite extends QueryTest with
SQLTestUtils {
}
}
- test("CTAS a managed table with the existing empty directory") {
-val tableLoc = new
File(spark.sessionState.catalog.defaultTablePath(TableIdentifier("tab1")))
-try {
+ protected def withTableLocation(tableNames: String)(f: File => Unit):
Unit = {
--- End diff --
How about `withEmptyDirInTablePath(dirName: String)`?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23194: [MINOR][SQL] Combine the same codes in test cases
Github user CarolinePeng commented on a diff in the pull request: https://github.com/apache/spark/pull/23194#discussion_r238063442 --- Diff: core/src/main/scala/org/apache/spark/memory/ExecutionMemoryPool.scala --- @@ -37,7 +37,7 @@ import org.apache.spark.internal.Logging * tasks was performed by the ShuffleMemoryManager. * * @param lock a [[MemoryManager]] instance to synchronize on - * @param memoryMode the type of memory tracked by this pool (on- or off-heap) + * @param memoryMode the type of memory tracked by this pool (on-heap or off-heap) --- End diff -- No, I think it can be modified here,so I changed it by the way. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23194: [MINOR][SQL] Combine the same codes in test cases
Github user kiszk commented on a diff in the pull request: https://github.com/apache/spark/pull/23194#discussion_r238062387 --- Diff: core/src/main/scala/org/apache/spark/memory/ExecutionMemoryPool.scala --- @@ -37,7 +37,7 @@ import org.apache.spark.internal.Logging * tasks was performed by the ShuffleMemoryManager. * * @param lock a [[MemoryManager]] instance to synchronize on - * @param memoryMode the type of memory tracked by this pool (on- or off-heap) + * @param memoryMode the type of memory tracked by this pool (on-heap or off-heap) --- End diff -- Is this change related to this PR? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23194: [MINOR][SQL] Combine the same codes in test cases
Github user kiszk commented on a diff in the pull request: https://github.com/apache/spark/pull/23194#discussion_r238062396 --- Diff: core/src/main/scala/org/apache/spark/memory/StorageMemoryPool.scala --- @@ -28,7 +28,7 @@ import org.apache.spark.storage.memory.MemoryStore * (caching). * * @param lock a [[MemoryManager]] instance to synchronize on - * @param memoryMode the type of memory tracked by this pool (on- or off-heap) + * @param memoryMode the type of memory tracked by this pool (on-heap or off-heap) --- End diff -- ditto --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23194: [MINOR][SQL] Combine the same codes in test cases
GitHub user CarolinePeng opened a pull request: https://github.com/apache/spark/pull/23194 [MINOR][SQL] Combine the same codes in test cases ## What changes were proposed in this pull request? In the DDLSuit, there are four test cases have the same codes , writing a function can combine the same code. ## How was this patch tested? existing tests. You can merge this pull request into a Git repository by running: $ git pull https://github.com/CarolinePeng/spark Update_temp Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/23194.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #23194 commit eff036c031fe96e9c609b30c882be3ac3bd64e86 Author: å½ç¿00244106 <00244106@...> Date: 2018-12-01T07:51:39Z update some codes --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
