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

[Bug] Incorrect parsing of the jdbc url. #5849

Closed
3 of 4 tasks
hameizi opened this issue Dec 12, 2023 · 1 comment
Closed
3 of 4 tasks

[Bug] Incorrect parsing of the jdbc url. #5849

hameizi opened this issue Dec 12, 2023 · 1 comment
Labels
kind:bug This is a clearly a bug priority:major

Comments

@hameizi
Copy link
Contributor

hameizi commented Dec 12, 2023

Code of Conduct

Search before asking

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

Describe the bug

When some configuration information in our jdbc url contains the same string as the host:port for connecting to kyuubi, this configuration information will be incorrectly overridden. For example, if I configure the following connection string:
jdbc:hive2://localhost:2182/;serviceDiscoveryMode=zooKeeper;zooKeeperNamespace=kyuubi-cluster?spark.sql.catalog.test_catalog.url=zookeeper://localhost:2182/default/test_catalog
kyuubi will interpret it as an incorrect connection string:
jdbc:hive2://localhost:10009/;serviceDiscoveryMode=zooKeeper;zooKeeperNamespace=kyuubi-cluster?spark.sql.catalog.test_catalog.url=zookeeper://dummyhost:00000/default/test_catalog

replacing my catalog url from zookeeper://localhost:2182/default/test_catalog to zookeeper://dummyhost:00000/default/test_catalog.

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.
@hameizi hameizi added kind:bug This is a clearly a bug priority:major labels Dec 12, 2023
Copy link

Hello @hameizi,
Thanks for finding the time to report the issue!
We really appreciate the community's efforts to improve Apache Kyuubi.

@hameizi hameizi changed the title [Bug] [Bug] Incorrect parsing of the jdbc url. Dec 12, 2023
pan3793 pushed a commit that referenced this issue Dec 13, 2023
# 🔍 Description
## Issue References 🔗

This pull request fixes #5849

## Describe Your Solution 🔧

When replacing the `host:port` information in the jdbc url, only the first matching string should be replaced.

## 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

---

# Checklists
## 📝 Author Self Checklist

- [x] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project
- [x] I have performed a self-review
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

## 📝 Committer Pre-Merge Checklist

- [x] Pull request title is okay.
- [x] No license issues.
- [x] Milestone correctly set?
- [x] Test coverage is ok
- [x] Assignees are selected.
- [x] Minimum number of approvals
- [x] No changes are requested

**Be nice. Be informative.**

Closes #5850 from hameizi/fix-url-parse.

Closes #5849

4914078 [wangzeyu] fix code style
32565f2 [wangzeyu] fix unit test
e1e6d0a [wangzeyu] add unit test
9af1d05 [wangzeyu] fix

Authored-by: wangzeyu <hameizi369@gmail.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
(cherry picked from commit 1b36ee5)
Signed-off-by: Cheng Pan <chengpan@apache.org>
pan3793 pushed a commit that referenced this issue Dec 13, 2023
# 🔍 Description
## Issue References 🔗

This pull request fixes #5849

## Describe Your Solution 🔧

When replacing the `host:port` information in the jdbc url, only the first matching string should be replaced.

## 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

---

# Checklists
## 📝 Author Self Checklist

- [x] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project
- [x] I have performed a self-review
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

## 📝 Committer Pre-Merge Checklist

- [x] Pull request title is okay.
- [x] No license issues.
- [x] Milestone correctly set?
- [x] Test coverage is ok
- [x] Assignees are selected.
- [x] Minimum number of approvals
- [x] No changes are requested

**Be nice. Be informative.**

Closes #5850 from hameizi/fix-url-parse.

Closes #5849

4914078 [wangzeyu] fix code style
32565f2 [wangzeyu] fix unit test
e1e6d0a [wangzeyu] add unit test
9af1d05 [wangzeyu] fix

Authored-by: wangzeyu <hameizi369@gmail.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
(cherry picked from commit 1b36ee5)
Signed-off-by: Cheng Pan <chengpan@apache.org>
zhaohehuhu pushed a commit to zhaohehuhu/incubator-kyuubi that referenced this issue Mar 21, 2024
…es colon

# 🔍 Description
## Issue References 🔗

This pull request fixes apache#5849

## Describe Your Solution 🔧

When replacing the `host:port` information in the jdbc url, only the first matching string should be replaced.

## 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

---

# Checklists
## 📝 Author Self Checklist

- [x] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project
- [x] I have performed a self-review
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

## 📝 Committer Pre-Merge Checklist

- [x] Pull request title is okay.
- [x] No license issues.
- [x] Milestone correctly set?
- [x] Test coverage is ok
- [x] Assignees are selected.
- [x] Minimum number of approvals
- [x] No changes are requested

**Be nice. Be informative.**

Closes apache#5850 from hameizi/fix-url-parse.

Closes apache#5849

4914078 [wangzeyu] fix code style
32565f2 [wangzeyu] fix unit test
e1e6d0a [wangzeyu] add unit test
9af1d05 [wangzeyu] fix

Authored-by: wangzeyu <hameizi369@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
kind:bug This is a clearly a bug priority:major
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant