trinodb · kokosing · Feb 23, 2023 · May 11, 2022
diff --git a/client/trino-cli/src/main/java/io/trino/cli/ClientOptions.java b/client/trino-cli/src/main/java/io/trino/cli/ClientOptions.java
@@ -136,6 +136,9 @@ public class ClientOptions
     @Option(names = "--debug", paramLabel = "<debug>", description = "Enable debug information")
     public boolean debug;
 
+    @Option(names = "--history-file", paramLabel = "<historyFile>", defaultValue = "${env:TRINO_HISTORY_FILE:-${sys:user.home}/.trino_history}", description = "Path to the history file " + DEFAULT_VALUE)
+    public String historyFile;
+
     @Option(names = "--network-logging", paramLabel = "<level>", defaultValue = "NONE", description = "Network logging level [${COMPLETION-CANDIDATES}] " + DEFAULT_VALUE)
     public HttpLoggingInterceptor.Level networkLogging;
 
@@ -151,6 +154,9 @@ public class ClientOptions
     @Option(names = "--output-format-interactive", paramLabel = "<format>", defaultValue = "ALIGNED", description = "Output format for interactive mode [${COMPLETION-CANDIDATES}] " + DEFAULT_VALUE)
     public OutputFormat outputFormatInteractive;
 
+    @Option(names = "--pager", paramLabel = "<pager>", defaultValue = "${env:TRINO_PAGER}", description = "Path to the pager program used to display the query results")
+    public Optional<String> pager;
+
     @Option(names = "--resource-estimate", paramLabel = "<estimate>", description = "Resource estimate (property can be used multiple times; format is key=value)")
     public final List<ClientResourceEstimate> resourceEstimates = new ArrayList<>();
 

diff --git a/client/trino-cli/src/main/java/io/trino/cli/Console.java b/client/trino-cli/src/main/java/io/trino/cli/Console.java
@@ -49,9 +49,7 @@
 
 import static com.google.common.base.CharMatcher.whitespace;
 import static com.google.common.base.Preconditions.checkState;
-import static com.google.common.base.StandardSystemProperty.USER_HOME;
 import static com.google.common.base.Strings.isNullOrEmpty;
-import static com.google.common.base.Strings.nullToEmpty;
 import static com.google.common.io.Files.asCharSource;
 import static com.google.common.util.concurrent.Uninterruptibles.awaitUninterruptibly;
 import static io.trino.cli.Completion.commandCompleter;
@@ -191,11 +189,14 @@ public boolean run()
                         clientOptions.progress.orElse(false));
             }
 
+            Optional<String> pager = clientOptions.pager;
             runConsole(
                     queryRunner,
                     exiting,
                     clientOptions.outputFormatInteractive,
                     clientOptions.editingMode,
+                    getHistoryFile(clientOptions.historyFile),
+                    pager,
                     clientOptions.progress.orElse(true),
                     clientOptions.disableAutoSuggestion);
             return true;
