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.

Sign up for GitHub

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

Jump to bottom

[Bug][TRINO] Trino engine use the engine level conf as session level conf #6147

Closed
3 of 4 tasks
zwangsheng opened this issue Mar 8, 2024 · 0 comments
Closed
3 of 4 tasks

[Bug][TRINO] Trino engine use the engine level conf as session level conf #6147

zwangsheng opened this issue Mar 8, 2024 · 0 comments
Labels
kind:bug This is a clearly a bug priority:major

Comments

@zwangsheng
Copy link
Contributor

Code of Conduct

Search before asking

  • I have searched in the issues and found no similar issues.

Describe the bug

In TrinoSessionImpl, it's wrong consider sessionManager.getConf as session level conf. Actually, it's engine level conf.

kyuubi/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionImpl.scala

Line 52 in 6fc7552

val sessionConf: KyuubiConf = sessionManager.getConf

We should use the passed arg conf as session level conf.

kyuubi/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionImpl.scala

Lines 43 to 50 in 6fc7552

class TrinoSessionImpl(
protocol: TProtocolVersion,
user: String,
password: String,
ipAddress: String,
conf: Map[String, String],
sessionManager: SessionManager)
extends AbstractSession(protocol, user, password, ipAddress, conf, sessionManager) {

Affects Version(s)

master

Kyuubi Server Log Output

No response

Kyuubi Engine Log Output

No response

Kyuubi Server Configurations

No response

Kyuubi Engine Configurations

No response

Additional context

No response

Are you willing to submit PR?

  • Yes. I would be willing to submit a PR with guidance from the Kyuubi community to fix.
  • No. I cannot submit a PR at this time.
@zwangsheng zwangsheng added kind:bug This is a clearly a bug priority:major labels Mar 8, 2024
zwangsheng added a commit to zwangsheng/kyuubi that referenced this issue Mar 8, 2024
@zwangsheng
[KYUUBI apache#6147][TRINO] Using the overlay conf as session conf
4ee7c04
@zwangsheng zwangsheng mentioned this issue Mar 8, 2024
Closed
4 tasks
@pan3793 pan3793 closed this as completed in f5b89b0 Mar 8, 2024
pan3793 added a commit that referenced this issue Mar 8, 2024
@zwangsheng @pan3793
[KYUUBI #6147][TRINO] Use the overlay conf as session conf
0b53703 
# 🔍 Description
## Issue References 🔗

This pull request fixes #6147

## Describe Your Solution 🔧

Overwriting the engine conf with the passed conf consider as the full session conf.

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests
None

---

# Checklist 📝

- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes #6148 from zwangsheng/KYUUBI#6147.

Closes #6147

3e0fb0f [Cheng Pan] Update externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionImpl.scala
452224e [zwangsheng] fix comments
4ee7c04 [zwangsheng] [KYUUBI #6147][TRINO] Using the overlay conf as session conf

Lead-authored-by: zwangsheng <binjieyang@apache.org>
Co-authored-by: Cheng Pan <pan3793@gmail.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
(cherry picked from commit f5b89b0)
Signed-off-by: Cheng Pan <chengpan@apache.org>
zhouyifan279 pushed a commit to zhouyifan279/kyuubi that referenced this issue Mar 11, 2024
@zwangsheng @pan3793 @zhouyifan279
[KYUUBI apache#6147][TRINO] Use the overlay conf as session conf
7037b72 
# 🔍 Description
## Issue References 🔗

This pull request fixes apache#6147

## Describe Your Solution 🔧

Overwriting the engine conf with the passed conf consider as the full session conf.

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests
None

---

# Checklist 📝

- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes apache#6148 from zwangsheng/KYUUBI#6147.

Closes apache#6147

3e0fb0f [Cheng Pan] Update externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionImpl.scala
452224e [zwangsheng] fix comments
4ee7c04 [zwangsheng] [KYUUBI apache#6147][TRINO] Using the overlay conf as session conf

Lead-authored-by: zwangsheng <binjieyang@apache.org>
Co-authored-by: Cheng Pan <pan3793@gmail.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
@lsm1 lsm1 mentioned this issue Mar 14, 2024
Closed
4 tasks
pan3793 pushed a commit that referenced this issue Mar 19, 2024
@lsm1 @pan3793
[KYUUBI #6182] JDBC engine using the overlay conf as session conf
f4f7956 
# 🔍 Description
## Issue References 🔗

## Describe Your Solution 🔧

Like #6147, JDBC engine should overwriting the engine conf with the passed conf consider as the full session conf.

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklist 📝

- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes #6182 from lsm1/branch-jdbc-engine-use-overlay-conf.

Closes #6182

9ca8b48 [senmiaoliu] Using the overlay conf as session conf

Authored-by: senmiaoliu <senmiaoliu@trip.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
pan3793 pushed a commit that referenced this issue Mar 19, 2024
@lsm1 @pan3793
[KYUUBI #6182] JDBC engine using the overlay conf as session conf
250438d 
# 🔍 Description
## Issue References 🔗

## Describe Your Solution 🔧

Like #6147, JDBC engine should overwriting the engine conf with the passed conf consider as the full session conf.

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklist 📝

- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes #6182 from lsm1/branch-jdbc-engine-use-overlay-conf.

Closes #6182

9ca8b48 [senmiaoliu] Using the overlay conf as session conf

Authored-by: senmiaoliu <senmiaoliu@trip.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
(cherry picked from commit f4f7956)
Signed-off-by: Cheng Pan <chengpan@apache.org>
pan3793 pushed a commit that referenced this issue Mar 19, 2024
@lsm1 @pan3793
[KYUUBI #6182] JDBC engine using the overlay conf as session conf
ea25bcc 
# 🔍 Description
## Issue References 🔗

## Describe Your Solution 🔧

Like #6147, JDBC engine should overwriting the engine conf with the passed conf consider as the full session conf.

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklist 📝

- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes #6182 from lsm1/branch-jdbc-engine-use-overlay-conf.

Closes #6182

9ca8b48 [senmiaoliu] Using the overlay conf as session conf

Authored-by: senmiaoliu <senmiaoliu@trip.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
(cherry picked from commit f4f7956)
Signed-off-by: Cheng Pan <chengpan@apache.org>
pan3793 pushed a commit that referenced this issue Mar 19, 2024
@lsm1 @pan3793
[KYUUBI #6182] JDBC engine using the overlay conf as session conf
39d565c 
# 🔍 Description
## Issue References 🔗

## Describe Your Solution 🔧

Like #6147, JDBC engine should overwriting the engine conf with the passed conf consider as the full session conf.

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklist 📝

- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes #6182 from lsm1/branch-jdbc-engine-use-overlay-conf.

Closes #6182

9ca8b48 [senmiaoliu] Using the overlay conf as session conf

Authored-by: senmiaoliu <senmiaoliu@trip.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
zhaohehuhu pushed a commit to zhaohehuhu/incubator-kyuubi that referenced this issue Mar 21, 2024
@zwangsheng @pan3793 @zhaohehuhu
[KYUUBI apache#6147][TRINO] Use the overlay conf as session conf
d02536d 
# 🔍 Description
## Issue References 🔗

This pull request fixes apache#6147

## Describe Your Solution 🔧

Overwriting the engine conf with the passed conf consider as the full session conf.

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests
None

---

# Checklist 📝

- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes apache#6148 from zwangsheng/KYUUBI#6147.

Closes apache#6147

3e0fb0f [Cheng Pan] Update externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionImpl.scala
452224e [zwangsheng] fix comments
4ee7c04 [zwangsheng] [KYUUBI apache#6147][TRINO] Using the overlay conf as session conf

Lead-authored-by: zwangsheng <binjieyang@apache.org>
Co-authored-by: Cheng Pan <pan3793@gmail.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
zhaohehuhu pushed a commit to zhaohehuhu/incubator-kyuubi that referenced this issue Mar 21, 2024
@lsm1 @zhaohehuhu
[KYUUBI apache#6182] JDBC engine using the overlay conf as session conf
ac3277c 
# 🔍 Description
## Issue References 🔗

## Describe Your Solution 🔧

Like apache#6147, JDBC engine should overwriting the engine conf with the passed conf consider as the full session conf.

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklist 📝

- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes apache#6182 from lsm1/branch-jdbc-engine-use-overlay-conf.

Closes apache#6182

9ca8b48 [senmiaoliu] Using the overlay conf as session conf

Authored-by: senmiaoliu <senmiaoliu@trip.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
Assignees
No one assigned
Labels
kind:bug This is a clearly a bug priority:major
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[KYUUBI #6147][TRINO] Use the overlay conf as session conf zwangsheng/kyuubi
1 participant
@zwangsheng