Skip to content

Conversation

@xubo245
Copy link
Contributor

@xubo245 xubo245 commented Jan 12, 2018

What changes were proposed in this pull request?

Correct some improper with view related method usage
Only change test cases

like:

 test("list global temp views") {
    try {
      sql("CREATE GLOBAL TEMP VIEW v1 AS SELECT 3, 4")
      sql("CREATE TEMP VIEW v2 AS SELECT 1, 2")

      checkAnswer(sql(s"SHOW TABLES IN $globalTempDB"),
        Row(globalTempDB, "v1", true) ::
        Row("", "v2", true) :: Nil)

      assert(spark.catalog.listTables(globalTempDB).collect().toSeq.map(_.name) == Seq("v1", "v2"))
    } finally {
      spark.catalog.dropTempView("v1")
      spark.catalog.dropGlobalTempView("v2")
    }
  }

other change please review the code.

How was this patch tested?

See test case.

@xubo245
Copy link
Contributor Author

xubo245 commented Jan 12, 2018

@dongjoon-hyun Please review it.

@xubo245
Copy link
Contributor Author

xubo245 commented Jan 15, 2018

@dongjoon-hyun @gatorsmile Please review it. thanks

@gatorsmile
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Jan 15, 2018

Test build #86134 has finished for PR 20250 at commit 8e49445.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Contributor

@jiangxb1987 jiangxb1987 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

assert(spark.catalog.listTables(globalTempDB).collect().toSeq.map(_.name) == Seq("v1", "v2"))
} finally {
spark.catalog.dropTempView("v1")
spark.catalog.dropGlobalTempView("v2")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM.

@dongjoon-hyun
Copy link
Member

Retest this please.

@SparkQA
Copy link

SparkQA commented Jan 18, 2018

Test build #86347 has finished for PR 20250 at commit 8e49445.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@xubo245
Copy link
Contributor Author

xubo245 commented Jan 28, 2018

@gatorsmile @jiangxb1987 @dongjoon-hyun Can this PR be merged ? I will fix if it has problem

@gatorsmile
Copy link
Member

retest this please

@SparkQA
Copy link

SparkQA commented Jan 29, 2018

Test build #86748 has finished for PR 20250 at commit 8e49445.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gatorsmile
Copy link
Member

gatorsmile commented Jan 29, 2018

Thanks! Merged to master

@asfgit asfgit closed this in fbce2ed Jan 29, 2018
@xubo245
Copy link
Contributor Author

xubo245 commented Jan 30, 2018

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants