Skip to content

Conversation

@zachschuermann
Copy link
Contributor

@zachschuermann zachschuermann commented Jun 23, 2022

What changes were proposed in this pull request?

Change setCurrentDatabase catalog API to support 3 layer namespace. We use sparkSession.sessionState.catalogManager.currentNamespace for the currentDatabase now.

Why are the changes needed?

setCurrentDatabase doesn't support 3 layer namespace.

Does this PR introduce any user-facing change?

Yes. This PR introduces a backwards-compatible API change to support 3 layer namespace (e.g. catalog.database.table) for setCurrentDatabase.

How was this patch tested?

UT

@github-actions github-actions bot added the SQL label Jun 23, 2022
@HyukjinKwon HyukjinKwon changed the title [SPARK-39235][SQL] make setCurrentDatabase compatible with 3 layer namespace [SPARK-39235][SQL] Make setCurrentDatabase compatible with 3 layer namespace Jun 24, 2022
@HyukjinKwon
Copy link
Member

cc @zhengruifeng FYI

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@zachschuermann zachschuermann changed the title [SPARK-39235][SQL] Make setCurrentDatabase compatible with 3 layer namespace [SPARK-39646][SQL] Make setCurrentDatabase compatible with 3 layer namespace Jun 30, 2022
@amaliujia
Copy link
Contributor

R: @cloud-fan

@github-actions github-actions bot added the R label Jun 30, 2022
@zhengruifeng
Copy link
Contributor

@schuermannator there are some conflicts to reslove.

@zachschuermann zachschuermann force-pushed the 3l-setCurrentDatabse branch from c3fcc66 to 8a58c03 Compare July 1, 2022 03:06
@zhengruifeng
Copy link
Contributor

SparkR failed:

══ Failed ══════════════════════════════════════════════════════════════════════
── 1. Failure (test_sparkSQL.R:4017:3): catalog APIs, currentDatabase, setCurren
`setCurrentDatabase("zxwtyswklpf")` threw an error with unexpected message.
Expected match: "Error in setCurrentDatabase : analysis error - Namespace 'zxwtyswklpf' does not exist"
Actual message: "Error in setCurrentDatabase : no such database - Database 'zxwtyswklpf' not found"

@zachschuermann
Copy link
Contributor Author

good to go now!

Copy link
Contributor

@amaliujia amaliujia left a comment

Choose a reason for hiding this comment

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

LGTM!

@zhengruifeng
Copy link
Contributor

Merged to master!

pan3793 pushed a commit to apache/kyuubi that referenced this pull request Jul 4, 2022
### _Why are the changes needed?_

To make the behavior consistent.

The master GA failed due to apache/spark#36969
<img width="1137" alt="image" src="https://user-images.githubusercontent.com/12025282/177069041-b0a102ca-51bf-4faf-b89d-53bfd950cbc2.png">

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [x] Add screenshots for manual tests if appropriate

- [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #2998 from ulysses-you/setCurrentNamespace.

Closes #2997

2646d4f [ulysses-you] style
79a9f33 [ulysses-you] simplify
246ed71 [ulysses-you] v2 catalog

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants