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] Do not throw exception in KyuubiStatement#getMoreResults(int) in case of closing current result set #6114

Closed
3 of 4 tasks
tigrulya-exe opened this issue Feb 28, 2024 · 1 comment
Labels
kind:bug This is a clearly a bug priority:major

Comments

@tigrulya-exe
Copy link
Contributor

Code of Conduct

Search before asking

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

Describe the bug

After fixing HIVE-7680 issue the method getMoreResults() of HiveStatement and respectively KyuubiStatementclasses returns false instead of throwing exception. The javadoc of the Statement#getMoreResults()method says, that it should implicitly close any current ResultSet object, i.e. is similar to calling the KyuubiStatement#getMoreResults(int) method with Statement.CLOSE_CURRENT_RESULT argument. Therefore, in the latter case, we should also return false instead of failing with an exception.

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.
@tigrulya-exe tigrulya-exe added kind:bug This is a clearly a bug priority:major labels Feb 28, 2024
Copy link

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

pan3793 pushed a commit that referenced this issue Feb 29, 2024
…URRENT_RESULT)

# 🔍 Description
## Issue References 🔗

This pull request fixes #6114

## Describe Your Solution 🔧

After fixing [HIVE-7680](https://issues.apache.org/jira/browse/HIVE-7680) issue the method getMoreResults() of HiveStatement and respectively KyuubiStatementclasses returns false instead of throwing exception. The javadoc of the Statement#getMoreResults()method says, that it should implicitly close any current ResultSet object, i.e. is similar to calling the KyuubiStatement#getMoreResults(int) method with Statement.CLOSE_CURRENT_RESULT argument. Therefore, in the latter case, we should also return false instead of failing with an exception. This fix also allows Kyuubi JDBC driver to be used in the JDBC engine.

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

- [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 #6115 from tigrulya-exe/bugfix/jdbc-driver-get-more-results.

Closes #6114

b674791 [Tigran Manasyan] Close result set in case of KyuubiStatement#getMoreResults(CLOSE_CURRENT_RESULT)
eeedaf8 [Tigran Manasyan] Do not throw exception in KyuubiStatement#getMoreResults(int) in case of closing current result set

Authored-by: Tigran Manasyan <t.manasyan@arenadata.io>
Signed-off-by: Cheng Pan <chengpan@apache.org>
(cherry picked from commit abb782c)
Signed-off-by: Cheng Pan <chengpan@apache.org>
zhaohehuhu pushed a commit to zhaohehuhu/incubator-kyuubi that referenced this issue Mar 21, 2024
…LOSE_CURRENT_RESULT)

# 🔍 Description
## Issue References 🔗

This pull request fixes apache#6114

## Describe Your Solution 🔧

After fixing [HIVE-7680](https://issues.apache.org/jira/browse/HIVE-7680) issue the method getMoreResults() of HiveStatement and respectively KyuubiStatementclasses returns false instead of throwing exception. The javadoc of the Statement#getMoreResults()method says, that it should implicitly close any current ResultSet object, i.e. is similar to calling the KyuubiStatement#getMoreResults(int) method with Statement.CLOSE_CURRENT_RESULT argument. Therefore, in the latter case, we should also return false instead of failing with an exception. This fix also allows Kyuubi JDBC driver to be used in the JDBC engine.

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

- [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#6115 from tigrulya-exe/bugfix/jdbc-driver-get-more-results.

Closes apache#6114

b674791 [Tigran Manasyan] Close result set in case of KyuubiStatement#getMoreResults(CLOSE_CURRENT_RESULT)
eeedaf8 [Tigran Manasyan] Do not throw exception in KyuubiStatement#getMoreResults(int) in case of closing current result set

Authored-by: Tigran Manasyan <t.manasyan@arenadata.io>
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
1 participant