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

Fork beeline module from Apache Hive 3.1.3 #6109

Closed
wants to merge 3 commits into from

Conversation

pan3793
Copy link
Member

@pan3793 pan3793 commented Feb 28, 2024

🔍 Description

Issue References 🔗

This is the first step of #6146, to gain a clear commit history, this PR just simply copied the hive-beeline module from Apache Hive 3.1.3, with minimal change to pass the tests and manually test basic functionalities, following PRs are going to remove other Hive deps gradually.

Describe Your Solution 🔧

  • Copy source code and test case from Apache Hive 3.1.3
  • Drop org.apache.hive:hive-beeline:3.1.3
  • Backport HIVE-21584 to support JDK 9+
  • Drop HiveCli, HiveSchemaTool and BeelineInPlaceUpdateStream, and the corresponding test cases
  • Temporary ignore(will fix later) TestClientCommandHookFactory#testConnectHook because of error NoClassDefFound org/apache/curator/RetryPolicy
  • Tune testing code to pass UT

Types of changes 🔖

  • 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 🧪

Minimal changes to pass the unit tests.


Checklist 📝

Be nice. Be informative.

@pan3793
Copy link
Member Author

pan3793 commented Feb 28, 2024

Before completing the work, I'd like to collect feedback from the community first.

@pan3793 pan3793 requested review from turboFei and yaooqinn February 28, 2024 05:58
@yaooqinn
Copy link
Member

I'm not a fan of fork instead of upstream first. I have no comments. Leave it to others to decide.

@pan3793
Copy link
Member Author

pan3793 commented Mar 7, 2024

The change is almost done, I manually tested some cases locally, and everything went well.

This is the last part to cut Hive deps out from Kyuubi server and client modules.

I'm going to split it to gain a clear changelog history.

@pan3793 pan3793 force-pushed the fork-beeline branch 2 times, most recently from e9161d7 to 87f31af Compare March 7, 2024 11:40
@codecov-commenter
Copy link

codecov-commenter commented Mar 10, 2024

Codecov Report

Attention: Patch coverage is 34.03951% with 2304 lines in your changes are missing coverage. Please review.

Project coverage is 58.69%. Comparing base (9c18a7e) to head (885f9fe).
Report is 1 commits behind head on master.

Files Patch % Lines
...rc/main/java/org/apache/hive/beeline/Commands.java 9.15% 797 Missing and 27 partials ⚠️
...src/main/java/org/apache/hive/beeline/BeeLine.java 43.45% 521 Missing and 62 partials ⚠️
...main/java/org/apache/hive/beeline/BeeLineOpts.java 46.35% 135 Missing and 12 partials ⚠️
...va/org/apache/hive/beeline/DatabaseConnection.java 16.55% 118 Missing and 8 partials ⚠️
.../beeline/hs2connection/HS2ConnectionFileUtils.java 39.68% 71 Missing and 5 partials ⚠️
...hs2connection/HiveSiteHS2ConnectionFileParser.java 0.00% 74 Missing ⚠️
.../hive/beeline/hs2connection/BeelineSiteParser.java 19.67% 45 Missing and 4 partials ⚠️
...ache/hive/beeline/SeparatedValuesOutputFormat.java 10.00% 45 Missing ⚠️
...c/main/java/org/apache/hive/beeline/Reflector.java 34.48% 17 Missing and 21 partials ⚠️
...ain/java/org/apache/hive/beeline/SQLCompleter.java 0.00% 38 Missing ⚠️
... and 26 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6109      +/-   ##
============================================
- Coverage     61.08%   58.69%   -2.40%     
  Complexity       24       24              
============================================
  Files           625      663      +38     
  Lines         37219    40712    +3493     
  Branches       5029     5602     +573     
============================================
+ Hits          22735    23895    +1160     
- Misses        12040    14187    +2147     
- Partials       2444     2630     +186     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pan3793 pan3793 marked this pull request as ready for review March 10, 2024 19:07
@github-actions github-actions bot added the kind:infra license, community building, project builds, asf infra related, etc. label Mar 10, 2024
@pan3793 pan3793 requested a review from ulysses-you March 11, 2024 02:16
@pan3793 pan3793 self-assigned this Mar 11, 2024
@pan3793 pan3793 added this to the v1.9.0 milestone Mar 11, 2024
@ulysses-you
Copy link
Contributor

