Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-34207][SQL] Rename isTemporaryTable to isTempView in SessionCatalog #31295

Closed

Conversation

MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Jan 22, 2021

What changes were proposed in this pull request?

Rename SessionCatalog.isTemporaryTable() to SessionCatalog.isTempView().

Why are the changes needed?

To improve code maintenance. Currently, there are two methods that do the same but have different names:

def isTempView(nameParts: Seq[String]): Boolean

and

def isTemporaryTable(name: TableIdentifier): Boolean

Does this PR introduce any user-facing change?

Should not since SessionCatalog is not public API.

How was this patch tested?

By running the existing tests:

$ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *SessionCatalogSuite"

Copy link
Member

@sunchao sunchao left a comment

Choose a reason for hiding this comment

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

+1 (non-binding)

@SparkQA
Copy link

SparkQA commented Jan 22, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38963/

@MaxGekk
Copy link
Member Author

MaxGekk commented Jan 22, 2021

@dongjoon-hyun @cloud-fan Could you review this PR, please.

@SparkQA
Copy link

SparkQA commented Jan 22, 2021

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38963/

@SparkQA
Copy link

SparkQA commented Jan 22, 2021

Test build #134376 has finished for PR 31295 at commit 8e33c19.

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

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.

@MaxGekk . Given the size of patch, we need JIRA ID for this for trace-ability. Could you file one?

@MaxGekk MaxGekk changed the title [MINOR][SQL] Rename isTemporaryTable to isTempView in SessionCatalog [SPARK-34207][SQL] Rename isTemporaryTable to isTempView in SessionCatalog Jan 22, 2021
@MaxGekk
Copy link
Member Author

MaxGekk commented Jan 22, 2021

@dongjoon-hyun I added an JIRA id to the PR title.

@dongjoon-hyun
Copy link
Member

Merged to master.
cc @cloud-fan

skestle pushed a commit to skestle/spark that referenced this pull request Feb 3, 2021
…ionCatalog`

### What changes were proposed in this pull request?
Rename `SessionCatalog.isTemporaryTable()` to `SessionCatalog.isTempView()`.

### Why are the changes needed?
To improve code maintenance. Currently, there are two methods that do the same but have different names:
```scala
def isTempView(nameParts: Seq[String]): Boolean
```
and
```scala
def isTemporaryTable(name: TableIdentifier): Boolean
```

### Does this PR introduce _any_ user-facing change?
Should not since `SessionCatalog` is not public API.

### How was this patch tested?
By running the existing tests:
```
$ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *SessionCatalogSuite"
```

Closes apache#31295 from MaxGekk/replace-isTemporaryTable-by-isTempView.

Authored-by: Max Gekk <max.gekk@gmail.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants