Skip to content

Conversation

@LA-Toth
Copy link

@LA-Toth LA-Toth commented Mar 2, 2022

What changes were proposed in this pull request?

  • Update jline to 3.21 - the behaviour is quite different. So a modified version of the JLine2's StringCompleter is added in hive-common.
  • Update sqlline to 1.12 as it uses jline 3.21
  • Add some missing dependencies to pom.xml files (the lack of these didn't cause issue possibly due to transitive dependency)

Why are the changes needed?

It's a step required to use JDK 17

Does this PR introduce any user-facing change?

No.

How was this patch tested?

By existing tests.

@LA-Toth
Copy link
Author

LA-Toth commented Mar 2, 2022

I created it partially based on pull request 2617.

@github-actions
Copy link

github-actions bot commented May 2, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.

@github-actions
Copy link

github-actions bot commented Aug 7, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.

@github-actions github-actions bot added the stale label Aug 7, 2022
@github-actions github-actions bot closed this Aug 15, 2022
@abstractdog
Copy link
Contributor

I just found this one, is there a chance that we can reopen, rebase @LA-Toth?
I was thinking about proper multiline support in HIVE-26542 and I think we have a better chance with jline3

@LA-Toth
Copy link
Author

LA-Toth commented Sep 19, 2022

I had one and only one issue: I was unable to run the tests to have a usable result. These are unstable even in @kgyrtkirk's docker image that is behind the jenkins AFAIK. So I was unable to decide if the current PR woks or not.

@LA-Toth
Copy link
Author

LA-Toth commented Sep 19, 2022

I still can't run the tests:

latest kgyrtkirk/hive-dev-box:executor docker image
freshly cloned hive repo from github (master branch)
mvn clean install -DskipTests -T 1C mvn clean install -pl beeline/ mvn clean install -pl beeline/ -Dtest=TestHiveCli#testDatabaseOptions
results:
[ERROR] Failures: [ERROR] TestHiveCli.testDatabaseOptions:107->verifyCMD:270 The expected keyword "testtbl" occur in the output: [ERROR] TestHiveCli.testSetHeaderValue:89->verifyCMD:270 The expected keyword "testtbl.a testtbl.b" occur in the output: [ERROR] TestHiveCli.testSourceCmd:114->verifyCMD:270 The expected keyword "sc1" occur in the output: [ERROR] TestHiveCli.testSourceCmd2:122->verifyCMD:270 The expected keyword "sc3" occur in the output: [ERROR] TestHiveCli.testSourceCmd4:138->verifyCMD:270 The expected keyword "testtbl" occur in the output: [ERROR] TestHiveCli.testSqlFromCmdWithDBName:166->verifyCMD:270 The expected keyword "testtbl" occur in the output:

and
[ERROR] Failures: [ERROR] TestHiveCli.setup:295->initFromFile:321->executeCMD:256 Supported return code is 0 while the actual is 2

@LA-Toth
Copy link
Author

LA-Toth commented Sep 19, 2022

with this change:

diff --git a/beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java b/beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java
index 5ea4d11b7a..f7be6875ee 100644
--- a/beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java
+++ b/beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java
@@ -291,8 +291,8 @@ public static void init(){
public void setup() throws IOException, URISyntaxException {
System.setProperty("datanucleus.schema.autoCreateAll", "true");
cli = new HiveCli();

  • initFromFile();
    redirectOutputStream();
  • initFromFile();
    }

private void redirectOutputStream() {

@nrg4878
Copy link
Contributor

nrg4878 commented May 14, 2024

@LA-Toth I have re-opened this to get some reviews. Seems like there is a conflict. Any chance we could have this rebased? seems like there is a conflict.

@github-actions github-actions bot removed the stale label May 15, 2024
@LA-Toth
Copy link
Author

LA-Toth commented May 29, 2024

Rebase itself seems to be relatively easy, the tests are still broken, at least:

[ERROR] Tests run: 9, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 4.667 s <<< FAILURE! - in org.apache.hadoop.hive.cli.TestCliDriverMethods
[ERROR] org.apache.hadoop.hive.cli.TestCliDriverMethods.testRun  Time elapsed: 0.829 s  <<< ERROR!
java.lang.NullPointerException

The first conflict during the rebase:
src/java/org/apache/hadoop/hive/cli/CliDriver.java' - the apache/master branch's changes are fine, but the code requries one more import:
import com.google.common.base.Splitter;

After two years I don't remember too much, it was just a regular change for me. And I haven't used Java for a while.
So, anyone who works on Hive daily can fix this faster than me.

Please feel free to improve my changes.

@abstractdog
Copy link
Contributor

abstractdog commented May 30, 2024

thanks @LA-Toth!
I just rebased the patch on top of the master, compiled locally, so I just started a new PR, we'll see the results

#5272

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.

@github-actions github-actions bot added the stale label Jul 30, 2024
@github-actions github-actions bot closed this Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants