You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by xubo245 <gi...@git.apache.org> on 2018/01/11 08:33:28 UTC

[GitHub] spark pull request #16592: [SPARK-19235] [SQL] [TESTS] Enable Test Cases in ...

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16592#discussion_r160889706
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala ---
    @@ -1082,24 +1173,21 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
           convertToDatasourceTable(catalog, tableIdent)
         }
         assert(catalog.getTableMetadata(tableIdent).storage.locationUri.isDefined)
    -    assert(catalog.getTableMetadata(tableIdent).storage.properties.isEmpty)
    +    assert(normalizeSerdeProp(catalog.getTableMetadata(tableIdent).storage.properties).isEmpty)
         assert(catalog.getPartition(tableIdent, partSpec).storage.locationUri.isDefined)
    -    assert(catalog.getPartition(tableIdent, partSpec).storage.properties.isEmpty)
    +    assert(
    +      normalizeSerdeProp(catalog.getPartition(tableIdent, partSpec).storage.properties).isEmpty)
    +
         // Verify that the location is set to the expected string
         def verifyLocation(expected: URI, spec: Option[TablePartitionSpec] = None): Unit = {
           val storageFormat = spec
             .map { s => catalog.getPartition(tableIdent, s).storage }
             .getOrElse { catalog.getTableMetadata(tableIdent).storage }
    -      if (isDatasourceTable) {
    -        if (spec.isDefined) {
    -          assert(storageFormat.properties.isEmpty)
    -          assert(storageFormat.locationUri === Some(expected))
    -        } else {
    -          assert(storageFormat.locationUri === Some(expected))
    -        }
    -      } else {
    -        assert(storageFormat.locationUri === Some(expected))
    -      }
    +      // TODO(gatorsmile): fix the bug in alter table set location.
    +      // if (isUsingHiveMetastore) {
    +      //  assert(storageFormat.properties.get("path") === expected)
    --- End diff --
    
    Do we need to fix this bug and satify this test case?
    When porting these test cases, a bug of SET LOCATION is found. path is not set when the location is changed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org