HIVE-25495: Upgrade JLine to version 3

Author: LA-Toth

beeline/pom.xml

@@ -80,7 +80,7 @@
       <artifactId>jackson-core</artifactId>
     </dependency>
     <dependency>
-      <groupId>jline</groupId>
+      <groupId>org.jline</groupId>
       <artifactId>jline</artifactId>
     </dependency>
     <dependency>
@@ -114,6 +114,11 @@
         </exclusion>
       </exclusions>
     </dependency>
+    <dependency>
+      <groupId>org.apache.hadoop</groupId>
+      <artifactId>hadoop-hdfs</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.apache.thrift</groupId>
       <artifactId>libthrift</artifactId>

beeline/src/java/org/apache/hive/beeline/AbstractCommandHandler.java

@@ -26,8 +26,8 @@
 import java.util.LinkedList;
 import java.util.List;
 
-import jline.console.completer.Completer;
-import jline.console.completer.NullCompleter;
+import org.jline.reader.Completer;
+import org.jline.reader.impl.completer.NullCompleter;
 
 /**
  * An abstract implementation of CommandHandler.

beeline/src/java/org/apache/hive/beeline/BeeLine.java

@@ -28,6 +28,7 @@
 import java.io.EOFException;
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.IOError;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -100,6 +101,7 @@
 import org.apache.hive.beeline.hs2connection.HS2ConnectionFileUtils;
 import org.apache.hive.beeline.hs2connection.HiveSiteHS2ConnectionFileParser;
 import org.apache.hive.beeline.hs2connection.UserHS2ConnectionFileParser;
+import org.apache.hive.common.util.MatchingStringsCompleter;
 import org.apache.hive.common.util.ShutdownHookManager;
 import org.apache.hive.common.util.HiveStringUtils;
 import org.apache.hive.jdbc.HiveConnection;
@@ -110,11 +112,18 @@
 
 import com.google.common.annotations.VisibleForTesting;
 
-import jline.console.ConsoleReader;
-import jline.console.completer.Completer;
-import jline.console.completer.FileNameCompleter;
-import jline.console.completer.StringsCompleter;
-import jline.console.history.FileHistory;
+import org.jline.reader.Completer;
+import org.jline.reader.EndOfFileException;
+import org.jline.reader.History;
+import org.jline.reader.LineReader;
+import org.jline.reader.LineReaderBuilder;
+import org.jline.reader.impl.LineReaderImpl;
+import org.jline.reader.impl.history.DefaultHistory;
+import org.jline.terminal.Terminal;
+import org.jline.terminal.TerminalBuilder;
+
+import static org.jline.builtins.Completers.FileNameCompleter;
+
 
 /**
  * A console SQL shell with command completion.
@@ -151,14 +160,14 @@ public class BeeLine implements Closeable {
   private OutputFile recordOutputFile = null;
   private PrintStream outputStream = new PrintStream(System.out, true);
   private PrintStream errorStream = new PrintStream(System.err, true);
-  private InputStream inputStream = System.in;
-  private ConsoleReader consoleReader;
+  private Terminal terminal;
+  private LineReader lineReader;
   private List<String> batch = null;
   private final Reflector reflector = new Reflector(this);
   private String dbName = null;
   private String currentDatabase = null;
 
-  private FileHistory history;
+  private History history;
   // Indicates if this instance of beeline is running in compatibility mode, or beeline mode
   private boolean isBeeLine = true;
 
@@ -204,7 +213,7 @@ public class BeeLine implements Closeable {
       new ReflectiveCommandHandler(this, new String[] {"quit", "done", "exit"},
           null),
       new ReflectiveCommandHandler(this, new String[] {"connect", "open"},
-          new Completer[] {new StringsCompleter(getConnectionURLExamples())}),
+          new Completer[] {new MatchingStringsCompleter(getConnectionURLExamples())}),
       new ReflectiveCommandHandler(this, new String[] {"describe"},
           new Completer[] {new TableNameCompletor(this)}),
       new ReflectiveCommandHandler(this, new String[] {"indexes"},
@@ -233,7 +242,7 @@ public class BeeLine implements Closeable {
           null),
       new ReflectiveCommandHandler(this, new String[] {"metadata"},
           new Completer[] {
-              new StringsCompleter(getMetadataMethodNames())}),
+              new MatchingStringsCompleter(getMetadataMethodNames())}),
       new ReflectiveCommandHandler(this, new String[] {"nativesql"},
           null),
       new ReflectiveCommandHandler(this, new String[] {"dbinfo"},
@@ -263,9 +272,9 @@ public class BeeLine implements Closeable {
       new ReflectiveCommandHandler(this, new String[] {"closeall"},
           null),
       new ReflectiveCommandHandler(this, new String[] {"isolation"},
-          new Completer[] {new StringsCompleter(getIsolationLevels())}),
+          new Completer[] {new MatchingStringsCompleter(getIsolationLevels())}),
       new ReflectiveCommandHandler(this, new String[] {"outputformat"},
-          new Completer[] {new StringsCompleter(
+          new Completer[] {new MatchingStringsCompleter(
               formats.keySet().toArray(new String[0]))}),
       new ReflectiveCommandHandler(this, new String[] {"autocommit"},
           null),
@@ -309,9 +318,9 @@ public class BeeLine implements Closeable {
 
   static {
     try {
-      Class.forName("jline.console.ConsoleReader");
+      Class.forName("org.jline.reader.LineReader");
     } catch (Throwable t) {
-      throw new ExceptionInInitializerError("jline-missing");
+      throw new ExceptionInInitializerError("jline3-missing");
     }
   }
 
@@ -400,7 +409,7 @@ public class BeeLine implements Closeable {
         .withLongOpt("help")
         .withDescription("Display this message")
         .create('h'));
-    
+
     // -getUrlsFromBeelineSite
     options.addOption(OptionBuilder
         .withLongOpt("getUrlsFromBeelineSite")
@@ -433,7 +442,6 @@ public class BeeLine implements Closeable {
         .create());
   }
 
-
   static Manifest getManifest() throws IOException {
     URL base = BeeLine.class.getResource("/META-INF/MANIFEST.MF");
     URLConnection c = base.openConnection();
@@ -569,19 +577,15 @@ public BeeLine() {
   public BeeLine(boolean isBeeLine) {
     this.isBeeLine = isBeeLine;
     this.signalHandler = new SunSignalHandler(this);
-    this.shutdownHook = new Runnable() {
-      @Override
-      public void run() {
-        try {
-          if (history != null) {
-            history.setMaxSize(getOpts().getMaxHistoryRows());
-            history.flush();
-          }
-        } catch (IOException e) {
-          error(e);
-        } finally {
-          close();
+    this.shutdownHook = () -> {
+      try {
+        if (history != null) {
+          history.save();
         }
+      } catch (IOException e) {
+        error(e);
+      } finally {
+        close();
       }
     };
   }
@@ -863,7 +867,7 @@ private boolean connectUsingArgs(BeelineParser beelineParser, CommandLine cl) {
       getOpts().setHelpAsked(true);
       return true;
     }
-    
+
     if (cl.hasOption("getUrlsFromBeelineSite")) {
       printBeelineSiteUrls();
       getOpts().setBeelineSiteUrlsAsked(true);
@@ -937,8 +941,8 @@ private boolean connectUsingArgs(BeelineParser beelineParser, CommandLine cl) {
     String propertyFile = cl.getOptionValue("property-file");
     if (propertyFile != null) {
       try {
-        this.consoleReader = new ConsoleReader();
-      } catch (IOException e) {
+        this.lineReader = LineReaderBuilder.builder().build();
+      } catch (IOError e) {
         handleException(e);
       }
       if (!dispatch("!properties " + propertyFile)) {
@@ -980,7 +984,7 @@ private void printBeelineSiteUrls() {
       }
     }
   }
-  
+
   private boolean isZkBasedUrl(String urlFromBeelineSite) {
     String zkJdbcUriParam = ("serviceDiscoveryMode=zooKeeper").toLowerCase();
     if (urlFromBeelineSite.toLowerCase().contains(zkJdbcUriParam)) {
@@ -1116,9 +1120,9 @@ public int begin(String[] args, InputStream inputStream, boolean keepHistory) th
     //add shutdown hook to cleanup the beeline for smooth exit
     addBeelineShutdownHook();
 
-    //this method also initializes the consoleReader which is
+    //this method also initializes the lineReader which is
     //needed by initArgs for certain execution paths
-    ConsoleReader reader = initializeConsoleReader(inputStream);
+    initializeLineReader(inputStream);
     if (isBeeLine) {
       int code = initArgs(args);
       if (code != 0) {
@@ -1146,7 +1150,7 @@ public int begin(String[] args, InputStream inputStream, boolean keepHistory) th
     } catch (Exception e) {
       // ignore
     }
-    return execute(reader, false);
+    return execute(lineReader, false);
   }
 
   /*
@@ -1350,7 +1354,7 @@ private int executeFile(String fileName) {
         }
         fileStream = fs.open(path);
       }
-      return execute(initializeConsoleReader(fileStream), !getOpts().getForce());
+      return execute(initializeLineReader(fileStream), !getOpts().getForce());
     } catch (Throwable t) {
       handleException(t);
       return ERRNO_OTHER;
@@ -1359,16 +1363,17 @@ private int executeFile(String fileName) {
     }
   }
 
-  private int execute(ConsoleReader reader, boolean exitOnError) {
+  private int execute(LineReader reader, boolean exitOnError) {
     int lastExecutionResult = ERRNO_OK;
     Character mask = (System.getProperty("jline.terminal", "").equals("jline.UnsupportedTerminal")) ? null
-                       : ConsoleReader.NULL_MASK;
+                       : LineReaderImpl.NULL_MASK;
 
+    String line;
     while (!exit) {
       try {
         // Execute one instruction; terminate on executing a script if there is an error
         // in silent mode, prevent the query and prompt being echoed back to terminal
-        String line = (getOpts().isSilent() && getOpts().getScriptFile() != null) ? reader
+        line = (getOpts().isSilent() && getOpts().getScriptFile() != null) ? reader
             .readLine(null, mask) : reader.readLine(getPrompt());
 
         // trim line
@@ -1384,7 +1389,17 @@ private int execute(ConsoleReader reader, boolean exitOnError) {
         } else if (line != null) {
           lastExecutionResult = ERRNO_OK;
         }
-
+      } catch (EndOfFileException t) {
+        /*
+         * If you're reading from a normal file (not from standard input or a terminal), JLine might raise an
+         * EndOfFileException when it reaches the end of the file. JLine uses readLine() for reading input, and it
+         * expects the input source to provide data interactively. When reading from a file, it might misinterpret
+         * the EOF condition.
+         * It's unlikely that this catch-and-return-OK block masks a real issue.
+         * In interactive usage, EndOfFileException is not triggered, and in script file mode (-f),
+         * any potential corruption would typically surface as a failing command.
+         */
+        return lastExecutionResult;
       } catch (Throwable t) {
         handleException(t);
         return ERRNO_OTHER;
@@ -1403,48 +1418,64 @@ private void setupHistory() throws IOException {
        return;
     }
 
-    this.history = new FileHistory(new File(getOpts().getHistoryFile()));
+    this.history = new DefaultHistory();
   }
 
   private void addBeelineShutdownHook() throws IOException {
     // add shutdown hook to flush the history to history file and it also close all open connections
     ShutdownHookManager.addShutdownHook(getShutdownHook());
   }
 