@@ -232,11 +233,13 @@ private static void runConsole(
             AtomicBoolean exiting,
             OutputFormat outputFormat,
             ClientOptions.EditingMode editingMode,
+            Optional<Path> historyFile,
+            Optional<String> pager,
             boolean progress,
             boolean disableAutoSuggestion)
     {
         try (TableNameCompleter tableNameCompleter = new TableNameCompleter(queryRunner);
-                InputReader reader = new InputReader(editingMode, getHistoryFile(), disableAutoSuggestion, commandCompleter(), tableNameCompleter)) {
+                InputReader reader = new InputReader(editingMode, historyFile, disableAutoSuggestion, commandCompleter(), tableNameCompleter)) {
             tableNameCompleter.populateCache();
             String remaining = "";
             while (!exiting.get()) {
@@ -301,7 +304,7 @@ private static void runConsole(
                         currentOutputFormat = OutputFormat.VERTICAL;
                     }
 
-                    process(queryRunner, split.statement(), currentOutputFormat, tableNameCompleter::populateCache, true, progress, reader.getTerminal(), System.out, System.out);
+                    process(queryRunner, split.statement(), currentOutputFormat, tableNameCompleter::populateCache, pager, progress, reader.getTerminal(), System.out, System.out);
                 }
 
                 // replace remaining with trailing partial statement
@@ -325,7 +328,7 @@ private static boolean executeCommand(
         StatementSplitter splitter = new StatementSplitter(query);
         for (Statement split : splitter.getCompleteStatements()) {
             if (!isEmptyStatement(split.statement())) {
-                if (!process(queryRunner, split.statement(), outputFormat, () -> {}, false, showProgress, getTerminal(), System.out, System.err)) {
+                if (!process(queryRunner, split.statement(), outputFormat, () -> {}, Optional.of(""), showProgress, getTerminal(), System.out, System.err)) {
                     if (!ignoreErrors) {
                         return false;
                     }
@@ -348,7 +351,7 @@ private static boolean process(
             String sql,
             OutputFormat outputFormat,
             Runnable schemaChanged,
-            boolean usePager,
+            Optional<String> pager,
             boolean showProgress,
             Terminal terminal,
             PrintStream out,
@@ -371,7 +374,7 @@ private static boolean process(
         }
 
         try (Query query = queryRunner.startQuery(finalSql)) {
-            boolean success = query.renderOutput(terminal, out, errorChannel, outputFormat, usePager, showProgress);
+            boolean success = query.renderOutput(terminal, out, errorChannel, outputFormat, pager, showProgress);
 
             ClientSession session = queryRunner.getSession();
 
@@ -440,12 +443,11 @@ private static boolean process(
         }
     }
 
-    private static Path getHistoryFile()
+    private static Optional<Path> getHistoryFile(String path)
     {
-        String path = System.getenv("TRINO_HISTORY_FILE");
-        if (!isNullOrEmpty(path)) {
-            return Paths.get(path);
+        if (isNullOrEmpty(path)) {
+            return Optional.empty();
         }
-        return Paths.get(nullToEmpty(USER_HOME.value()), ".trino_history");
+        return Optional.of(Paths.get(path));
     }
 }
diff --git a/client/trino-cli/src/main/java/io/trino/cli/InputReader.java b/client/trino-cli/src/main/java/io/trino/cli/InputReader.java
@@ -26,6 +26,7 @@
 import java.io.Closeable;
 import java.io.IOException;
 import java.nio.file.Path;
+import java.util.Optional;
 
 import static io.trino.cli.TerminalUtils.isRealTerminal;
 import static org.jline.reader.LineReader.BLINK_MATCHING_PAREN;
@@ -42,19 +43,20 @@ public class InputReader
 {
     private final LineReader reader;
 
-    public InputReader(ClientOptions.EditingMode editingMode, Path historyFile, boolean disableAutoSuggestion, Completer... completers)
+    public InputReader(ClientOptions.EditingMode editingMode, Optional<Path> historyFile, boolean disableAutoSuggestion, Completer... completers)
             throws IOException
     {
-        reader = LineReaderBuilder.builder()
+        LineReaderBuilder builder = LineReaderBuilder.builder()
                 .terminal(TerminalUtils.getTerminal())
                 .variable(HISTORY_FILE, historyFile)
                 .variable(SECONDARY_PROMPT_PATTERN, isRealTerminal() ? colored("%P -> ") : "") // workaround for https://github.com/jline/jline3/issues/751
                 .variable(BLINK_MATCHING_PAREN, 0)
                 .option(HISTORY_IGNORE_SPACE, false) // store history even if the query starts with spaces
                 .parser(new InputParser())
                 .highlighter(new InputHighlighter())
-                .completer(new AggregateCompleter(completers))
-                .build();
+                .completer(new AggregateCompleter(completers));
+        historyFile.ifPresent(path -> builder.variable(HISTORY_FILE, path));
+        reader = builder.build();
 
         reader.getKeyMaps().put(MAIN, reader.getKeyMaps().get(editingMode.getKeyMap()));
         reader.unsetOpt(HISTORY_TIMESTAMPED);

diff --git a/client/trino-cli/src/main/java/io/trino/cli/Pager.java b/client/trino-cli/src/main/java/io/trino/cli/Pager.java
@@ -21,14 +21,14 @@
 import java.io.IOException;
 import java.io.OutputStream;
 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 
 import static com.google.common.base.Preconditions.checkState;
 
 public class Pager
         extends FilterOutputStream
 {
-    public static final String ENV_PAGER = "TRINO_PAGER";
     public static final List<String> LESS = ImmutableList.of("less", "-FXRSn");
 
     private final Process process;
@@ -131,17 +131,10 @@ private static IOException propagateIOException(IOException e)
         throw e;
     }
 
-    public static Pager create()
+    public static Pager create(Optional<String> pagerName)
     {
-        String pager = System.getenv(ENV_PAGER);
-        if (pager == null) {
-            return create(LESS);
-        }
-        pager = pager.trim();
-        if (pager.isEmpty()) {
-            return createNullPager();
-        }
-        return create(ImmutableList.of("/bin/sh", "-c", pager));
+        return pagerName.map(name -> create(ImmutableList.of("/bin/sh", "-c", name)))
+                .orElseGet(() -> create(LESS));
     }
 
     public static Pager create(List<String> command)

diff --git a/client/trino-cli/src/main/java/io/trino/cli/Query.java b/client/trino-cli/src/main/java/io/trino/cli/Query.java
@@ -126,7 +126,7 @@ public boolean isClearTransactionId()
         return client.isClearTransactionId();
     }
 
-    public boolean renderOutput(Terminal terminal, PrintStream out, PrintStream errorChannel, OutputFormat outputFormat, boolean usePager, boolean showProgress)
+    public boolean renderOutput(Terminal terminal, PrintStream out, PrintStream errorChannel, OutputFormat outputFormat, Optional<String> pager, boolean showProgress)
     {
         Thread clientThread = Thread.currentThread();
         SignalHandler oldHandler = terminal.handle(Signal.INT, signal -> {
@@ -137,21 +137,21 @@ public boolean renderOutput(Terminal terminal, PrintStream out, PrintStream erro
             clientThread.interrupt();
         });
         try {
-            return renderQueryOutput(terminal, out, errorChannel, outputFormat, usePager, showProgress);
+            return renderQueryOutput(terminal, out, errorChannel, outputFormat, pager, showProgress);
         }
         finally {
             terminal.handle(Signal.INT, oldHandler);
             Thread.interrupted(); // clear interrupt status
         }
     }
 
-    private boolean renderQueryOutput(Terminal terminal, PrintStream out, PrintStream errorChannel, OutputFormat outputFormat, boolean usePager, boolean showProgress)
+    private boolean renderQueryOutput(Terminal terminal, PrintStream out, PrintStream errorChannel, OutputFormat outputFormat, Optional<String> pager, boolean showProgress)
     {
         StatusPrinter statusPrinter = null;
         WarningsPrinter warningsPrinter = new PrintStreamWarningsPrinter(errorChannel);
 
         if (showProgress) {
-            statusPrinter = new StatusPrinter(client, errorChannel, debug, usePager);
+            statusPrinter = new StatusPrinter(client, errorChannel, debug, isInteractive(pager));
             statusPrinter.printInitialStatusUpdates(terminal);
         }
         else {
@@ -162,7 +162,7 @@ private boolean renderQueryOutput(Terminal terminal, PrintStream out, PrintStrea
         if (client.isRunning() || (client.isFinished() && client.finalStatusInfo().getError() == null)) {
             QueryStatusInfo results = client.isRunning() ? client.currentStatusInfo() : client.finalStatusInfo();
             if (results.getUpdateType() != null) {
-                renderUpdate(terminal, errorChannel, results, outputFormat, usePager);
+                renderUpdate(terminal, errorChannel, results, outputFormat, pager);
             }
             // TODO once https://github.com/trinodb/trino/issues/14253 is done this else here should be needed
             // and should be replaced with just simple:
@@ -173,7 +173,7 @@ else if (results.getColumns() == null) {
                 return false;
             }
             else {
-                renderResults(terminal, out, outputFormat, usePager, results.getColumns());
+                renderResults(terminal, out, outputFormat, pager, results.getColumns());
             }
         }
 
@@ -203,6 +203,11 @@ else if (results.getColumns() == null) {
         return true;
     }
 
+    private boolean isInteractive(Optional<String> pager)
+    {
+        return pager.map(name -> name.trim().length() != 0).orElse(true);
+    }
+
     private void processInitialStatusUpdates(WarningsPrinter warningsPrinter)
     {
         while (client.isRunning() && (client.currentData().getData() == null)) {
@@ -224,7 +229,7 @@ private void processInitialStatusUpdates(WarningsPrinter warningsPrinter)
         warningsPrinter.print(warnings, false, true);
     }
 
-    private void renderUpdate(Terminal terminal, PrintStream out, QueryStatusInfo results, OutputFormat outputFormat, boolean usePager)
+    private void renderUpdate(Terminal terminal, PrintStream out, QueryStatusInfo results, OutputFormat outputFormat, Optional<String> pager)
     {
         String status = results.getUpdateType();
         if (results.getUpdateCount() != null) {
@@ -234,7 +239,7 @@ private void renderUpdate(Terminal terminal, PrintStream out, QueryStatusInfo re
         }
         else if (results.getColumns() != null && !results.getColumns().isEmpty()) {
             out.println(status);
-            renderResults(terminal, out, outputFormat, usePager, results.getColumns());
+            renderResults(terminal, out, outputFormat, pager, results.getColumns());
         }
         else {
             out.println(status);
@@ -252,10 +257,10 @@ private void discardResults()
         }
     }
 
-    private void renderResults(Terminal terminal, PrintStream out, OutputFormat outputFormat, boolean interactive, List<Column> columns)
+    private void renderResults(Terminal terminal, PrintStream out, OutputFormat outputFormat, Optional<String> pager, List<Column> columns)
     {
         try {
-            doRenderResults(terminal, out, outputFormat, interactive, columns);
+            doRenderResults(terminal, out, outputFormat, pager, columns);
         }
         catch (QueryAbortedException e) {
             System.out.println("(query aborted by user)");
@@ -266,21 +271,21 @@ private void renderResults(Terminal terminal, PrintStream out, OutputFormat outp
         }
     }
 
-    private void doRenderResults(Terminal terminal, PrintStream out, OutputFormat format, boolean interactive, List<Column> columns)
+    private void doRenderResults(Terminal terminal, PrintStream out, OutputFormat format, Optional<String> pager, List<Column> columns)
             throws IOException
     {
-        if (interactive) {
-            pageOutput(format, terminal.getWidth(), columns);
+        if (isInteractive(pager)) {
+            pageOutput(pager, format, terminal.getWidth(), columns);
         }
         else {
             sendOutput(out, format, terminal.getWidth(), columns);
         }
     }
 
-    private void pageOutput(OutputFormat format, int maxWidth, List<Column> columns)
+    private void pageOutput(Optional<String> pagerName, OutputFormat format, int maxWidth, List<Column> columns)
             throws IOException
     {
-        try (Pager pager = Pager.create();
+        try (Pager pager = Pager.create(pagerName);
                 ThreadInterruptor clientThread = new ThreadInterruptor();
                 Writer writer = createWriter(pager);
                 OutputHandler handler = createOutputHandler(format, maxWidth, writer, columns)) {

diff --git a/client/trino-cli/src/test/java/io/trino/cli/TestInsecureQueryRunner.java b/client/trino-cli/src/test/java/io/trino/cli/TestInsecureQueryRunner.java
@@ -26,6 +26,7 @@
 import java.io.InputStream;
 import java.security.KeyStore;
 import java.security.SecureRandom;
+import java.util.Optional;
 
 import static com.google.common.io.Resources.getResource;
 import static com.google.common.net.HttpHeaders.CONTENT_TYPE;
@@ -74,7 +75,7 @@ public void testInsecureConnection()
         QueryRunner queryRunner = createQueryRunner(createClientSession(server), true);
 
         try (Query query = queryRunner.startQuery("query with insecure mode")) {
-            query.renderOutput(getTerminal(), nullPrintStream(), nullPrintStream(), CSV, false, false);
+            query.renderOutput(getTerminal(), nullPrintStream(), nullPrintStream(), CSV, Optional.of(""), false);
         }
 
         assertEquals(server.takeRequest().getPath(), "/v1/statement");

diff --git a/client/trino-cli/src/test/java/io/trino/cli/TestQueryRunner.java b/client/trino-cli/src/test/java/io/trino/cli/TestQueryRunner.java
@@ -88,10 +88,10 @@ public void testCookie()
         QueryRunner queryRunner = createQueryRunner(createClientSession(server), false);
 
         try (Query query = queryRunner.startQuery("first query will introduce a cookie")) {
-            query.renderOutput(getTerminal(), nullPrintStream(), nullPrintStream(), CSV, false, false);
+            query.renderOutput(getTerminal(), nullPrintStream(), nullPrintStream(), CSV, Optional.of(""), false);
         }
         try (Query query = queryRunner.startQuery("second query should carry the cookie")) {
-            query.renderOutput(getTerminal(), nullPrintStream(), nullPrintStream(), CSV, false, false);
+            query.renderOutput(getTerminal(), nullPrintStream(), nullPrintStream(), CSV, Optional.of(""), false);
         }
 
         assertNull(server.takeRequest().getHeader("Cookie"));

diff --git a/docs/src/main/sphinx/client/cli.rst b/docs/src/main/sphinx/client/cli.rst
@@ -161,13 +161,19 @@ mode:
       EMACS editors. Defaults to ``EMACS``.
   * - ``--http-proxy``
     - Configures the URL of the HTTP proxy to connect to Trino.
+  * - ``--history-file``
+    - Path to the :ref:`history file <cli-history>`. Defaults to ``~/.trino_history``.
   * - ``--network-logging``
     - Configures the level of detail provided for network logging of the CLI.
       Defaults to ``NONE``, other options are ``BASIC``, ``HEADERS``, or
       ``BODY``.
   * - ``--output-format-interactive=<format>``
     - Specify the :ref:`format <cli-output-format>` to use
       for printing query results. Defaults to ``ALIGNED``.
+  * - ``--pager=<pager>``
+    - Path to the pager program used to display the query results. Set to
+      an empty value to completely disable pagination. Defaults to ``less``
+      with a carefully selected set of options.
   * - ``--no-progress``
     - Do not show query processing progress.
   * - ``--password``
@@ -423,10 +429,13 @@ Pagination
 
 By default, the results of queries are paginated using the ``less`` program
 which is configured with a carefully selected set of options. This behavior
-can be overridden by setting the environment variable ``TRINO_PAGER`` to the
-name of a different program such as ``more`` or `pspg <https://github.com/okbob/pspg>`_,
+can be overridden by setting the ``--pager`` option or
+the ``TRINO_PAGER`` environment variable to the name of a different program
+such as ``more`` or `pspg <https://github.com/okbob/pspg>`_,
 or it can be set to an empty value to completely disable pagination.
 
+.. _cli-history:
+
 History
 -------
 
@@ -436,7 +445,8 @@ history by scrolling or searching. Use the up and down arrows to scroll and
 press :kbd:`Enter`.
 
 By default, you can locate the Trino history file in ``~/.trino_history``.
-Use the ``TRINO_HISTORY_FILE`` environment variable to change the default.
+Use the ``--history-file`` option or the ```TRINO_HISTORY_FILE`` environment variable
+to change the default.
 
 Auto suggestion
 ^^^^^^^^^^^^^^^