Skip to content

Conversation

@clockfly
Copy link
Contributor

@clockfly clockfly commented May 6, 2016

What changes were proposed in this pull request?

Deprecates registerTempTable and add dataset.createTempView, dataset.createOrReplaceTempView.

How was this patch tested?

Unit tests.

@clockfly clockfly changed the title [SPARK-15171][SQL]Deprecate registerTempTable and add dataset.createTempView [SPARK-15171][SQL][WIP]Deprecate registerTempTable and add dataset.createTempView May 6, 2016
@SparkQA
Copy link

SparkQA commented May 6, 2016

Test build #57947 has finished for PR 12945 at commit e4675b0.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 6, 2016

Test build #57949 has finished for PR 12945 at commit 8612976.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 6, 2016

Test build #57950 has finished for PR 12945 at commit ac4e502.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 6, 2016

Test build #57953 has finished for PR 12945 at commit 0bcbe8c.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 6, 2016

Test build #57956 has finished for PR 12945 at commit 029ce73.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 6, 2016

Test build #57964 has finished for PR 12945 at commit e17bd6d.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@clockfly
Copy link
Contributor Author

clockfly commented May 6, 2016

There are two places I am not very sure:

  1. Should we also deprecate the python and R api registerTempTable?
  2. Which we allow replacing table or not for API createTempView(table)? Now it calls createTempView(table, replaceIfExists = false), should we change it to createTempView(table, replaceIfExists = true)?

@SparkQA
Copy link

SparkQA commented May 6, 2016

Test build #57986 has finished for PR 12945 at commit 5bd77e9.

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

@liancheng
Copy link
Contributor

For question 1, I think we should also deprecate corresponding API in PySpark and SparkR. For question 2, I don't get it...

@liancheng
Copy link
Contributor

I think we should also deprecate Catalog.dropTempTable and SQLContext.dropTempTable by Catalog.dropTempView and SQLContext.dropTempView.

@yhuai
Copy link
Contributor

yhuai commented May 9, 2016

If possible, can you have a pr to just add the new API and deprecate the old API?

Copy link
Contributor

Choose a reason for hiding this comment

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

SQLContext -> SparkSession

@clockfly clockfly force-pushed the spark-15171 branch 2 times, most recently from d53ca63 to 59654e6 Compare May 10, 2016 06:56
@SparkQA
Copy link

SparkQA commented May 10, 2016

Test build #58217 has finished for PR 12945 at commit 59654e6.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 10, 2016

Test build #58219 has finished for PR 12945 at commit d372575.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 10, 2016

Test build #58213 has finished for PR 12945 at commit 3f834a4.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 10, 2016

Test build #58214 has finished for PR 12945 at commit d53ca63.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 10, 2016

Test build #58223 has finished for PR 12945 at commit 2c7b5f5.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 10, 2016

Test build #58226 has finished for PR 12945 at commit cc74aaa.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.


return DataFrame(jdf, self._wrapped)

def _createTempView(self, viewName, dataFrame, replaceIfExists):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we add this method?

@liancheng
Copy link
Contributor

The PR description is inconsistent with changes made in the PR now. Please update.

@SparkQA
Copy link

SparkQA commented May 12, 2016

Test build #58449 has finished for PR 12945 at commit a9bdd5b.

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

noException should be thrownBy {
// Overrrides the existing temporary view with same name
dataset.createOrReplaceTempView("tempView")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

In general, we try to avoid using fancy ScalaTest assertion facilities like this one. In this case, a simple call to createOrReplaceTempView and a comment saying that no exception should be thrown is enough.

* @group basic
* @since 1.6.0
*/
@deprecated("Use createOrReplaceTempView(tableName) instead.", "2.0.0")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: tableName => viewName

@liancheng
Copy link
Contributor

LGTM except for one minor comment for the deprecation warning message.

@liancheng
Copy link
Contributor

OK, I'm merging this to master and branch-2.0 since the last commit only changes deprecation message.

@asfgit asfgit closed this in 33c6eb5 May 12, 2016
asfgit pushed a commit that referenced this pull request May 12, 2016
…TempView

## What changes were proposed in this pull request?

Deprecates registerTempTable and add dataset.createTempView, dataset.createOrReplaceTempView.

## How was this patch tested?

Unit tests.

Author: Sean Zhong <[email protected]>

Closes #12945 from clockfly/spark-15171.

(cherry picked from commit 33c6eb5)
Signed-off-by: Cheng Lian <[email protected]>
@SparkQA
Copy link

SparkQA commented May 12, 2016

Test build #58456 has finished for PR 12945 at commit 977e01d.

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

@SparkQA
Copy link

SparkQA commented May 12, 2016

Test build #58459 has finished for PR 12945 at commit b024e99.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 12, 2016

Test build #58457 has finished for PR 12945 at commit dd52802.

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

asfgit pushed a commit that referenced this pull request May 18, 2016
….registerTempTable

## What changes were proposed in this pull request?

Update the unit test code, examples, and documents to remove calls to deprecated method `dataset.registerTempTable`.

## How was this patch tested?

This PR only changes the unit test code, examples, and comments. It should be safe.
This is a follow up of PR #12945 which was merged.

Author: Sean Zhong <[email protected]>

Closes #13098 from clockfly/spark-15171-remove-deprecation.
asfgit pushed a commit that referenced this pull request May 18, 2016
….registerTempTable

## What changes were proposed in this pull request?

Update the unit test code, examples, and documents to remove calls to deprecated method `dataset.registerTempTable`.

## How was this patch tested?

This PR only changes the unit test code, examples, and comments. It should be safe.
This is a follow up of PR #12945 which was merged.

Author: Sean Zhong <[email protected]>

Closes #13098 from clockfly/spark-15171-remove-deprecation.

(cherry picked from commit 25b315e)
Signed-off-by: Cheng Lian <[email protected]>
asfgit pushed a commit that referenced this pull request Jun 13, 2016
…placeTempView

## What changes were proposed in this pull request?

This PR replaces `registerTempTable` with `createOrReplaceTempView` as a follow-up task of #12945.

## How was this patch tested?

Existing SparkR tests.

Author: Cheng Lian <[email protected]>

Closes #13644 from liancheng/spark-15925-temp-view-for-r.
asfgit pushed a commit that referenced this pull request Jun 13, 2016
…placeTempView

## What changes were proposed in this pull request?

This PR replaces `registerTempTable` with `createOrReplaceTempView` as a follow-up task of #12945.

## How was this patch tested?

Existing SparkR tests.

Author: Cheng Lian <[email protected]>

Closes #13644 from liancheng/spark-15925-temp-view-for-r.

(cherry picked from commit ced8d66)
Signed-off-by: Shivaram Venkataraman <[email protected]>
asfgit pushed a commit that referenced this pull request Jul 23, 2016
## What changes were proposed in this pull request?

after #12945, we renamed the `registerTempTable` to `createTempView`, as we do create a view actually. This PR renames `SQLTestUtils.withTempTable` to reflect this change.

## How was this patch tested?

N/A

Author: Wenchen Fan <[email protected]>

Closes #14318 from cloud-fan/minor4.
asfgit pushed a commit that referenced this pull request Jul 23, 2016
after #12945, we renamed the `registerTempTable` to `createTempView`, as we do create a view actually. This PR renames `SQLTestUtils.withTempTable` to reflect this change.

N/A

Author: Wenchen Fan <[email protected]>

Closes #14318 from cloud-fan/minor4.

(cherry picked from commit 86c2752)
Signed-off-by: Reynold Xin <[email protected]>
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.

6 participants