Skip to content

Commit

Permalink
[KYUUBI #5918] Kyuubi BeeLine should check the relocated TTransportEx…
Browse files Browse the repository at this point in the history
…ception

# 🔍 Description
## Issue References 🔗

This pull request fixes #5918, when Kyuubi beeline executes incorrect SQL,it will exit with exception:
```
java.lang.NoClassDefFoundError: org/apache/thrift/transport/TTransportException
```

## Describe Your Solution 🔧
1.Supply `org.apache.thrift:libthrift` to `kyuubi-beeline` module.
2.We should override BeeLine's handleSQLException method,makes it compatible
  As we discussed, only `org.apache.kyuubi.shaded.thrift.transport.TTransportException` will be received by `kyuubi-beeline`,so it‘s  unnecessary  to add libthrift dependency to `kyuubi-beeline`.

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

- [ ] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project
- [ ] I have performed a self-review
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] 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?
- [ ] Test coverage is ok
- [x] Assignees are selected.
- [x] Minimum number of approvals
- [x] No changes are requested

**Be nice. Be informative.**

Closes #5919 from hadoopkandy/KYUUBI-5918.

Closes #5918

ee96c44 [kandy01.wang] [KYUUBI #5918] [Bug] kyuubi beeline exit with exception : java.lang.NoClassDefFoundError: org/apache/thrift/transport/TTransportException

Authored-by: kandy01.wang <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
  • Loading branch information
hadoopkandy authored and pan3793 committed Dec 27, 2023
1 parent c6177ab commit 2e817d5
Showing 1 changed file with 55 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@
import java.io.IOException;
import java.io.InputStream;
import java.sql.Driver;
import java.sql.SQLException;
import java.sql.SQLWarning;
import java.util.*;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.Options;
import org.apache.commons.cli.ParseException;
import org.apache.hive.common.util.HiveStringUtils;
import org.apache.kyuubi.shaded.thrift.transport.TTransportException;
import org.apache.kyuubi.util.reflect.DynConstructors;
import org.apache.kyuubi.util.reflect.DynFields;
import org.apache.kyuubi.util.reflect.DynMethods;
Expand Down Expand Up @@ -299,4 +302,56 @@ int runInit() {
boolean dispatch(String line) {
return super.dispatch(isPythonMode() ? line : HiveStringUtils.removeComments(line));
}

@Override
void handleSQLException(SQLException e) {
if (e instanceof SQLWarning && !(getOpts().getShowWarnings())) {
return;
}

if (e.getCause() instanceof TTransportException) {
switch (((TTransportException) e.getCause()).getType()) {
case TTransportException.ALREADY_OPEN:
error(loc("hs2-connection-already-open"));
break;
case TTransportException.END_OF_FILE:
error(loc("hs2-unexpected-end-of-file"));
break;
case TTransportException.NOT_OPEN:
error(loc("hs2-could-not-open-connection"));
break;
case TTransportException.TIMED_OUT:
error(loc("hs2-connection-timed-out"));
break;
case TTransportException.UNKNOWN:
error(loc("hs2-unknown-connection-problem"));
break;
default:
error(loc("hs2-unexpected-error"));
}
}

error(
loc(
e instanceof SQLWarning ? "Warning" : "Error",
new Object[] {
e.getMessage() == null ? "" : e.getMessage().trim(),
e.getSQLState() == null ? "" : e.getSQLState().trim(),
new Integer(e.getErrorCode())
}));

if (getOpts().getVerbose()) {
e.printStackTrace(getErrorStream());
}

if (!getOpts().getShowNestedErrs()) {
return;
}

for (SQLException nested = e.getNextException();
nested != null && nested != e;
nested = nested.getNextException()) {
handleSQLException(nested);
}
}
}

0 comments on commit 2e817d5

Please sign in to comment.