DRILL-5868: Support SQL syntax highlighting of queries #1084

kkhatua · 2018-01-06T01:55:44Z

Based on the commit for DRILL-5981 ([PR #1043), this commit further leverages the Ace JavaScript library with customizations specific to Drill.

This introduces the following to Drill:

Syntax highlighting (This is also supported for submitted query profiles)
Auto-complete supported in all SQL editors (including the Edit Query tab within an existing profile to rerunning the query).
For browsers like Chrome, you should be able to type Ctrl+Space to show a drop down list for use. Arrow keys are used for navigating to the desired option.
Specifying Drill specific keywords and functions in visible autocomplete
Key snippets (template SQLs) allowing for rapid writing of syntax:
i. Query System Tables
ii. CView, CTAS, CTempTAS and CFuncJar
iii. Alter Session
iv. Explain and Select * queries
This is done by leveraging the Snippets feature in the Ace library, which allows for writing SQL code from templates. Like auto-complete, this will insert a template for you. The required/optional fields are then selected (in order) for you to populate, as you complete each and navigate to the next using Tab.

NOTE: The lists for items 3 and 4 are not exhaustive. As more features are added to Drill, these lists can be expanded.

kkhatua · 2018-01-06T01:56:31Z

@arina-ielchiieva / @cgivre , would you care to review this? Let me know if you are unable to build and need screenshots.

kkhatua · 2018-01-06T02:12:35Z

When viewing a query profile:

When editing the query of a submitted profile:

When writing a new query and using snippets to autocomplete ( ctas and s* ):

arina-ielchiieva · 2018-01-09T16:03:10Z

exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/mode-sql.js

+        "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"


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?

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!

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.

arina-ielchiieva · 2018-01-09T16:10:03Z

exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js

+ * Drill SQL Syntax Snippets
+ */
+
+ace.define("ace/snippets/sql",["require","exports","module"], function(require, exports, module) {


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 :)

When using snippets, I saw the following warning.

Could you please check?

+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.

arina-ielchiieva · 2018-01-09T16:19:18Z

exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/mode-sql.js

+    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|" +


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?

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.

Sounds good, could you please then just file a Jira for this improvement?

arina-ielchiieva · 2018-01-09T16:33:01Z

exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js

+
+exports.snippetText = "snippet info\n\
+	select * from INFORMATION_SCHEMA.${1:<tableName>};\n\
+snippet sysmem\n\


Also snippets are case-sensitive. Is it common practice?

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.

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?

arina-ielchiieva · 2018-01-10T11:22:51Z

exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js

+
+exports.snippetText = "snippet info\n\
+	select * from INFORMATION_SCHEMA.${1:<tableName>};\n\
+snippet sysmem\n\


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?

arina-ielchiieva · 2018-01-10T11:26:32Z

exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js

+	select * from sys.conn;\n\
+snippet sysprof\n\
+	select * from sys.prof;\n\
+snippet cview\n\


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 |

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.

arina-ielchiieva · 2018-01-10T11:31:31Z

exec/java-exec/src/main/resources/rest/static/js/ace-code-editor/snippets/sql.js

+	create table ${1:<tableName>} ( ${2:<columnName>} )  as \n\
+	${3:<query>};\n\
+snippet ctemp\n\
+	create temporary table ${1:<tableName>} ( ${2:<columnName>} )  as \n\


Could you please also add snippet for create function: CREATE FUNCTION USING JAR '<jar_name>.jar';?

kkhatua · 2018-01-12T23:48:48Z

@arina-ielchiieva updated the changes based on your review. Please have a look.

arina-ielchiieva · 2018-01-18T12:37:40Z

exec/java-exec/src/main/resources/rest/profile/profile.ftl

-            <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}"/>


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.

Let me take a look. This might have slipped past during a merge conflict.

Patched and Verified that with Impersonation enabled, there is only one editor for the profile page.

arina-ielchiieva · 2018-01-19T08:55:41Z

+1, LGTM.

amansinha100 · 2018-01-22T16:17:25Z

@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 ?

Based on the commit for DRILL-5981 (PR apache#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. Updates based on review comments 1. Disabled warning message 2. Extended reserved keyword list 3. Fixed bugs List: Update 1: Bug fix for when Impersonation is enabled Update 2: Remove the duplicate editor that might get injected Update 3: Removed trailing whitespaces in Javascripts

kkhatua · 2018-01-23T18:45:13Z

Found some Javascript files with trailing white spaces. Removed these spaces and squashed after a rebase.

Based on the commit for DRILL-5981 (PR apache#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 apache#3 and apache#4 are not exhaustive. As more features are added to Drill, these lists can be expanded. Updates based on review comments 1. Disabled warning message 2. Extended reserved keyword list 3. Fixed bugs List: Update 1: Bug fix for when Impersonation is enabled Update 2: Remove the duplicate editor that might get injected Update 3: Removed trailing whitespaces in Javascripts close apache#1084

kkhatua force-pushed the DRILL-5868 branch 2 times, most recently from 8f41c4e to 8c396d6 Compare January 9, 2018 02:13

arina-ielchiieva reviewed Jan 9, 2018

View reviewed changes

kkhatua force-pushed the DRILL-5868 branch 2 times, most recently from f4635ce to 9dabd5f Compare January 9, 2018 19:15

arina-ielchiieva requested changes Jan 10, 2018

View reviewed changes

kkhatua force-pushed the DRILL-5868 branch 3 times, most recently from a76002b to b19d901 Compare January 12, 2018 18:40

arina-ielchiieva reviewed Jan 18, 2018

View reviewed changes

kkhatua force-pushed the DRILL-5868 branch from c6b0579 to 5fa4059 Compare January 22, 2018 20:05

asfgit closed this in 4c8a55b Jan 24, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRILL-5868: Support SQL syntax highlighting of queries #1084

DRILL-5868: Support SQL syntax highlighting of queries #1084

kkhatua commented Jan 6, 2018 •

edited

kkhatua commented Jan 6, 2018

kkhatua commented Jan 6, 2018

arina-ielchiieva Jan 9, 2018

kkhatua Jan 9, 2018

kkhatua Jan 9, 2018

arina-ielchiieva Jan 9, 2018

kkhatua Jan 9, 2018

arina-ielchiieva Jan 9, 2018

kkhatua Jan 9, 2018 •

edited

arina-ielchiieva Jan 9, 2018

arina-ielchiieva Jan 9, 2018

kkhatua Jan 9, 2018

arina-ielchiieva Jan 10, 2018

arina-ielchiieva Jan 10, 2018

arina-ielchiieva Jan 10, 2018

kkhatua Jan 12, 2018

arina-ielchiieva Jan 10, 2018

kkhatua Jan 12, 2018

kkhatua commented Jan 12, 2018

arina-ielchiieva Jan 18, 2018

kkhatua Jan 18, 2018

kkhatua Jan 18, 2018

arina-ielchiieva commented Jan 19, 2018

amansinha100 commented Jan 22, 2018

kkhatua commented Jan 23, 2018

DRILL-5868: Support SQL syntax highlighting of queries #1084

DRILL-5868: Support SQL syntax highlighting of queries #1084

Conversation

kkhatua commented Jan 6, 2018 • edited

kkhatua commented Jan 6, 2018

kkhatua commented Jan 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kkhatua Jan 9, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kkhatua commented Jan 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arina-ielchiieva commented Jan 19, 2018

amansinha100 commented Jan 22, 2018

kkhatua commented Jan 23, 2018

kkhatua commented Jan 6, 2018 •

edited

kkhatua Jan 9, 2018 •

edited