You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by kkhatua <gi...@git.apache.org> on 2018/01/06 01:55:47 UTC

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

GitHub user kkhatua opened a pull request:

    https://github.com/apache/drill/pull/1084

    DRILL-5868: Support SQL syntax highlighting of queries

    Based on the commit for DRILL-5981 (PR #1043), this commit further leverages the Ace JavaScript library with customizations specific to Drill.
    
    This commit introduces the following to Drill:
    1. Syntax highlighting (This is also supported for submitted query profiles)
    2. Autocomplete supported in all SQL editors (including the Edit Query tab within an existing profile to rerunning the query)
    3. Specifying Drill specific keywords and functions in visible autocomplete
    4. Key snippets (template SQLs) allowing for rapid writing of syntax:
      i. Query System Tables
      ii. CView, CTAS and CTempTAS
      iii. Alter Session
      iv. Explain and Select * queries
    
    NOTE: The lists for items 3 and 4 are not exhaustive. As more features are added to Drill, these lists can be expanded.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/kkhatua/drill DRILL-5868

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/drill/pull/1084.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1084
    
----
commit 7cc7e3e70e9ab757fd73dd54755e733f9b3cf696
Author: Kunal Khatua <kk...@...>
Date:   2018-01-06T01:52:46Z

    DRILL-5868: Support SQL syntax highlighting of queries
    
    Based on the commit for DRILL-5981 (PR #1043), this commit further leverages the Ace JavaScript library with customizations specific to Drill.
    
    This commit introduces the following to the Query Editor (including the Edit Query tab within an existing profile to rerunning the query).
    1. Syntax highlighting (This is supported for submitted query profiles
    2. Autocomplete supported in editors
    3. Specifying Drill specific keywords and functions in visible autocomplete
    4. Key snippets (template SQLs) allowing for rapid writing of syntax:
      i. Query System Tables
      ii. CView, CTAS and CTempTAS
      iii. Alter Session
      iv. Explain and Select * queries
    
    NOTE: The lists for #3 and #4 are not exhaustive. As more features are added to Drill, these lists can be expanded.

----


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r160457025
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js ---
    @@ -0,0 +1,46 @@
    +/**
    + * Drill SQL Syntax Snippets
    + */
    +
    +ace.define("ace/snippets/sql",["require","exports","module"], function(require, exports, module) {
    +"use strict";
    +
    +exports.snippetText = "snippet info\n\
    +	select * from INFORMATION_SCHEMA.${1:<tableName>};\n\
    +snippet sysmem\n\
    --- End diff --
    
    Also snippets are case-sensitive. Is it common practice?


---

[GitHub] drill issue #1084: DRILL-5868: Support SQL syntax highlighting of queries

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:

    https://github.com/apache/drill/pull/1084
  
    @arina-ielchiieva updated the changes based on your review. Please have a look.


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r162329512
  
    --- Diff: exec/java-exec/src/main/resources/rest/profile/profile.ftl ---
    @@ -77,16 +84,17 @@ table.sortable thead .sorting_desc { background-image: url("/static/img/black-de
           <p>
     
             <#if model.isOnlyImpersonationEnabled()>
    +          <div id="query-editor" class="form-group">${model.getProfile().query}</div>
    +          <input class="form-control" id="query" name="query" type="hidden" value="${model.getProfile().query}"/>
               <div class="form-group">
                 <label for="userName">User Name</label>
                 <input type="text" size="30" name="userName" id="userName" placeholder="User Name" value="${model.getProfile().user}">
               </div>
             </#if>
     
             <form role="form" id="queryForm" action="/query" method="POST">
    -          <div class="form-group">
    -            <textarea class="form-control" id="query" name="query" style="font-family: Courier;">${model.getProfile().query}</textarea>
    -          </div>
    +          <div id="query-editor" class="form-group">${model.getProfile().query}</div>
    +          <input class="form-control" id="query" name="query" type="hidden" value="${model.getProfile().query}"/>
    --- End diff --
    
    It looks like you have the same code on lines 87-88. These code lines are included when only impersonation is enabled. So if I am not mistaken, you'll end up with two query editors when only impersonation is enabled. Please check.


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r160452751
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/mode-sql.js ---
    @@ -1 +1,128 @@
    -ace.define("ace/mode/sql_highlight_rules",["require","exports","module","ace/lib/oop","ace/mode/text_highlight_rules"],function(e,t,n){"use strict";var r=e("../lib/oop"),i=e("./text_highlight_rules").TextHighlightRules,s=function(){var e="select|insert|update|delete|from|where|and|or|group|by|order|limit|offset|having|as|case|when|else|end|type|left|right|join|on|outer|desc|asc|union|create|table|primary|key|if|foreign|not|references|default|null|inner|cross|natural|database|drop|grant",t="true|false",n="avg|count|first|last|max|min|sum|ucase|lcase|mid|len|round|rank|now|format|coalesce|ifnull|isnull|nvl",r="int|numeric|decimal|date|varchar|char|bigint|float|double|bit|binary|text|set|timestamp|money|real|number|integer",i=this.createKeywordMapper({"support.function":n,keyword:e,"constant.language":t,"storage.type":r},"identifier",!0);this.$rules={start:[{token:"comment",regex:"--.*$"},{token:"comment",start:"/\\*",end:"\\*/"},{token:"string",regex:'".*?"'},{token:"string",regex
 :"'.*?'"},{token:"string",regex:"`.*?`"},{token:"constant.numeric",regex:"[+-]?\\d+(?:(?:\\.\\d*)?(?:[eE][+-]?\\d+)?)?\\b"},{token:i,regex:"[a-zA-Z_$][a-zA-Z0-9_$]*\\b"},{token:"keyword.operator",regex:"\\+|\\-|\\/|\\/\\/|%|<@>|@>|<@|&|\\^|~|<|>|<=|=>|==|!=|<>|="},{token:"paren.lparen",regex:"[\\(]"},{token:"paren.rparen",regex:"[\\)]"},{token:"text",regex:"\\s+"}]},this.normalizeRules()};r.inherits(s,i),t.SqlHighlightRules=s}),ace.define("ace/mode/sql",["require","exports","module","ace/lib/oop","ace/mode/text","ace/mode/sql_highlight_rules"],function(e,t,n){"use strict";var r=e("../lib/oop"),i=e("./text").Mode,s=e("./sql_highlight_rules").SqlHighlightRules,o=function(){this.HighlightRules=s,this.$behaviour=this.$defaultBehaviour};r.inherits(o,i),function(){this.lineCommentStart="--",this.$id="ace/mode/sql"}.call(o.prototype),t.Mode=o})
    \ No newline at end of file
    +/**
    + * Drill SQL Definition (Forked from SqlServer definition)
    + */
    +
    +ace.define("ace/mode/sql_highlight_rules",["require","exports","module","ace/lib/oop","ace/mode/text_highlight_rules"], function(require, exports, module) {
    +"use strict";
    +
    +var oop = require("../lib/oop");
    +var TextHighlightRules = require("./text_highlight_rules").TextHighlightRules;
    +
    +var SqlHighlightRules = function() {
    +
    +    //TODO: https://drill.apache.org/docs/reserved-keywords/
    +	//e.g. Cubing operators like ROLLUP are not listed
    +    var keywords = (
    +        "select|insert|update|delete|from|where|and|or|group|by|order|limit|offset|having|as|case|" +
    +        "when|else|end|type|left|right|join|on|outer|desc|asc|union|create|table|key|if|" +
    +        "not|default|null|inner|database|drop|" +
    +		"flatten|kvgen|columns"
    +    );
    +	//Confirmed to be UnSupported as of Drill-1.12.0
    +	/* cross|natural|primary|foreign|references|grant */
    +
    +    var builtinConstants = (
    +        "true|false"
    +    );
    +
    +    //Drill-specific
    +    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|" +
    --- End diff --
    
    I believe n future we'll have system table with Drill functions. Will be there a possibility to pass list of functions to the js script instead of hard-coding them?


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r160650903
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js ---
    @@ -0,0 +1,46 @@
    +/**
    + * Drill SQL Syntax Snippets
    + */
    +
    +ace.define("ace/snippets/sql",["require","exports","module"], function(require, exports, module) {
    +"use strict";
    +
    +exports.snippetText = "snippet info\n\
    +	select * from INFORMATION_SCHEMA.${1:<tableName>};\n\
    +snippet sysmem\n\
    +	select * from sys.mem;\n\
    +snippet sysopt\n\
    +	select * from sys.opt;\n\
    +snippet sysbit\n\
    +	select * from sys.bit;\n\
    +snippet sysconn\n\
    +	select * from sys.conn;\n\
    +snippet sysprof\n\
    +	select * from sys.prof;\n\
    +snippet cview\n\
    --- End diff --
    
    I am not sure that snippets for sys tables are correct. Please see below list of current tables present in sys schema:
    ```
    0: jdbc:drill:drillbit=localhost> show tables;
    +---------------+-----------------------+
    | TABLE_SCHEMA  |      TABLE_NAME       |
    +---------------+-----------------------+
    | sys           | profiles_json         |
    | sys           | drillbits             |
    | sys           | boot                  |
    | sys           | internal_options      |
    | sys           | threads               |
    | sys           | options_val           |
    | sys           | profiles              |
    | sys           | connections           |
    | sys           | internal_options_val  |
    | sys           | memory                |
    | sys           | version               |
    | sys           | options               |
    ```


---

[GitHub] drill issue #1084: DRILL-5868: Support SQL syntax highlighting of queries

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the issue:

    https://github.com/apache/drill/pull/1084
  
    +1, LGTM.


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r160491764
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/mode-sql.js ---
    @@ -1 +1,128 @@
    -ace.define("ace/mode/sql_highlight_rules",["require","exports","module","ace/lib/oop","ace/mode/text_highlight_rules"],function(e,t,n){"use strict";var r=e("../lib/oop"),i=e("./text_highlight_rules").TextHighlightRules,s=function(){var e="select|insert|update|delete|from|where|and|or|group|by|order|limit|offset|having|as|case|when|else|end|type|left|right|join|on|outer|desc|asc|union|create|table|primary|key|if|foreign|not|references|default|null|inner|cross|natural|database|drop|grant",t="true|false",n="avg|count|first|last|max|min|sum|ucase|lcase|mid|len|round|rank|now|format|coalesce|ifnull|isnull|nvl",r="int|numeric|decimal|date|varchar|char|bigint|float|double|bit|binary|text|set|timestamp|money|real|number|integer",i=this.createKeywordMapper({"support.function":n,keyword:e,"constant.language":t,"storage.type":r},"identifier",!0);this.$rules={start:[{token:"comment",regex:"--.*$"},{token:"comment",start:"/\\*",end:"\\*/"},{token:"string",regex:'".*?"'},{token:"string",regex
 :"'.*?'"},{token:"string",regex:"`.*?`"},{token:"constant.numeric",regex:"[+-]?\\d+(?:(?:\\.\\d*)?(?:[eE][+-]?\\d+)?)?\\b"},{token:i,regex:"[a-zA-Z_$][a-zA-Z0-9_$]*\\b"},{token:"keyword.operator",regex:"\\+|\\-|\\/|\\/\\/|%|<@>|@>|<@|&|\\^|~|<|>|<=|=>|==|!=|<>|="},{token:"paren.lparen",regex:"[\\(]"},{token:"paren.rparen",regex:"[\\)]"},{token:"text",regex:"\\s+"}]},this.normalizeRules()};r.inherits(s,i),t.SqlHighlightRules=s}),ace.define("ace/mode/sql",["require","exports","module","ace/lib/oop","ace/mode/text","ace/mode/sql_highlight_rules"],function(e,t,n){"use strict";var r=e("../lib/oop"),i=e("./text").Mode,s=e("./sql_highlight_rules").SqlHighlightRules,o=function(){this.HighlightRules=s,this.$behaviour=this.$defaultBehaviour};r.inherits(o,i),function(){this.lineCommentStart="--",this.$id="ace/mode/sql"}.call(o.prototype),t.Mode=o})
    \ No newline at end of file
    +/**
    + * Drill SQL Definition (Forked from SqlServer definition)
    + */
    +
    +ace.define("ace/mode/sql_highlight_rules",["require","exports","module","ace/lib/oop","ace/mode/text_highlight_rules"], function(require, exports, module) {
    +"use strict";
    +
    +var oop = require("../lib/oop");
    +var TextHighlightRules = require("./text_highlight_rules").TextHighlightRules;
    +
    +var SqlHighlightRules = function() {
    +
    +    //TODO: https://drill.apache.org/docs/reserved-keywords/
    +	//e.g. Cubing operators like ROLLUP are not listed
    +    var keywords = (
    +        "select|insert|update|delete|from|where|and|or|group|by|order|limit|offset|having|as|case|" +
    +        "when|else|end|type|left|right|join|on|outer|desc|asc|union|create|table|key|if|" +
    +        "not|default|null|inner|database|drop|" +
    +		"flatten|kvgen|columns"
    +    );
    +	//Confirmed to be UnSupported as of Drill-1.12.0
    +	/* cross|natural|primary|foreign|references|grant */
    +
    +    var builtinConstants = (
    +        "true|false"
    +    );
    +
    +    //Drill-specific
    +    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|" +
    --- End diff --
    
    Sounds good, could you please then just file a Jira for this improvement?


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r161350730
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js ---
    @@ -0,0 +1,46 @@
    +/**
    + * Drill SQL Syntax Snippets
    + */
    +
    +ace.define("ace/snippets/sql",["require","exports","module"], function(require, exports, module) {
    +"use strict";
    +
    +exports.snippetText = "snippet info\n\
    +	select * from INFORMATION_SCHEMA.${1:<tableName>};\n\
    +snippet sysmem\n\
    +	select * from sys.mem;\n\
    +snippet sysopt\n\
    +	select * from sys.opt;\n\
    +snippet sysbit\n\
    +	select * from sys.bit;\n\
    +snippet sysconn\n\
    +	select * from sys.conn;\n\
    +snippet sysprof\n\
    +	select * from sys.prof;\n\
    +snippet cview\n\
    +	create view ${1:[workspace]}.${2:<viewName>} ( ${3:<columnName>} )  as \n\
    +	${4:<query>};\n\
    +snippet ctas\n\
    +	create table ${1:<tableName>} ( ${2:<columnName>} )  as \n\
    +	${3:<query>};\n\
    +snippet ctemp\n\
    +	create temporary table ${1:<tableName>} ( ${2:<columnName>} )  as \n\
    --- End diff --
    
    Done.


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r160485223
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js ---
    @@ -0,0 +1,46 @@
    +/**
    + * Drill SQL Syntax Snippets
    + */
    +
    +ace.define("ace/snippets/sql",["require","exports","module"], function(require, exports, module) {
    --- End diff --
    
    +1
    I did see this initially, but ignored it as I made progress. Guess I got trained to ignoring the contents in the end :)
    Will fix this.


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r160482817
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/mode-sql.js ---
    @@ -1 +1,128 @@
    -ace.define("ace/mode/sql_highlight_rules",["require","exports","module","ace/lib/oop","ace/mode/text_highlight_rules"],function(e,t,n){"use strict";var r=e("../lib/oop"),i=e("./text_highlight_rules").TextHighlightRules,s=function(){var e="select|insert|update|delete|from|where|and|or|group|by|order|limit|offset|having|as|case|when|else|end|type|left|right|join|on|outer|desc|asc|union|create|table|primary|key|if|foreign|not|references|default|null|inner|cross|natural|database|drop|grant",t="true|false",n="avg|count|first|last|max|min|sum|ucase|lcase|mid|len|round|rank|now|format|coalesce|ifnull|isnull|nvl",r="int|numeric|decimal|date|varchar|char|bigint|float|double|bit|binary|text|set|timestamp|money|real|number|integer",i=this.createKeywordMapper({"support.function":n,keyword:e,"constant.language":t,"storage.type":r},"identifier",!0);this.$rules={start:[{token:"comment",regex:"--.*$"},{token:"comment",start:"/\\*",end:"\\*/"},{token:"string",regex:'".*?"'},{token:"string",regex
 :"'.*?'"},{token:"string",regex:"`.*?`"},{token:"constant.numeric",regex:"[+-]?\\d+(?:(?:\\.\\d*)?(?:[eE][+-]?\\d+)?)?\\b"},{token:i,regex:"[a-zA-Z_$][a-zA-Z0-9_$]*\\b"},{token:"keyword.operator",regex:"\\+|\\-|\\/|\\/\\/|%|<@>|@>|<@|&|\\^|~|<|>|<=|=>|==|!=|<>|="},{token:"paren.lparen",regex:"[\\(]"},{token:"paren.rparen",regex:"[\\)]"},{token:"text",regex:"\\s+"}]},this.normalizeRules()};r.inherits(s,i),t.SqlHighlightRules=s}),ace.define("ace/mode/sql",["require","exports","module","ace/lib/oop","ace/mode/text","ace/mode/sql_highlight_rules"],function(e,t,n){"use strict";var r=e("../lib/oop"),i=e("./text").Mode,s=e("./sql_highlight_rules").SqlHighlightRules,o=function(){this.HighlightRules=s,this.$behaviour=this.$defaultBehaviour};r.inherits(o,i),function(){this.lineCommentStart="--",this.$id="ace/mode/sql"}.call(o.prototype),t.Mode=o})
    \ No newline at end of file
    +/**
    + * Drill SQL Definition (Forked from SqlServer definition)
    + */
    +
    +ace.define("ace/mode/sql_highlight_rules",["require","exports","module","ace/lib/oop","ace/mode/text_highlight_rules"], function(require, exports, module) {
    +"use strict";
    +
    +var oop = require("../lib/oop");
    +var TextHighlightRules = require("./text_highlight_rules").TextHighlightRules;
    +
    +var SqlHighlightRules = function() {
    +
    +    //TODO: https://drill.apache.org/docs/reserved-keywords/
    +	//e.g. Cubing operators like ROLLUP are not listed
    +    var keywords = (
    +        "select|insert|update|delete|from|where|and|or|group|by|order|limit|offset|having|as|case|" +
    +        "when|else|end|type|left|right|join|on|outer|desc|asc|union|create|table|key|if|" +
    +        "not|default|null|inner|database|drop|" +
    +		"flatten|kvgen|columns"
    +    );
    +	//Confirmed to be UnSupported as of Drill-1.12.0
    +	/* cross|natural|primary|foreign|references|grant */
    +
    +    var builtinConstants = (
    +        "true|false"
    +    );
    +
    +    //Drill-specific
    +    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|" +
    --- End diff --
    
    I tried looking at that option, but it is quite difficult to figure out how the libraries read these files. We could do a dynamic generation of this list, when the {sys.functions} table does get implemented. 


---

[GitHub] drill issue #1084: DRILL-5868: Support SQL syntax highlighting of queries

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:

    https://github.com/apache/drill/pull/1084
  
    When viewing a query profile:
    ![image](https://user-images.githubusercontent.com/4335237/34635374-5793b98c-f243-11e7-87db-2078892c571c.png)
    
    When editing the query of a submitted profile:
    ![image](https://user-images.githubusercontent.com/4335237/34635402-a5fbe1d0-f243-11e7-87e2-8dc9eecd38e6.png)
    
    When writing a new query and using snippets to autocomplete ( `ctas` and `s*` ):
    ![image](https://user-images.githubusercontent.com/4335237/34635428-eb1b166e-f243-11e7-8937-499040ef4dba.png)
    



---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r162478583
  
    --- Diff: exec/java-exec/src/main/resources/rest/profile/profile.ftl ---
    @@ -77,16 +84,17 @@ table.sortable thead .sorting_desc { background-image: url("/static/img/black-de
           <p>
     
             <#if model.isOnlyImpersonationEnabled()>
    +          <div id="query-editor" class="form-group">${model.getProfile().query}</div>
    +          <input class="form-control" id="query" name="query" type="hidden" value="${model.getProfile().query}"/>
               <div class="form-group">
                 <label for="userName">User Name</label>
                 <input type="text" size="30" name="userName" id="userName" placeholder="User Name" value="${model.getProfile().user}">
               </div>
             </#if>
     
             <form role="form" id="queryForm" action="/query" method="POST">
    -          <div class="form-group">
    -            <textarea class="form-control" id="query" name="query" style="font-family: Courier;">${model.getProfile().query}</textarea>
    -          </div>
    +          <div id="query-editor" class="form-group">${model.getProfile().query}</div>
    +          <input class="form-control" id="query" name="query" type="hidden" value="${model.getProfile().query}"/>
    --- End diff --
    
    Patched and Verified that with Impersonation enabled, there is only one editor for the profile page.


---

[GitHub] drill issue #1084: DRILL-5868: Support SQL syntax highlighting of queries

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:

    https://github.com/apache/drill/pull/1084
  
    Found some Javascript files with trailing white spaces. Removed these spaces and squashed after a rebase.


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/drill/pull/1084


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r160650090
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js ---
    @@ -0,0 +1,46 @@
    +/**
    + * Drill SQL Syntax Snippets
    + */
    +
    +ace.define("ace/snippets/sql",["require","exports","module"], function(require, exports, module) {
    +"use strict";
    +
    +exports.snippetText = "snippet info\n\
    +	select * from INFORMATION_SCHEMA.${1:<tableName>};\n\
    +snippet sysmem\n\
    --- End diff --
    
    Turns out that my Chrome, it works when using -> Ctrl + space.
    Could you please update Jira with list of available snippets and ways how to enable them?


---

[GitHub] drill issue #1084: DRILL-5868: Support SQL syntax highlighting of queries

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:

    https://github.com/apache/drill/pull/1084
  
    @arina-ielchiieva / @cgivre , would you care to review this? Let me know if you are unable to build and need screenshots.


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r162384019
  
    --- Diff: exec/java-exec/src/main/resources/rest/profile/profile.ftl ---
    @@ -77,16 +84,17 @@ table.sortable thead .sorting_desc { background-image: url("/static/img/black-de
           <p>
     
             <#if model.isOnlyImpersonationEnabled()>
    +          <div id="query-editor" class="form-group">${model.getProfile().query}</div>
    +          <input class="form-control" id="query" name="query" type="hidden" value="${model.getProfile().query}"/>
               <div class="form-group">
                 <label for="userName">User Name</label>
                 <input type="text" size="30" name="userName" id="userName" placeholder="User Name" value="${model.getProfile().user}">
               </div>
             </#if>
     
             <form role="form" id="queryForm" action="/query" method="POST">
    -          <div class="form-group">
    -            <textarea class="form-control" id="query" name="query" style="font-family: Courier;">${model.getProfile().query}</textarea>
    -          </div>
    +          <div id="query-editor" class="form-group">${model.getProfile().query}</div>
    +          <input class="form-control" id="query" name="query" type="hidden" value="${model.getProfile().query}"/>
    --- End diff --
    
    Let me take a look. This might have slipped past during a merge conflict. 


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r160651815
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js ---
    @@ -0,0 +1,46 @@
    +/**
    + * Drill SQL Syntax Snippets
    + */
    +
    +ace.define("ace/snippets/sql",["require","exports","module"], function(require, exports, module) {
    +"use strict";
    +
    +exports.snippetText = "snippet info\n\
    +	select * from INFORMATION_SCHEMA.${1:<tableName>};\n\
    +snippet sysmem\n\
    +	select * from sys.mem;\n\
    +snippet sysopt\n\
    +	select * from sys.opt;\n\
    +snippet sysbit\n\
    +	select * from sys.bit;\n\
    +snippet sysconn\n\
    +	select * from sys.conn;\n\
    +snippet sysprof\n\
    +	select * from sys.prof;\n\
    +snippet cview\n\
    +	create view ${1:[workspace]}.${2:<viewName>} ( ${3:<columnName>} )  as \n\
    +	${4:<query>};\n\
    +snippet ctas\n\
    +	create table ${1:<tableName>} ( ${2:<columnName>} )  as \n\
    +	${3:<query>};\n\
    +snippet ctemp\n\
    +	create temporary table ${1:<tableName>} ( ${2:<columnName>} )  as \n\
    --- End diff --
    
    Could you please also add snippet for create function: `CREATE FUNCTION USING JAR '<jar_name>.jar';`?


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r160487621
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js ---
    @@ -0,0 +1,46 @@
    +/**
    + * Drill SQL Syntax Snippets
    + */
    +
    +ace.define("ace/snippets/sql",["require","exports","module"], function(require, exports, module) {
    +"use strict";
    +
    +exports.snippetText = "snippet info\n\
    +	select * from INFORMATION_SCHEMA.${1:<tableName>};\n\
    +snippet sysmem\n\
    --- End diff --
    
    Not sure what you mean.
    All available snippet keywords (e.g. 'info') are in lower-case, but I don't think that is affecting the suggested autocomplete options. The template, however, is projected verbatim to the definition. So, INFORMATION_SCHEMA will remain in upper case. 
    ![image](https://user-images.githubusercontent.com/4335237/34736399-a2c01e78-f527-11e7-84aa-fcdfcefc6a5c.png)



---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r160447919
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/mode-sql.js ---
    @@ -1 +1,128 @@
    -ace.define("ace/mode/sql_highlight_rules",["require","exports","module","ace/lib/oop","ace/mode/text_highlight_rules"],function(e,t,n){"use strict";var r=e("../lib/oop"),i=e("./text_highlight_rules").TextHighlightRules,s=function(){var e="select|insert|update|delete|from|where|and|or|group|by|order|limit|offset|having|as|case|when|else|end|type|left|right|join|on|outer|desc|asc|union|create|table|primary|key|if|foreign|not|references|default|null|inner|cross|natural|database|drop|grant",t="true|false",n="avg|count|first|last|max|min|sum|ucase|lcase|mid|len|round|rank|now|format|coalesce|ifnull|isnull|nvl",r="int|numeric|decimal|date|varchar|char|bigint|float|double|bit|binary|text|set|timestamp|money|real|number|integer",i=this.createKeywordMapper({"support.function":n,keyword:e,"constant.language":t,"storage.type":r},"identifier",!0);this.$rules={start:[{token:"comment",regex:"--.*$"},{token:"comment",start:"/\\*",end:"\\*/"},{token:"string",regex:'".*?"'},{token:"string",regex
 :"'.*?'"},{token:"string",regex:"`.*?`"},{token:"constant.numeric",regex:"[+-]?\\d+(?:(?:\\.\\d*)?(?:[eE][+-]?\\d+)?)?\\b"},{token:i,regex:"[a-zA-Z_$][a-zA-Z0-9_$]*\\b"},{token:"keyword.operator",regex:"\\+|\\-|\\/|\\/\\/|%|<@>|@>|<@|&|\\^|~|<|>|<=|=>|==|!=|<>|="},{token:"paren.lparen",regex:"[\\(]"},{token:"paren.rparen",regex:"[\\)]"},{token:"text",regex:"\\s+"}]},this.normalizeRules()};r.inherits(s,i),t.SqlHighlightRules=s}),ace.define("ace/mode/sql",["require","exports","module","ace/lib/oop","ace/mode/text","ace/mode/sql_highlight_rules"],function(e,t,n){"use strict";var r=e("../lib/oop"),i=e("./text").Mode,s=e("./sql_highlight_rules").SqlHighlightRules,o=function(){this.HighlightRules=s,this.$behaviour=this.$defaultBehaviour};r.inherits(o,i),function(){this.lineCommentStart="--",this.$id="ace/mode/sql"}.call(o.prototype),t.Mode=o})
    \ No newline at end of file
    +/**
    + * Drill SQL Definition (Forked from SqlServer definition)
    + */
    +
    +ace.define("ace/mode/sql_highlight_rules",["require","exports","module","ace/lib/oop","ace/mode/text_highlight_rules"], function(require, exports, module) {
    +"use strict";
    +
    +var oop = require("../lib/oop");
    +var TextHighlightRules = require("./text_highlight_rules").TextHighlightRules;
    +
    +var SqlHighlightRules = function() {
    +
    +    //TODO: https://drill.apache.org/docs/reserved-keywords/
    +	//e.g. Cubing operators like ROLLUP are not listed
    +    var keywords = (
    +        "select|insert|update|delete|from|where|and|or|group|by|order|limit|offset|having|as|case|" +
    +        "when|else|end|type|left|right|join|on|outer|desc|asc|union|create|table|key|if|" +
    +        "not|default|null|inner|database|drop|" +
    +		"flatten|kvgen|columns"
    --- End diff --
    
    We have more keywords which would be nice to highlight and which are in https://drill.apache.org/docs/reserved-keywords/ list, for example: alter, function, explain, session, system etc. maybe we can add them in this PR?


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r160449936
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js ---
    @@ -0,0 +1,46 @@
    +/**
    + * Drill SQL Syntax Snippets
    + */
    +
    +ace.define("ace/snippets/sql",["require","exports","module"], function(require, exports, module) {
    --- End diff --
    
    1. Could you please add information about snippets to Jira description. We might what to add it to Drill documentation later so user knows which snippets can be used. Let's say, it's hard to guess the snippers without looking into the code :)
    
    2. When using snippets, I saw the following warning.
    
    ![image](https://user-images.githubusercontent.com/15086720/34730558-6a34fde4-f568-11e7-81e9-3e6d62091189.png)
    Could you please check?
      


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r160491309
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/mode-sql.js ---
    @@ -1 +1,128 @@
    -ace.define("ace/mode/sql_highlight_rules",["require","exports","module","ace/lib/oop","ace/mode/text_highlight_rules"],function(e,t,n){"use strict";var r=e("../lib/oop"),i=e("./text_highlight_rules").TextHighlightRules,s=function(){var e="select|insert|update|delete|from|where|and|or|group|by|order|limit|offset|having|as|case|when|else|end|type|left|right|join|on|outer|desc|asc|union|create|table|primary|key|if|foreign|not|references|default|null|inner|cross|natural|database|drop|grant",t="true|false",n="avg|count|first|last|max|min|sum|ucase|lcase|mid|len|round|rank|now|format|coalesce|ifnull|isnull|nvl",r="int|numeric|decimal|date|varchar|char|bigint|float|double|bit|binary|text|set|timestamp|money|real|number|integer",i=this.createKeywordMapper({"support.function":n,keyword:e,"constant.language":t,"storage.type":r},"identifier",!0);this.$rules={start:[{token:"comment",regex:"--.*$"},{token:"comment",start:"/\\*",end:"\\*/"},{token:"string",regex:'".*?"'},{token:"string",regex
 :"'.*?'"},{token:"string",regex:"`.*?`"},{token:"constant.numeric",regex:"[+-]?\\d+(?:(?:\\.\\d*)?(?:[eE][+-]?\\d+)?)?\\b"},{token:i,regex:"[a-zA-Z_$][a-zA-Z0-9_$]*\\b"},{token:"keyword.operator",regex:"\\+|\\-|\\/|\\/\\/|%|<@>|@>|<@|&|\\^|~|<|>|<=|=>|==|!=|<>|="},{token:"paren.lparen",regex:"[\\(]"},{token:"paren.rparen",regex:"[\\)]"},{token:"text",regex:"\\s+"}]},this.normalizeRules()};r.inherits(s,i),t.SqlHighlightRules=s}),ace.define("ace/mode/sql",["require","exports","module","ace/lib/oop","ace/mode/text","ace/mode/sql_highlight_rules"],function(e,t,n){"use strict";var r=e("../lib/oop"),i=e("./text").Mode,s=e("./sql_highlight_rules").SqlHighlightRules,o=function(){this.HighlightRules=s,this.$behaviour=this.$defaultBehaviour};r.inherits(o,i),function(){this.lineCommentStart="--",this.$id="ace/mode/sql"}.call(o.prototype),t.Mode=o})
    \ No newline at end of file
    +/**
    + * Drill SQL Definition (Forked from SqlServer definition)
    + */
    +
    +ace.define("ace/mode/sql_highlight_rules",["require","exports","module","ace/lib/oop","ace/mode/text_highlight_rules"], function(require, exports, module) {
    +"use strict";
    +
    +var oop = require("../lib/oop");
    +var TextHighlightRules = require("./text_highlight_rules").TextHighlightRules;
    +
    +var SqlHighlightRules = function() {
    +
    +    //TODO: https://drill.apache.org/docs/reserved-keywords/
    +	//e.g. Cubing operators like ROLLUP are not listed
    +    var keywords = (
    +        "select|insert|update|delete|from|where|and|or|group|by|order|limit|offset|having|as|case|" +
    +        "when|else|end|type|left|right|join|on|outer|desc|asc|union|create|table|key|if|" +
    +        "not|default|null|inner|database|drop|" +
    +		"flatten|kvgen|columns"
    --- End diff --
    
    There is a small challenge with this list. The 300+ keywords are a mix of reserved SQL keywords, functions and other words. I could be wrong, but I think some of these are not even supported (e.g. TRIGGER).
    I would need to separate the functions and SQL keywords for the colour highlighting to appear correctly.
    Let me walk through the list and at least identify the SQL keywords. Functions were already covered separately.


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r161288148
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js ---
    @@ -0,0 +1,46 @@
    +/**
    + * Drill SQL Syntax Snippets
    + */
    +
    +ace.define("ace/snippets/sql",["require","exports","module"], function(require, exports, module) {
    +"use strict";
    +
    +exports.snippetText = "snippet info\n\
    +	select * from INFORMATION_SCHEMA.${1:<tableName>};\n\
    +snippet sysmem\n\
    +	select * from sys.mem;\n\
    +snippet sysopt\n\
    +	select * from sys.opt;\n\
    +snippet sysbit\n\
    +	select * from sys.bit;\n\
    +snippet sysconn\n\
    +	select * from sys.conn;\n\
    +snippet sysprof\n\
    +	select * from sys.prof;\n\
    +snippet cview\n\
    --- End diff --
    
    Good catch. 
    I was generating the code for the snippets script, so I missed these (they went into a recent commit).  I'll fix these. 


---

[GitHub] drill pull request #1084: DRILL-5868: Support SQL syntax highlighting of que...

Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1084#discussion_r160482221
  
    --- Diff: exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/mode-sql.js ---
    @@ -1 +1,128 @@
    -ace.define("ace/mode/sql_highlight_rules",["require","exports","module","ace/lib/oop","ace/mode/text_highlight_rules"],function(e,t,n){"use strict";var r=e("../lib/oop"),i=e("./text_highlight_rules").TextHighlightRules,s=function(){var e="select|insert|update|delete|from|where|and|or|group|by|order|limit|offset|having|as|case|when|else|end|type|left|right|join|on|outer|desc|asc|union|create|table|primary|key|if|foreign|not|references|default|null|inner|cross|natural|database|drop|grant",t="true|false",n="avg|count|first|last|max|min|sum|ucase|lcase|mid|len|round|rank|now|format|coalesce|ifnull|isnull|nvl",r="int|numeric|decimal|date|varchar|char|bigint|float|double|bit|binary|text|set|timestamp|money|real|number|integer",i=this.createKeywordMapper({"support.function":n,keyword:e,"constant.language":t,"storage.type":r},"identifier",!0);this.$rules={start:[{token:"comment",regex:"--.*$"},{token:"comment",start:"/\\*",end:"\\*/"},{token:"string",regex:'".*?"'},{token:"string",regex
 :"'.*?'"},{token:"string",regex:"`.*?`"},{token:"constant.numeric",regex:"[+-]?\\d+(?:(?:\\.\\d*)?(?:[eE][+-]?\\d+)?)?\\b"},{token:i,regex:"[a-zA-Z_$][a-zA-Z0-9_$]*\\b"},{token:"keyword.operator",regex:"\\+|\\-|\\/|\\/\\/|%|<@>|@>|<@|&|\\^|~|<|>|<=|=>|==|!=|<>|="},{token:"paren.lparen",regex:"[\\(]"},{token:"paren.rparen",regex:"[\\)]"},{token:"text",regex:"\\s+"}]},this.normalizeRules()};r.inherits(s,i),t.SqlHighlightRules=s}),ace.define("ace/mode/sql",["require","exports","module","ace/lib/oop","ace/mode/text","ace/mode/sql_highlight_rules"],function(e,t,n){"use strict";var r=e("../lib/oop"),i=e("./text").Mode,s=e("./sql_highlight_rules").SqlHighlightRules,o=function(){this.HighlightRules=s,this.$behaviour=this.$defaultBehaviour};r.inherits(o,i),function(){this.lineCommentStart="--",this.$id="ace/mode/sql"}.call(o.prototype),t.Mode=o})
    \ No newline at end of file
    +/**
    + * Drill SQL Definition (Forked from SqlServer definition)
    + */
    +
    +ace.define("ace/mode/sql_highlight_rules",["require","exports","module","ace/lib/oop","ace/mode/text_highlight_rules"], function(require, exports, module) {
    +"use strict";
    +
    +var oop = require("../lib/oop");
    +var TextHighlightRules = require("./text_highlight_rules").TextHighlightRules;
    +
    +var SqlHighlightRules = function() {
    +
    +    //TODO: https://drill.apache.org/docs/reserved-keywords/
    +	//e.g. Cubing operators like ROLLUP are not listed
    +    var keywords = (
    +        "select|insert|update|delete|from|where|and|or|group|by|order|limit|offset|having|as|case|" +
    +        "when|else|end|type|left|right|join|on|outer|desc|asc|union|create|table|key|if|" +
    +        "not|default|null|inner|database|drop|" +
    +		"flatten|kvgen|columns"
    --- End diff --
    
    This list I could add. Adding everything else is a bit harder. I'll update the list in an add-on comit and attach to this PR. Thnx!


---

[GitHub] drill issue #1084: DRILL-5868: Support SQL syntax highlighting of queries

Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on the issue:

    https://github.com/apache/drill/pull/1084
  
    @kkhatua I saw some check style errors when applying the patch : 
    
    .git/rebase-apply/patch:215: trailing whitespace.           
    .git/rebase-apply/patch:396: trailing whitespace.	
    .git/rebase-apply/patch:478: trailing whitespace.
        //[Cannot supported due to space] 
    warning: 3 lines add whitespace errors.
    
    Can you rebase on latest master and fixup ?



---