why fork beeline module can resolve CVE, does those transitive dependencies go away ?

@pan3793
Copy link
Member Author

pan3793 commented Mar 11, 2024

why fork beeline module can resolve CVE, does those transitive dependencies go away ?

Fork and do minor changes can cut out all Hive's vulnerable deps. To gain a clear git history, those minor changes are planed in follow-up PRs.

@pan3793
Copy link
Member Author

pan3793 commented Mar 11, 2024

@ulysses-you the whole proposal is listed under #6146

@cfmcgrady
Copy link
Contributor

Fork and do minor changes can cut out all Hive's vulnerable deps.

+1

Copy link
Contributor

@ulysses-you ulysses-you left a comment

Choose a reason for hiding this comment

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

we can go back to upstream once Hive community become active on beeline module

@pan3793 pan3793 closed this in 64058a0 Mar 11, 2024
@pan3793
Copy link
Member Author

pan3793 commented Mar 11, 2024

Thanks, merged to master

zhouyifan279 pushed a commit to zhouyifan279/kyuubi that referenced this pull request Mar 11, 2024
# 🔍 Description
## Issue References 🔗

This is the first step of apache#6146, to gain a clear commit history, this PR just simply copied the `hive-beeline` module from Apache Hive 3.1.3, with minimal change to pass the tests and manually test basic functionalities, following PRs are going to remove other Hive deps gradually.

## Describe Your Solution 🔧

- Copy source code and test case from Apache Hive 3.1.3
- Drop `org.apache.hive:hive-beeline:3.1.3`
- Backport HIVE-21584 to support JDK 9+
- Drop `HiveCli`, `HiveSchemaTool` and `BeelineInPlaceUpdateStream`, and the corresponding test cases
- Temporary ignore(will fix later) `TestClientCommandHookFactory#testConnectHook` because of error `NoClassDefFound org/apache/curator/RetryPolicy`
- Tune testing code to pass UT

## Types of changes :bookmark:

- [ ] 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 🧪

Minimal changes to pass the 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#6109 from pan3793/fork-beeline.

Closes apache#6109

885f9fe [Cheng Pan] NOTICE
a2efa1c [Cheng Pan] fix
5bb1cc9 [Cheng Pan] Copy from Apache Hive 3.1.3

Authored-by: Cheng Pan <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
zhaohehuhu pushed a commit to zhaohehuhu/incubator-kyuubi that referenced this pull request Mar 21, 2024
# 🔍 Description
## Issue References 🔗

This is the first step of apache#6146, to gain a clear commit history, this PR just simply copied the `hive-beeline` module from Apache Hive 3.1.3, with minimal change to pass the tests and manually test basic functionalities, following PRs are going to remove other Hive deps gradually.

## Describe Your Solution 🔧

- Copy source code and test case from Apache Hive 3.1.3
- Drop `org.apache.hive:hive-beeline:3.1.3`
- Backport HIVE-21584 to support JDK 9+
- Drop `HiveCli`, `HiveSchemaTool` and `BeelineInPlaceUpdateStream`, and the corresponding test cases
- Temporary ignore(will fix later) `TestClientCommandHookFactory#testConnectHook` because of error `NoClassDefFound org/apache/curator/RetryPolicy`
- Tune testing code to pass UT

## Types of changes :bookmark:

- [ ] 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 🧪

Minimal changes to pass the 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#6109 from pan3793/fork-beeline.

Closes apache#6109

885f9fe [Cheng Pan] NOTICE
a2efa1c [Cheng Pan] fix
5bb1cc9 [Cheng Pan] Copy from Apache Hive 3.1.3

Authored-by: Cheng Pan <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
@pan3793 pan3793 deleted the fork-beeline branch June 6, 2024 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:build kind:infra license, community building, project builds, asf infra related, etc. module:ctl module:hive
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants