SQL: Whitelist SQL utility class for better scripting #30681

costin · 2018-05-17T12:34:51Z

Add SQL class for reusing code inside SQL functions within Painless

elasticmachine · 2018-05-17T12:34:58Z

Pinging @elastic/es-search-aggs

Add SQL class for reusing code inside SQL functions within Painless Fix 29832

nik9000

I'm fairly sure we don't want to expose SQL functions to all scripts so we're kind of stuck with the whole "custom query" thing.

nik9000 · 2018-05-17T13:32:54Z

.../main/java/org/elasticsearch/xpack/sql/expression/function/scalar/script/ScriptTemplate.java

@@ -93,6 +92,6 @@ public String toString() {
    }

    public static String formatTemplate(String template) {
-        return template.replace("{}", "params.%s");
+        return template.replace("{sql}", "SqlScripts").replace("{}", "params.%s");


I wonder if we should just write SqlScripts rather than {sql}.

nik9000 · 2018-05-17T13:35:41Z

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPainlessExtension.java

+
+    @Override
+    public Map<ScriptContext<?>, List<Whitelist>> getContextWhitelists() {
+        Map<ScriptContext<?>, List<Whitelist>> whitelist = new LinkedHashMap<>();


I don't think we want to expose SQL functions to all queries though....

A bunch of us talked about this. While we'd prefer not to expose these all the other options are worse....

Could you switch this to a HashMap? I don't believe the ordering matters here.

jdconrad · 2018-05-31T15:53:15Z

I was under the impression we had all agreed to the custom query for exposing the required SQL functions as @nik9000 has mentioned because we want them only available to SQL.

nik9000 · 2018-05-31T16:06:06Z

A custom query isn't good enough because SQL is also used as the script for aggregations. To make that work we'd have to have hack all the aggregations somehow. We also talked about just using scripted aggregations with SQL which would work for something but would be inefficient and isn't super likely to work for all things.

I'd proposed making a sql-painless language that delegates to painless and flips the contexts around to painless ones but that is difficult because of the way that ScriptService is built. It still might be the best solution though.

jdconrad · 2018-05-31T16:33:29Z

@nik9000 Thanks for the explanation!

Added Internal in the name to better indicate that it should _not_ be used externally

costin · 2018-06-11T15:56:41Z

Renamed SqlScripts to InternalSqlScripts.

Add a more verbose name to better convey the class intent

nik9000

I left two minor things.

nik9000 · 2018-06-13T15:25:44Z

x-pack/plugin/sql/licenses/antlr4-runtime-4.5.3.jar.sha1

@@ -1 +0,0 @@
-2609e36f18f7e8d593cc1cddfb2ac776dc96b8e0


Why remove these?

Because of the extended-plugin functionality, the painless classpath is available to SQL and that already contains antlr. Which is great since it's one less jar (and its version) we need to worry about.

Hmmmm. I don't particularly like that we inherit Painless's antlr but I suppose I have to live with it.

nik9000 · 2018-06-13T15:27:23Z

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPainlessExtension.java

+
+    @Override
+    public Map<ScriptContext<?>, List<Whitelist>> getContextWhitelists() {
+        Map<ScriptContext<?>, List<Whitelist>> whitelist = new LinkedHashMap<>();


A bunch of us talked about this. While we'd prefer not to expose these all the other options are worse....

Could you switch this to a HashMap? I don't believe the ordering matters here.

nik9000 · 2018-06-13T15:28:22Z

x-pack/plugin/src/test/resources/rest-api-spec/test/sql/whitelist.yml

+                _id:    1
+            - str: test1
+              int: 1
+    - do:


I think this is a fairly internal detail so we shouldn't have an integration test for it. I think the normal SQL integration tests cover this use case plenty so we don't need this anyway.

nik9000 · 2018-06-13T16:09:10Z

x-pack/plugin/sql/licenses/antlr4-runtime-4.5.3.jar.sha1

@@ -1 +0,0 @@
-2609e36f18f7e8d593cc1cddfb2ac776dc96b8e0


Hmmmm. I don't particularly like that we inherit Painless's antlr but I suppose I have to live with it.

Add SQL class for reusing code inside SQL functions within Painless Fix elastic#29832

* elastic/master: (40 commits) [DOC] Extend SQL docs Immediately flush channel after writing to buffer (elastic#31301) [DOCS] Shortens ML API intros Use quotes in the call invocation (elastic#31249) move security ingest processors to a sub ingest directory (elastic#31306) Add 5.6.11 version constant. Fix version detection. SQL: Whitelist SQL utility class for better scripting (elastic#30681) [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (elastic#31299) CCS: don't proxy requests for already connected node (elastic#31273) Mute ScriptedMetricAggregatorTests testSelfReferencingAggStateAfterMap [test] opensuse packaging turn up debug logging Add unreleased version 6.3.1 Removes experimental tag from scripted_metric aggregation (elastic#31298) [Rollup] Metric config parser must use builder so validation runs (elastic#31159) [ML] Check licence when datafeeds use cross cluster search (elastic#31247) Add notion of internal index settings (elastic#31286) Test: Remove broken yml test feature (elastic#31255) REST hl client: cluster health to default to cluster level (elastic#31268) [ML] Update test thresholds to account for changes to memory control (elastic#31289) ...

* elastic/master: (29 commits) [DOC] Extend SQL docs Immediately flush channel after writing to buffer (elastic#31301) [DOCS] Shortens ML API intros Use quotes in the call invocation (elastic#31249) move security ingest processors to a sub ingest directory (elastic#31306) Add 5.6.11 version constant. Fix version detection. SQL: Whitelist SQL utility class for better scripting (elastic#30681) [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (elastic#31299) CCS: don't proxy requests for already connected node (elastic#31273) Mute ScriptedMetricAggregatorTests testSelfReferencingAggStateAfterMap [test] opensuse packaging turn up debug logging Add unreleased version 6.3.1 Removes experimental tag from scripted_metric aggregation (elastic#31298) [Rollup] Metric config parser must use builder so validation runs (elastic#31159) [ML] Check licence when datafeeds use cross cluster search (elastic#31247) Add notion of internal index settings (elastic#31286) Test: Remove broken yml test feature (elastic#31255) REST hl client: cluster health to default to cluster level (elastic#31268) [ML] Update test thresholds to account for changes to memory control (elastic#31289) ...

* master: Remove RestGetAllAliasesAction (#31308) Temporary fix for broken build Reenable Checkstyle's unused import rule (#31270) Remove remaining unused imports before merging #31270 Fix non-REST doc snippet [DOC] Extend SQL docs Immediately flush channel after writing to buffer (#31301) [DOCS] Shortens ML API intros Use quotes in the call invocation (#31249) move security ingest processors to a sub ingest directory (#31306) Add 5.6.11 version constant. Fix version detection. SQL: Whitelist SQL utility class for better scripting (#30681) [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (#31299) CCS: don't proxy requests for already connected node (#31273) Mute ScriptedMetricAggregatorTests testSelfReferencingAggStateAfterMap [test] opensuse packaging turn up debug logging Add unreleased version 6.3.1 Removes experimental tag from scripted_metric aggregation (#31298) [Rollup] Metric config parser must use builder so validation runs (#31159) [ML] Check licence when datafeeds use cross cluster search (#31247) Add notion of internal index settings (#31286) Test: Remove broken yml test feature (#31255) REST hl client: cluster health to default to cluster level (#31268) [ML] Update test thresholds to account for changes to memory control (#31289) Log warnings when cluster state publication failed to some nodes (#31233) Fix AntFixture waiting condition (#31272) Ignore numeric shard count if waiting for ALL (#31265) [ML] Implement new rules design (#31110) index_prefixes back-compat should test 6.3 (#30951) Core: Remove plain execute method on TransportAction (#30998) Update checkstyle to 8.10.1 (#31269) Set analyzer version in PreBuiltAnalyzerProviderFactory (#31202) Modify pipelining handlers to require full requests (#31280) Revert upgrade to Netty 4.1.25.Final (#31282) Use armored input stream for reading public key (#31229) Fix Netty 4 Server Transport tests. Again. REST hl client: adjust wait_for_active_shards param in cluster health (#31266) REST high-level Client: remove deprecated API methods (#31200) [DOCS] Mark SQL feature as experimental [DOCS] Updates machine learning custom URL screenshots (#31222) Fix naming conventions check for XPackTestCase Fix security Netty 4 transport tests Fix race in clear scroll (#31259) [DOCS] Clarify audit index settings when remote indexing (#30923) Delete typos in SAML docs (#31199) REST high-level client: add Cluster Health API (#29331) [ML][TEST] Mute tests using rules (#31204) Support RequestedAuthnContext (#31238) SyncedFlushResponse to implement ToXContentObject (#31155) Add Get Aliases API to the high-level REST client (#28799) Remove some line length supressions (#31209) Validate xContentType in PutWatchRequest. (#31088) [INGEST] Interrupt the current thread if evaluation grok expressions take too long (#31024) Suppress extras FS on caching directory tests Revert "[DOCS] Added 6.3 info & updated the upgrade table. (#30940)" Revert "Fix snippets in upgrade docs" Fix snippets in upgrade docs [DOCS] Added 6.3 info & updated the upgrade table. (#30940) LLClient: Support host selection (#30523) Upgrade to Netty 4.1.25.Final (#31232) Enable custom credentials for core REST tests (#31235) Move ESIndexLevelReplicationTestCase to test framework (#31243) Encapsulate Translog in Engine (#31220) HLRest: Add get index templates API (#31161) Remove all unused imports and fix CRLF (#31207) [Tests] Fix self-referencing tests [TEST] Fix testRecoveryAfterPrimaryPromotion [Docs] Remove mention pattern files in Grok processor (#31170) Use stronger write-once semantics for Azure repository (#30437) Don't swallow exceptions on replication (#31179) Limit the number of concurrent requests per node (#31206) Call ensureNoSelfReferences() on _agg state variable after scripted metric agg script executions (#31044) Move java version checker back to its own jar (#30708) [test] add fix for rare virtualbox error (#31212)

* 6.x: SQL: Fix build on Java 10 [Tests] Mutualize fixtures code in BaseHttpFixture (#31210) [TEST] Fix RemoteClusterClientTests#testEnsureWeReconnect [ML] Update test thresholds to account for changes to memory control (#31289) Reenable Checkstyle's unused import rule (#31270) [ML] Check licence when datafeeds use cross cluster search (#31247) Fix non-REST doc snippet [DOC] Extend SQL docs [DOCS] Shortens ML API intros Use quotes in the call invocation (#31249) move security ingest processors to a sub ingest directory (#31306) SQL: Whitelist SQL utility class for better scripting (#30681) Add 5.6.11 version constant. Fix version detection. [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (#31299) Add missing release notes. Security: fix token bwc with pre 6.0.0-beta2 (#31254) Fix compilation error in UpdateSettingsIT (#31304) Test: Remove broken yml test feature (#31255) Add unreleased version 6.3.1 [Rollup] Metric config parser must use builder so validation runs (#31159) Removes experimental tag from scripted_metric aggregation (#31298) [DOCS] Removes coming tag from 6.3.0 release notes 6.3 release notes. Add notion of internal index settings (#31286) REST high-level client: add Cluster Health API (#29331) Remove leftover usage of deprecated client API SyncedFlushResponse to implement ToXContentObject (#31155) Add Get Aliases API to the high-level REST client (#28799) HLRest: Add get index templates API (#31161) Log warnings when cluster state publication failed to some nodes (#31233) Fix AntFixture waiting condition (#31272) [TEST] Mute RecoveryIT.testHistoryUUIDIsGenerated Ignore numeric shard count if waiting for ALL (#31265) Update checkstyle to 8.10.1 (#31269) Set analyzer version in PreBuiltAnalyzerProviderFactory (#31202) Revert upgrade to Netty 4.1.25.Final (#31282) Use armored input stream for reading public key (#31229) [DOCS] Added 'fail_on_unsupported_field' param to MLT. Closes #28008 (#31160) Fix Netty 4 Server Transport tests. Again. [DOCS] Fixed typo. [DOCS] Added release highlights for 6.3 (#31256) [DOCS] Mark SQL feature as experimental [DOCS] Updates machine learning custom URL screenshots (#31222) Fix naming conventions check for XPackTestCase Fix security Netty 4 transport tests Fix race in clear scroll (#31259) [DOCS] Clarify audit index settings when remote indexing (#30923) [ML][TEST] Mute tests using rules (#31204) Support RequestedAuthnContext (#31238) Validate xContentType in PutWatchRequest. (#31088) [INGEST] Interrupt the current thread if evaluation grok expressions take too long (#31024) Upgrade to Netty 4.1.25.Final (#31232) Suppress extras FS on caching directory tests Revert "[DOCS] Added 6.3 info & updated the upgrade table. (#30940)" Revert "Fix snippets in upgrade docs" Fix snippets in upgrade docs [DOCS] Added 6.3 info & updated the upgrade table. (#30940) Enable custom credentials for core REST tests (#31235) Move ESIndexLevelReplicationTestCase to test framework (#31243) Encapsulate Translog in Engine (#31220) [DOCS] Adds machine learning 6.3.0 release notes (#31217) Remove all unused imports and fix CRLF (#31207) [TEST] Fix testRecoveryAfterPrimaryPromotion [Docs] Remove mention pattern files in Grok processor (#31170) Use stronger write-once semantics for Azure repository (#30437) Don't swallow exceptions on replication (#31179) Compliant SAML Response destination check (#31175) Move java version checker back to its own jar (#30708) TEST: Retry synced-flush if ongoing ops on primary (#30978) [test] add fix for rare virtualbox error (#31212)

costin added v7.0.0 :Analytics/SQL SQL querying v6.4.0 labels May 17, 2018

costin requested review from nik9000, imotov and astefan May 17, 2018 12:34

SQL: Whitelist SQL utility class for better scripting

92609bf

Add SQL class for reusing code inside SQL functions within Painless Fix 29832

costin force-pushed the fix-29832 branch from e56db9a to 92609bf Compare May 17, 2018 12:56

costin mentioned this pull request May 17, 2018

SQL: Extend ScriptTemplate to support functions #30594

Closed

nik9000 requested changes May 17, 2018

View reviewed changes

costin added 2 commits May 17, 2018 20:36

Remove ANTLR

b63a3e2

Merge branch 'fix-29832-script-engine' into fix-29832

d6b17e9

costin added 2 commits June 11, 2018 18:28

Merge remote-tracking branch 'remotes/upstream/master' into fix-29832

1cc2e5d

Rename SqlScripts to InternalSqlScript

c60e901

Added Internal in the name to better indicate that it should _not_ be used externally

costin requested a review from colings86 June 11, 2018 15:56

costin added 3 commits June 12, 2018 17:57

Add whitelisting to the script sort context as well

5b2a86b

Rename whitelisted class

d32b40b

Add a more verbose name to better convey the class intent

Merge remote-tracking branch 'remotes/upstream/master' into fix-29832

5f4e6bb

nik9000 requested changes Jun 13, 2018

View reviewed changes

Address feedback

83d1bbc

nik9000 approved these changes Jun 13, 2018

View reviewed changes

Merge branch 'master' into fix-29832

a06cb53

costin merged commit 43cb240 into elastic:master Jun 13, 2018

costin added a commit to costin/elasticsearch that referenced this pull request Jun 13, 2018

SQL: Whitelist SQL utility class for better scripting (elastic#30681)

e681512

Add SQL class for reusing code inside SQL functions within Painless Fix elastic#29832

costin deleted the fix-29832 branch June 13, 2018 20:36

tvernum mentioned this pull request Jun 14, 2018

ASM incompatibility between SQL/Painless and FeatureAwareCheck #31322

Closed

lcawl added the >enhancement label Aug 20, 2018

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SQL: Whitelist SQL utility class for better scripting #30681

SQL: Whitelist SQL utility class for better scripting #30681

costin commented May 17, 2018 •

edited by lcawl

Loading

elasticmachine commented May 17, 2018

nik9000 left a comment

nik9000 May 17, 2018

nik9000 May 17, 2018

nik9000 Jun 13, 2018

jdconrad commented May 31, 2018

nik9000 commented May 31, 2018

jdconrad commented May 31, 2018

costin commented Jun 11, 2018

nik9000 left a comment

nik9000 Jun 13, 2018

costin Jun 13, 2018

nik9000 Jun 13, 2018

nik9000 Jun 13, 2018

nik9000 Jun 13, 2018

nik9000 Jun 13, 2018

		@@ -1 +0,0 @@
		2609e36f18f7e8d593cc1cddfb2ac776dc96b8e0

		@@ -1 +0,0 @@
		2609e36f18f7e8d593cc1cddfb2ac776dc96b8e0

SQL: Whitelist SQL utility class for better scripting #30681

SQL: Whitelist SQL utility class for better scripting #30681

Conversation

costin commented May 17, 2018 • edited by lcawl Loading

elasticmachine commented May 17, 2018

nik9000 left a 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

jdconrad commented May 31, 2018

nik9000 commented May 31, 2018

jdconrad commented May 31, 2018

costin commented Jun 11, 2018

nik9000 left a 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

costin commented May 17, 2018 •

edited by lcawl

Loading