You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/11/25 02:07:04 UTC

[GitHub] [flink] fsk119 commented on a diff in pull request #17830: [FLINK-24893][Table SQL/Client][FLIP-189] SQL Client prompts customization

fsk119 commented on code in PR #17830:
URL: https://github.com/apache/flink/pull/17830#discussion_r1017909829


##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/PromptHandler.java:
##########
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.client.cli;
+
+import org.apache.flink.table.client.config.SqlClientOptions;
+import org.apache.flink.table.client.gateway.Executor;
+import org.apache.flink.table.utils.ThreadLocalCache;
+
+import org.jline.terminal.Terminal;
+import org.jline.utils.AttributedStringBuilder;
+import org.jline.utils.AttributedStyle;
+import org.jline.utils.StyleResolver;
+
+import java.text.SimpleDateFormat;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+import java.util.function.Supplier;
+
+/**
+ * Prompt handler class which allows customization for the prompt shown at the start (left prompt)
+ * and the end (right prompt) of each line.
+ */
+public class PromptHandler {
+    private static final char ESCAPE_BACKSLASH = '\\';
+    private static final Map<Character, String> DATE_TIME_FORMATS;
+    private static final ThreadLocalCache<String, SimpleDateFormat> FORMATTER_CACHE =

Review Comment:
   Why use ThreadLocalCache here? The client always serves one thread here. I think HashMap is enough here.



##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/PromptHandler.java:
##########
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.client.cli;
+
+import org.apache.flink.table.client.config.SqlClientOptions;
+import org.apache.flink.table.client.gateway.Executor;
+import org.apache.flink.table.utils.ThreadLocalCache;
+
+import org.jline.terminal.Terminal;
+import org.jline.utils.AttributedStringBuilder;
+import org.jline.utils.AttributedStyle;
+import org.jline.utils.StyleResolver;
+
+import java.text.SimpleDateFormat;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+import java.util.function.Supplier;
+
+/**
+ * Prompt handler class which allows customization for the prompt shown at the start (left prompt)
+ * and the end (right prompt) of each line.
+ */
+public class PromptHandler {
+    private static final char ESCAPE_BACKSLASH = '\\';
+    private static final Map<Character, String> DATE_TIME_FORMATS;
+    private static final ThreadLocalCache<String, SimpleDateFormat> FORMATTER_CACHE =
+            new ThreadLocalCache<String, SimpleDateFormat>() {
+                @Override
+                public SimpleDateFormat getNewInstance(String key) {
+                    return new SimpleDateFormat(key, Locale.ROOT);
+                }
+            };
+
+    static {
+        Map<Character, String> map = new HashMap<>();
+        map.put('D', "yyyy-MM-dd HH:mm:ss.SSS");
+        map.put('m', "mm");
+        map.put('o', "MM");
+        map.put('O', "MMM");
+        map.put('P', "aa");
+        map.put('r', "hh:mm");
+        map.put('R', "HH:mm");
+        map.put('s', "ss");
+        map.put('w', "d");
+        map.put('W', "E");
+        map.put('y', "YY");
+        map.put('Y', "YYYY");
+        DATE_TIME_FORMATS = Collections.unmodifiableMap(map);
+    }
+
+    private static final StyleResolver STYLE_RESOLVER = new StyleResolver(s -> "");
+
+    private final String sessionId;
+    private final Executor executor;
+    private final Supplier<Terminal> terminalSupplier;
+
+    public PromptHandler(String sessionId, Executor executor, Supplier<Terminal> terminalSupplier) {
+        this.sessionId = sessionId;
+        this.executor = executor;
+        this.terminalSupplier = terminalSupplier;
+    }
+
+    public String getPrompt() {
+        return buildPrompt(
+                executor.getSessionConfig(sessionId).get(SqlClientOptions.PROMPT),
+                SqlClientOptions.PROMPT.defaultValue());
+    }
+
+    public String getRightPrompt() {
+        return buildPrompt(
+                executor.getSessionConfig(sessionId).get(SqlClientOptions.RIGHT_PROMPT),
+                SqlClientOptions.RIGHT_PROMPT.defaultValue());
+    }
+
+    private String buildPrompt(String pattern, String defaultValue) {
+        AttributedStringBuilder promptStringBuilder = new AttributedStringBuilder();
+        try {
+            for (int i = 0; i < pattern.length(); i++) {
+                final char c = pattern.charAt(i);
+                if (c == ESCAPE_BACKSLASH) {
+                    if (i == pattern.length() - 1) {
+                        continue;
+                    }
+                    final char nextChar = pattern.charAt(i + 1);
+                    switch (nextChar) {
+                        case 'D':
+                        case 'm':
+                        case 'o':
+                        case 'O':
+                        case 'P':
+                        case 'r':
+                        case 'R':
+                        case 's':
+                        case 'w':
+                        case 'W':
+                        case 'y':
+                        case 'Y':
+                            promptStringBuilder.append(
+                                    FORMATTER_CACHE
+                                            .get(DATE_TIME_FORMATS.get(nextChar))
+                                            .format(new Date()));
+                            i++;
+                            break;
+                        case 'c':
+                            promptStringBuilder.append(executor.getCurrentCatalogName(sessionId));
+                            i++;
+                            break;
+                        case 'd':
+                            promptStringBuilder.append(executor.getCurrentDatabaseName(sessionId));
+                            i++;
+                            break;
+                        case ESCAPE_BACKSLASH:
+                            promptStringBuilder.append(ESCAPE_BACKSLASH);
+                            i++;
+                            break;
+                            // date time pattern \{...\}
+                        case '{':
+                            int dateTimeMaskCloseIndex =
+                                    pattern.indexOf(ESCAPE_BACKSLASH + "}", i + 1);
+                            if (dateTimeMaskCloseIndex > 0) {
+                                String mask = pattern.substring(i + 2, dateTimeMaskCloseIndex);
+                                promptStringBuilder.append(
+                                        FORMATTER_CACHE.get(mask).format(new Date()));
+                                i = dateTimeMaskCloseIndex + 1;
+                            }
+                            break;
+                            // color and style pattern \[...\]
+                        case '[':
+                            int closeBracketIndex = pattern.indexOf(ESCAPE_BACKSLASH + "]", i + 1);
+                            if (closeBracketIndex > 0) {
+                                String color = pattern.substring(i + 2, closeBracketIndex);
+                                AttributedStyle style = STYLE_RESOLVER.resolve(color);
+                                promptStringBuilder.style(style);
+                                i = closeBracketIndex + 1;
+                            }
+                            break;
+                            // property value pattern \:...\:
+                        case ':':
+                            int nextColonIndex = pattern.indexOf(ESCAPE_BACKSLASH + ":", i + 1);
+                            String propertyValue;
+                            if (nextColonIndex > 0) {
+                                String propertyName = pattern.substring(i + 2, nextColonIndex);
+                                propertyValue =
+                                        executor.getSessionConfigMap(sessionId).get(propertyName);
+                                promptStringBuilder.append(propertyValue);
+                                i = nextColonIndex + 1;
+                            }
+                            break;
+                    }
+                } else {
+                    promptStringBuilder.append(c);
+                }
+            }
+            return promptStringBuilder.toAnsi();
+        } catch (IllegalArgumentException e) {
+            terminalSupplier

Review Comment:
   Can we validate this when setting the value? I just doubt user can not continue input if users set a invalid value 



##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/Executor.java:
##########
@@ -149,4 +149,10 @@ TypedResult<Integer> snapshotResult(String sessionId, String resultId, int pageS
     Optional<String> stopJob(
             String sessionId, String jobId, boolean isWithSavepoint, boolean isWithDrain)
             throws SqlExecutionException;
+
+    /** Get current catalog name. */
+    String getCurrentCatalogName(String sessionId);
+
+    /** Get current database name. */
+    String getCurrentDatabaseName(String sessionId);

Review Comment:
   Recently we are supporting the SQL Client to submit SQL to the SQL Gateway. In the remote mode, I think we may need to send a request to the remote SQL Gateway when sql-client prints a promot? If so, I think it may takes some time to get the prompts.



##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/PromptHandler.java:
##########
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.client.cli;
+
+import org.apache.flink.table.client.config.SqlClientOptions;
+import org.apache.flink.table.client.gateway.Executor;
+import org.apache.flink.table.utils.ThreadLocalCache;
+
+import org.jline.terminal.Terminal;
+import org.jline.utils.AttributedStringBuilder;
+import org.jline.utils.AttributedStyle;
+import org.jline.utils.StyleResolver;
+
+import java.text.SimpleDateFormat;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+import java.util.function.Supplier;
+
+/**
+ * Prompt handler class which allows customization for the prompt shown at the start (left prompt)
+ * and the end (right prompt) of each line.
+ */
+public class PromptHandler {
+    private static final char ESCAPE_BACKSLASH = '\\';
+    private static final Map<Character, String> DATE_TIME_FORMATS;
+    private static final ThreadLocalCache<String, SimpleDateFormat> FORMATTER_CACHE =
+            new ThreadLocalCache<String, SimpleDateFormat>() {
+                @Override
+                public SimpleDateFormat getNewInstance(String key) {
+                    return new SimpleDateFormat(key, Locale.ROOT);
+                }
+            };
+
+    static {
+        Map<Character, String> map = new HashMap<>();
+        map.put('D', "yyyy-MM-dd HH:mm:ss.SSS");
+        map.put('m', "mm");
+        map.put('o', "MM");
+        map.put('O', "MMM");
+        map.put('P', "aa");
+        map.put('r', "hh:mm");
+        map.put('R', "HH:mm");
+        map.put('s', "ss");
+        map.put('w', "d");
+        map.put('W', "E");
+        map.put('y', "YY");
+        map.put('Y', "YYYY");
+        DATE_TIME_FORMATS = Collections.unmodifiableMap(map);
+    }
+
+    private static final StyleResolver STYLE_RESOLVER = new StyleResolver(s -> "");
+
+    private final String sessionId;
+    private final Executor executor;
+    private final Supplier<Terminal> terminalSupplier;
+
+    public PromptHandler(String sessionId, Executor executor, Supplier<Terminal> terminalSupplier) {
+        this.sessionId = sessionId;
+        this.executor = executor;
+        this.terminalSupplier = terminalSupplier;
+    }
+
+    public String getPrompt() {
+        return buildPrompt(
+                executor.getSessionConfig(sessionId).get(SqlClientOptions.PROMPT),
+                SqlClientOptions.PROMPT.defaultValue());
+    }
+
+    public String getRightPrompt() {
+        return buildPrompt(
+                executor.getSessionConfig(sessionId).get(SqlClientOptions.RIGHT_PROMPT),
+                SqlClientOptions.RIGHT_PROMPT.defaultValue());
+    }
+
+    private String buildPrompt(String pattern, String defaultValue) {
+        AttributedStringBuilder promptStringBuilder = new AttributedStringBuilder();
+        try {
+            for (int i = 0; i < pattern.length(); i++) {
+                final char c = pattern.charAt(i);
+                if (c == ESCAPE_BACKSLASH) {
+                    if (i == pattern.length() - 1) {
+                        continue;
+                    }
+                    final char nextChar = pattern.charAt(i + 1);
+                    switch (nextChar) {
+                        case 'D':
+                        case 'm':
+                        case 'o':
+                        case 'O':
+                        case 'P':
+                        case 'r':
+                        case 'R':
+                        case 's':
+                        case 'w':
+                        case 'W':
+                        case 'y':
+                        case 'Y':
+                            promptStringBuilder.append(

Review Comment:
   When the pattern is "cd", the users may get "default_catalogdefault_database" now? If so, it may not be very easy for users to read?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org