You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2018/10/18 00:30:05 UTC

[GitHub] kkhatua closed pull request #1491: DRILL-6084: Show Drill functions in WebUI for autocomplete

kkhatua closed pull request #1491: DRILL-6084: Show Drill functions in WebUI for autocomplete
URL: https://github.com/apache/drill/pull/1491
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/exec/java-exec/src/main/codegen/templates/CastEmptyStringVarTypesToNullableNumeric.java b/exec/java-exec/src/main/codegen/templates/CastEmptyStringVarTypesToNullableNumeric.java
index e0942e9df73..65127715e39 100644
--- a/exec/java-exec/src/main/codegen/templates/CastEmptyStringVarTypesToNullableNumeric.java
+++ b/exec/java-exec/src/main/codegen/templates/CastEmptyStringVarTypesToNullableNumeric.java
@@ -48,7 +48,7 @@
  */
 
 @SuppressWarnings("unused")
-@FunctionTemplate(name = "castEmptyString${type.from}To${type.to?upper_case}", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.INTERNAL)
+@FunctionTemplate(name = "castEmptyString${type.from}To${type.to?upper_case}", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.INTERNAL, isInternal=true)
 public class CastEmptyString${type.from}To${type.to} implements DrillSimpleFunc{
 
     @Param ${type.from}Holder in;
diff --git a/exec/java-exec/src/main/codegen/templates/ConvertToNullableHolder.java b/exec/java-exec/src/main/codegen/templates/ConvertToNullableHolder.java
index 7615bc5def8..875038c3d47 100644
--- a/exec/java-exec/src/main/codegen/templates/ConvertToNullableHolder.java
+++ b/exec/java-exec/src/main/codegen/templates/ConvertToNullableHolder.java
@@ -42,7 +42,8 @@
     <#elseif minor.class.startsWith("Var")>
     returnType = FunctionTemplate.ReturnType.SAME_IN_OUT_LENGTH,
     </#if>
-    nulls = FunctionTemplate.NullHandling.INTERNAL)
+    nulls = FunctionTemplate.NullHandling.INTERNAL,
+    isInternal = true)
 public class ${className} implements DrillSimpleFunc {
 
   @Param ${minor.class}Holder input;
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/FunctionTemplate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/FunctionTemplate.java
index 11914ea9fde..3aa73650b7b 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/FunctionTemplate.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/FunctionTemplate.java
@@ -90,6 +90,11 @@
   boolean isNiladic() default false;
   boolean checkPrecisionRange() default false;
 
+  /**
+   * Defines if a function is internal and not intended for public use [e.g. castEmptyStringNullableVarBinaryToNullableVarDecimal(..) ]
+   */
+  boolean isInternal() default false;
+
   /**
    * This enum will be used to estimate the average size of the output
    * produced by a function that produces variable length output
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java
index d6f6ea21b0b..fc021c95032 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java
@@ -107,6 +107,10 @@ public boolean isNiladic() {
   }
 
 
+  public boolean isInternal() {
+    return attributes.isInternal();
+  }
+
   /**
    * Generates string representation of function input parameters:
    * PARAMETER_TYPE_1-PARAMETER_MODE_1,PARAMETER_TYPE_2-PARAMETER_MODE_2
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionAttributes.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionAttributes.java
index 6d1b7678288..5f744e58962 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionAttributes.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionAttributes.java
@@ -103,5 +103,9 @@ public boolean isNiladic() {
     return template.isNiladic();
   }
 
+  public boolean isInternal() {
+    return template.isInternal();
+  }
+
   public boolean checkPrecisionRange() { return template.checkPrecisionRange(); }
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java
index a6f9a7f7ca7..5621c44caed 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java
@@ -293,6 +293,10 @@ public boolean isFunctionComplexOutput(String name) {
     return false;
   }
 
+  public LocalFunctionRegistry getLocalFunctionRegistry() {
+    return localFunctionRegistry;
+  }
+
   public RemoteFunctionRegistry getRemoteFunctionRegistry() {
     return remoteFunctionRegistry;
   }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ValueReference.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ValueReference.java
index 42d39674bab..1ee39ff0095 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ValueReference.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ValueReference.java
@@ -28,6 +28,7 @@
   private boolean isConstant = false;
   private boolean isFieldReader = false;
   private boolean isComplexWriter = false;
+  private boolean isInternal = false;
 
   public ValueReference(MajorType type, String name) {
     Preconditions.checkNotNull(type);
@@ -52,6 +53,14 @@ public boolean isConstant() {
     return isConstant;
   }
 
+  public void setInternal(boolean isInternal) {
+    this.isInternal = isInternal;
+  }
+
+  public boolean isInternal() {
+    return isInternal;
+  }
+
   public boolean isFieldReader() {
     return isFieldReader;
   }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
index fd9950e6c4c..8cab4ddd474 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
@@ -20,8 +20,6 @@
 import com.codahale.metrics.MetricRegistry;
 import com.codahale.metrics.servlets.MetricsServlet;
 import com.codahale.metrics.servlets.ThreadDumpServlet;
-import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
-import org.apache.drill.shaded.guava.com.google.common.io.Files;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang3.RandomStringUtils;
@@ -32,6 +30,8 @@
 import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.exec.ssl.SSLConfig;
 import org.apache.drill.exec.exception.DrillbitStartupException;
+import org.apache.drill.exec.expr.fn.registry.FunctionHolder;
+import org.apache.drill.exec.expr.fn.registry.LocalFunctionRegistry;
 import org.apache.drill.exec.server.BootStrapContext;
 import org.apache.drill.exec.server.Drillbit;
 import org.apache.drill.exec.server.options.OptionList;
@@ -84,20 +84,34 @@
 import java.io.InputStream;
 import java.math.BigInteger;
 import java.net.BindException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.security.KeyPair;
 import java.security.KeyPairGenerator;
 import java.security.KeyStore;
 import java.security.SecureRandom;
 import java.security.cert.X509Certificate;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
 import java.util.EnumSet;
 import java.util.List;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  * Wrapper class around jetty based webserver.
  */
 public class WebServer implements AutoCloseable {
+  private static final String ACE_MODE_SQL_TEMPLATE_JS = "ace.mode-sql.template.js";
+  private static final String ACE_MODE_SQL_JS = "mode-sql.js";
+  private static final String DRILL_FUNCTIONS_PLACEHOLDER = "__DRILL_FUNCTIONS__";
+
+  private static final String STATUS_THREADS_PATH = "/status/threads";
+  private static final String STATUS_METRICS_PATH = "/status/metrics";
+
   private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(WebServer.class);
   private static final String OPTIONS_DESCRIBE_JS = "options.describe.js";
   private static final String OPTIONS_DESCRIBE_TEMPLATE_JS = "options.describe.template.js";
@@ -117,11 +131,12 @@
 
   public File getTmpJavaScriptDir() {
     if (tmpJavaScriptDir == null) {
-      tmpJavaScriptDir = Files.createTempDir();
+      tmpJavaScriptDir = org.apache.drill.shaded.guava.com.google.common.io.Files.createTempDir();
       tmpJavaScriptDir.deleteOnExit();
       //Perform All auto generated files at this point
       try {
         generateOptionsDescriptionJSFile();
+        generateFunctionJS();
       } catch (IOException e) {
         logger.error("Unable to create temp dir for JavaScripts. {}", e);
       }
@@ -177,7 +192,7 @@ public void start() throws Exception {
     FilterHolder filterHolder = new FilterHolder(CrossOriginFilter.class);
     filterHolder.setInitParameter("allowedOrigins", "*");
     //Allowing CORS for metrics only
-    webServerContext.addFilter(filterHolder, "/status/metrics", null);
+    webServerContext.addFilter(filterHolder, STATUS_METRICS_PATH, null);
     embeddedJetty.setHandler(webServerContext);
 
     ServerConnector connector = createConnector(port, acceptors, selectors);
@@ -215,8 +230,8 @@ private ServletContextHandler createServletContextHandler(final boolean authEnab
     servletHolder.setInitOrder(1);
     servletContextHandler.addServlet(servletHolder, "/*");
 
-    servletContextHandler.addServlet(new ServletHolder(new MetricsServlet(metrics)), "/status/metrics");
-    servletContextHandler.addServlet(new ServletHolder(new ThreadDumpServlet()), "/status/threads");
+    servletContextHandler.addServlet(new ServletHolder(new MetricsServlet(metrics)), STATUS_METRICS_PATH);
+    servletContextHandler.addServlet(new ServletHolder(new ThreadDumpServlet()), STATUS_THREADS_PATH);
 
     final ServletHolder staticHolder = new ServletHolder("static", DefaultServlet.class);
     // Get resource URL for Drill static assets, based on where Drill icon is located
@@ -456,14 +471,15 @@ public void close() throws Exception {
     FileUtils.deleteDirectory(getTmpJavaScriptDir());
   }
 
-  private static final String FILE_CONTENT_FOOTER = "};";
-
-  //Generate Options Description JavaScript
+  /**
+   * Generate Options Description JavaScript to serve http://drillhost/options ACE library search features
+   * @throws IOException
+   */
   private void generateOptionsDescriptionJSFile() throws IOException {
     //Obtain list of Options & their descriptions
     OptionManager optionManager = this.drillbit.getContext().getOptionManager();
     OptionList publicOptions = optionManager.getPublicOptionList();
-    List<OptionValue> options = Lists.newArrayList(publicOptions);
+    List<OptionValue> options = new ArrayList<>(publicOptions);
     Collections.sort(options);
     int numLeftToWrite = options.size();
 
@@ -471,6 +487,7 @@ private void generateOptionsDescriptionJSFile() throws IOException {
     InputStream optionsDescripTemplateStream = Resource.newClassPathResource(OPTIONS_DESCRIBE_TEMPLATE_JS).getInputStream();
     //Generated file
     File optionsDescriptionFile = new File(getTmpJavaScriptDir(), OPTIONS_DESCRIBE_JS);
+    final String file_content_footer = "};";
     optionsDescriptionFile.deleteOnExit();
     //Create a copy of a template and write with that!
     java.nio.file.Files.copy(optionsDescripTemplateStream, optionsDescriptionFile.toPath());
@@ -490,9 +507,56 @@ private void generateOptionsDescriptionJSFile() throws IOException {
           writer.newLine();
         }
       }
-      writer.append(FILE_CONTENT_FOOTER);
+      writer.append(file_content_footer);
       writer.newLine();
       writer.flush();
     }
   }
+
+  /**
+   * Generates ACE library javascript populated with list of available SQL functions
+   * @throws IOException
+   */
+  private void generateFunctionJS() throws IOException {
+    //Naturally ordered set of function names
+    TreeSet<String> functionSet = new TreeSet<>();
+    //Extracting ONLY builtIn functions (i.e those already available)
+    List<FunctionHolder> builtInFuncHolderList = this.drillbit.getContext().getFunctionImplementationRegistry().getLocalFunctionRegistry()
+        .getAllJarsWithFunctionsHolders().get(LocalFunctionRegistry.BUILT_IN);
+
+    //Build List of 'usable' functions (i.e. functions that start with an alphabet and can be autocompleted by the ACE library)
+    //Example of 'unusable' functions would be operators like '<', '!'
+    int skipCount = 0;
+    for (FunctionHolder builtInFunctionHolder : builtInFuncHolderList) {
+      String name = builtInFunctionHolder.getName();
+      if (!name.contains(" ") && name.matches("([a-z]|[A-Z])\\w+") && !builtInFunctionHolder.getHolder().isInternal()) {
+        functionSet.add(name);
+      } else {
+        logger.debug("Non-alphabetic leading character. Function skipped : {} ", name);
+        skipCount++;
+      }
+    }
+    logger.debug("{} functions will not be available in WebUI", skipCount);
+
+    //Generated file
+    File functionsListFile = new File(getTmpJavaScriptDir(), ACE_MODE_SQL_JS);
+    functionsListFile.deleteOnExit();
+    //Template source Javascript file
+    try (InputStream aceModeSqlTemplateStream = Resource.newClassPathResource(ACE_MODE_SQL_TEMPLATE_JS).getInputStream()) {
+      //Create a copy of a template and write with that!
+      java.nio.file.Files.copy(aceModeSqlTemplateStream, functionsListFile.toPath());
+    }
+
+    //Construct String
+    String funcListString = String.join("|", functionSet);
+
+    Path path = Paths.get(functionsListFile.getPath());
+    try (Stream<String> lines = Files.lines(path)) {
+      List <String> replaced =
+          lines //Replacing first occurrence
+            .map(line -> line.replaceFirst(DRILL_FUNCTIONS_PLACEHOLDER, funcListString))
+            .collect(Collectors.toList());
+      Files.write(path, replaced);
+    }
+  }
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/FunctionsIterator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/FunctionsIterator.java
index 86e88174e8d..63798a29058 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/FunctionsIterator.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/FunctionsIterator.java
@@ -87,11 +87,12 @@ private void populateFunctionMap(Map<String, FunctionInfo> functionMap, String j
     String registeredNames[] = dfh.getRegisteredNames();
     String signature = dfh.getInputParameters();
     String returnType = dfh.getReturnType().getMinorType().toString();
+    boolean isInternal = dfh.isInternal();
     for (String name : registeredNames) {
       //Generate a unique key for a function holder as 'functionName#functionSignature'
       //Bumping capacity from default 16 to 64 (since key is expected to be bigger, and reduce probability of resizing)
       String funcSignatureKey = new StringBuilder(64).append(name).append('#').append(signature).toString();
-      functionMap.put(funcSignatureKey, new FunctionInfo(name, signature, returnType, jarName));
+      functionMap.put(funcSignatureKey, new FunctionInfo(name, signature, returnType, jarName, isInternal));
     }
   }
 
@@ -117,12 +118,19 @@ public FunctionInfo next() {
     public final String returnType;
     @NonNullable
     public final String source;
+    @NonNullable
+    public final boolean internal;
 
     public FunctionInfo(String funcName, String funcSignature, String funcReturnType, String jarName) {
+      this(funcName, funcSignature, funcReturnType, jarName, false);
+    }
+
+    public FunctionInfo(String funcName, String funcSignature, String funcReturnType, String jarName, boolean isInternal) {
       this.name = funcName;
       this.signature = funcSignature;
       this.returnType = funcReturnType;
       this.source = jarName;
+      this.internal = isInternal;
     }
   }
 }
diff --git a/exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/mode-sql.js b/exec/java-exec/src/main/resources/ace.mode-sql.template.js
similarity index 80%
rename from exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/mode-sql.js
rename to exec/java-exec/src/main/resources/ace.mode-sql.template.js
index ceb47c8f047..057d8dffe88 100644
--- a/exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/mode-sql.js
+++ b/exec/java-exec/src/main/resources/ace.mode-sql.template.js
@@ -31,22 +31,9 @@ var SqlHighlightRules = function() {
         "true|false"
     );
 
-    //Drill-specific
+    //Drill-specific (auto-generated: See DRILL-6084)
     var builtinFunctions = (
-        //Math and Trignometric
-        "abs|cbrt|ceil|ceiling|degrees|e|exp|floor|log|log|log10|lshift|mod|negative|pi|pow|radians|rand|round|round|rshift|sign|sqrt|trunc|trunc|" +
-        "sin|cos|tan|asin|acos|atan|sinh|cosh|tanh|" +
-        //datatype conversion
-        "cast|convert_to|convert_from|string_binary|binary_string|" +
-        //time-conversion
-        "to_char|to_date|to_number|to_timestamp|to_timestamp|" +
-        "age|extract|current_date|current_time|current_timestamp|date_add|date_part|date_sub|localtime|localtimestamp|now|timeofday|unix_timestamp|" +
-        //string manipulation
-        "byte_substr|char_length|concat|ilike|initcap|length|lower|lpad|ltrim|position|regexp_replace|rpad|rtrim|strpos|substr|trim|upper|" +
-        //statistical
-        "avg|count|max|min|sum|stddev|stddev_pop|stddev_samp|variance|var_pop|var_samp|" +
-        //null-handling
-        "coalesce|nullif"
+        "__DRILL_FUNCTIONS__"
     );
 
     //Drill-specific
diff --git a/exec/java-exec/src/main/resources/rest/query/query.ftl b/exec/java-exec/src/main/resources/rest/query/query.ftl
index 936b1a53c63..078333e3f8e 100644
--- a/exec/java-exec/src/main/resources/rest/query/query.ftl
+++ b/exec/java-exec/src/main/resources/rest/query/query.ftl
@@ -25,7 +25,8 @@
     </#if>
   <!-- Ace Libraries for Syntax Formatting -->
   <script src="/static/js/ace-code-editor/ace.js" type="text/javascript" charset="utf-8"></script>
-  <script src="/static/js/ace-code-editor/mode-sql.js" type="text/javascript" charset="utf-8"></script>
+  <!-- Disabled in favour of dynamic: script src="/static/js/ace-code-editor/mode-sql.js" type="text/javascript" charset="utf-8" -->
+  <script src="/dynamic/mode-sql.js" type="text/javascript" charset="utf-8"></script>
   <script src="/static/js/ace-code-editor/ext-language_tools.js" type="text/javascript" charset="utf-8"></script>
   <script src="/static/js/ace-code-editor/theme-sqlserver.js" type="text/javascript" charset="utf-8"></script>
   <script src="/static/js/ace-code-editor/snippets/sql.js" type="text/javascript" charset="utf-8"></script>
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/sys/TestSystemTable.java b/exec/java-exec/src/test/java/org/apache/drill/exec/store/sys/TestSystemTable.java
index 5c9990a4b63..2ed5f6ea5c4 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/store/sys/TestSystemTable.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/sys/TestSystemTable.java
@@ -81,6 +81,13 @@ public void functionsTable() throws Exception {
     test("select * from sys.functions");
   }
 
+  @Test
+  public void testInternalFunctionsTable() throws Exception {
+    String query = "select internal, count(*) from sys.functions group by internal";
+    //Testing a mix of public and internal functions defined in FunctionTemplate
+    assertEquals(2, testSql(query));
+  }
+
   @Test
   public void profilesTable() throws Exception {
     test("select * from sys.profiles");
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/work/metadata/TestMetadataProvider.java b/exec/java-exec/src/test/java/org/apache/drill/exec/work/metadata/TestMetadataProvider.java
index ce58cb05af3..2e785b73a11 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/work/metadata/TestMetadataProvider.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/work/metadata/TestMetadataProvider.java
@@ -240,7 +240,7 @@ public void columns() throws Exception {
 
     assertEquals(RequestStatus.OK, resp.getStatus());
     List<ColumnMetadata> columns = resp.getColumnsList();
-    assertEquals(139, columns.size());
+    assertEquals(140, columns.size());
     // too many records to verify the output.
   }
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services