-  public ConsoleReader initializeConsoleReader(InputStream inputStream) throws IOException {
-    if (inputStream != null) {
-      // ### NOTE: fix for sf.net bug 879425.
-      // Working around an issue in jline-2.1.2, see https://github.com/jline/jline/issues/10
-      // by appending a newline to the end of inputstream
-      InputStream inputStreamAppendedNewline = new SequenceInputStream(inputStream,
-          new ByteArrayInputStream((new String("\n")).getBytes()));
-      consoleReader = new ConsoleReader(inputStreamAppendedNewline, getErrorStream());
-      consoleReader.setCopyPasteDetection(true); // jline will detect if <tab> is regular character
-    } else {
-      consoleReader = new ConsoleReader(getInputStream(), getErrorStream());
-    }
-
-    //disable the expandEvents for the purpose of backward compatibility
-    consoleReader.setExpandEvents(false);
+  public LineReader initializeLineReader(InputStream inputStream) throws IOException {
+    final LineReaderBuilder builder = LineReaderBuilder.builder();
+    terminal = buildTerminal(prepareInputStream(inputStream));
+    builder.terminal(terminal);
 
+    if (inputStream instanceof FileInputStream || inputStream instanceof FSDataInputStream) {
+      // from script.. no need to load history and no need of completer, either
+      lineReader = builder.build();
+      return lineReader;
+    }
     try {
       // now set the output for the history
       if (this.history != null) {
-        consoleReader.setHistory(this.history);
-      } else {
-        consoleReader.setHistoryEnabled(false);
+        builder.history(this.history);
+        builder.variable(LineReader.HISTORY_FILE, new File(getOpts().getHistoryFile()));
+        builder.variable(LineReader.HISTORY_FILE_SIZE, getOpts().getMaxHistoryRows());
+        // in-memory keep more data, but at least 500 entries
+        builder.variable(LineReader.HISTORY_SIZE, Math.max(500, 3 * getOpts().getMaxHistoryRows()));
       }
     } catch (Exception e) {
       handleException(e);
     }
 
-    if (inputStream instanceof FileInputStream || inputStream instanceof FSDataInputStream) {
-      // from script.. no need to load history and no need of completer, either
-      return consoleReader;
+    builder.completer(new BeeLineCompleter(this));
+    lineReader = builder.build();
+    lineReader.unsetOpt(LineReader.Option.HISTORY_TIMESTAMPED);
+
+    if (this.history != null) {
+      this.history.attach(lineReader);
+    }
+
+    return lineReader;
+
+  }
+
+  private InputStream prepareInputStream(InputStream inputStream) {
+    if (inputStream != null) {
+      inputStream = new SequenceInputStream(inputStream,
+          new ByteArrayInputStream((new String("\n")).getBytes()));
     }
+    return inputStream;
+  }
 
-    consoleReader.addCompleter(new BeeLineCompleter(this));
-    return consoleReader;
+  protected Terminal buildTerminal(InputStream inputStream) throws IOException {
+    if (inputStream != null) { // typically when there is a file script to read from
+      return TerminalBuilder.builder().streams(inputStream, getErrorStream()).build();
+    } else { // no input stream, normal operation: proper behavior needs a system terminal
+      // system terminal can only be created with system streams
+      return TerminalBuilder.builder().system(true).dumb(false).streams(System.in, System.err).build();
+    }
   }
 
   void usage() {
@@ -1495,13 +1526,12 @@ boolean dispatch(String line) {
     }
 
     line = HiveStringUtils.removeComments(line);
+    line = line.trim();
 
-    if (line.trim().length() == 0) {
+    if (line.length() == 0) {
       return true;
     }
 
-    line = line.trim();
-
     // save it to the current script, if any
     if (scriptOutputFile != null) {
       scriptOutputFile.addLine(line);
@@ -2493,16 +2523,8 @@ PrintStream getErrorStream() {
     return errorStream;
   }
 
-  InputStream getInputStream() {
-    return inputStream;
-  }
-
-  ConsoleReader getConsoleReader() {
-    return consoleReader;
-  }
-
-  void setConsoleReader(ConsoleReader reader) {
-    this.consoleReader = reader;
+  LineReader getLineReader() {
+    return lineReader;
   }
 
   List<String> getBatch() {