From 7689a1af28d9dd8127843b0b59beba062a6f9146 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Wed, 11 May 2016 02:29:35 -0400 Subject: [PATCH] Add 'ctx' keyword to painless. --- .../org/elasticsearch/painless/Analyzer.java | 4 ++- .../painless/AnalyzerExternal.java | 18 +++++++------ .../org/elasticsearch/painless/Metadata.java | 16 ++++++++++-- .../org/elasticsearch/painless/Writer.java | 9 +++++++ .../painless/WriterExternal.java | 4 --- .../painless/WriterStatement.java | 4 --- .../painless/ReservedWordTests.java | 26 ++++++++++++++++++- .../rest-api-spec/test/plan_a/15_update.yaml | 4 +-- .../test/plan_a/25_script_upsert.yaml | 6 ++--- 9 files changed, 66 insertions(+), 25 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Analyzer.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Analyzer.java index eb9ac2979c7..f374bccb54c 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Analyzer.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Analyzer.java @@ -101,7 +101,7 @@ class Analyzer extends PainlessParserBaseVisitor { // scorer parameter passed to the script. internal use only. metadata.scorerValueSlot = utility.addVariable(null, "#scorer", definition.objectType).slot; // doc parameter passed to the script. - // TODO: currently working as a def type, should be smapType... + // TODO: currently working as a Map, we can do better? metadata.docValueSlot = utility.addVariable(null, "doc", definition.smapType).slot; // // reserved words implemented as local variables @@ -110,6 +110,8 @@ class Analyzer extends PainlessParserBaseVisitor { metadata.loopCounterSlot = utility.addVariable(null, "#loop", definition.intType).slot; // document's score as a read-only float. metadata.scoreValueSlot = utility.addVariable(null, "_score", definition.floatType).slot; + // ctx map set by executable scripts as a read-only map. + metadata.ctxValueSlot = utility.addVariable(null, "ctx", definition.smapType).slot; metadata.createStatementMetadata(metadata.root); visit(metadata.root); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/AnalyzerExternal.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/AnalyzerExternal.java index f907138f752..e8e8cb88fef 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/AnalyzerExternal.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/AnalyzerExternal.java @@ -449,16 +449,18 @@ class AnalyzerExternal { } // special cases: reserved words - if (varenmd.last && ("_score".equals(id) || "doc".equals(id))) { - // read-only: don't allow stores - if (parentemd.storeExpr != null) { + if ("_score".equals(id) || "doc".equals(id) || "ctx".equals(id)) { + // read-only: don't allow stores to ourself + if (varenmd.last && parentemd.storeExpr != null) { throw new IllegalArgumentException(AnalyzerUtility.error(ctx) + "Variable [" + id + "] is read-only."); } - } - - // track if the _score value is ever used, we will invoke Scorer.score() only once if so. - if ("_score".equals(id)) { - metadata.scoreValueUsed = true; + if ("_score".equals(id)) { + // track if the _score value is ever used, we will invoke Scorer.score() only once if so. + metadata.scoreValueUsed = true; + } else if ("ctx".equals(id)) { + // track if ctx value is ever used, we will invoke Map.get() only once if so. + metadata.ctxValueUsed = true; + } } varenmd.target = variable.slot; diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Metadata.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Metadata.java index 85657360ef0..8692ea4a0a1 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Metadata.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Metadata.java @@ -429,8 +429,8 @@ class Metadata { int scoreValueSlot = -1; /** - * Used to determine if the _score variable is actually used. This is used in the {@link Analyzer} to update - * variable slots at the completion of analysis if _score is not used. + * Used to determine if the _score variable is actually used. This is used to know if we should call + * Scorer.score() once and cache into a local variable, and expose NeedsScore interface (to allow query caching) */ boolean scoreValueUsed = false; @@ -439,6 +439,18 @@ class Metadata { * the doc variable is accessed. */ int docValueSlot = -1; + + /** + * Used to determine what slot the ctx variable is stored in. This is used in the {@link Writer} whenever + * the ctx variable is accessed. + */ + int ctxValueSlot = -1; + + /** + * Used to determine if the ctx variable is actually used. This is used to determine if we should call + * Map.get once and store into a local variable on startup. + */ + boolean ctxValueUsed = false; /** * Maps the relevant ANTLR node to its metadata. diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Writer.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Writer.java index 6a57c40b817..20b609fa8c7 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Writer.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Writer.java @@ -157,6 +157,15 @@ class Writer extends PainlessParserBaseVisitor { execute.invokeVirtual(WriterConstants.SCORER_TYPE, WriterConstants.SCORER_SCORE); execute.visitVarInsn(Opcodes.FSTORE, metadata.scoreValueSlot); } + + if (metadata.ctxValueUsed) { + // if the _ctx value is used, we do this once: + // Map ctx = input.get("ctx"); + execute.visitVarInsn(Opcodes.ALOAD, metadata.inputValueSlot); + execute.push("ctx"); + execute.invokeInterface(WriterConstants.MAP_TYPE, WriterConstants.MAP_GET); + execute.visitVarInsn(Opcodes.ASTORE, metadata.ctxValueSlot); + } execute.push(settings.getMaxLoopCounter()); execute.visitVarInsn(Opcodes.ISTORE, metadata.loopCounterSlot); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterExternal.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterExternal.java index f25780ab496..2ce231412c1 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterExternal.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterExternal.java @@ -422,10 +422,6 @@ class WriterExternal { throw new IllegalStateException(WriterUtility.error(source) + "Cannot load/store void type."); } - if (!metadata.scoreValueUsed && slot > metadata.scoreValueSlot) { - --slot; - } - if (store) { execute.visitVarInsn(type.type.getOpcode(Opcodes.ISTORE), slot); } else { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterStatement.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterStatement.java index 4cd698a74cd..fc8b5ba71e4 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterStatement.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterStatement.java @@ -328,10 +328,6 @@ class WriterStatement { final Sort sort = declvaremd.to.sort; int slot = (int)declvaremd.postConst; - if (!metadata.scoreValueUsed && slot > metadata.scoreValueSlot) { - --slot; - } - final ExpressionContext exprctx = ctx.expression(); final boolean initialize = exprctx == null; diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/ReservedWordTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/ReservedWordTests.java index 64296812bda..46e08d60c2d 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/ReservedWordTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/ReservedWordTests.java @@ -19,6 +19,9 @@ package org.elasticsearch.painless; +import java.util.Collections; +import java.util.HashMap; + /** Tests for special reserved words such as _score */ public class ReservedWordTests extends ScriptTestCase { @@ -46,11 +49,32 @@ public class ReservedWordTests extends ScriptTestCase { assertTrue(expected.getMessage().contains("Variable name [doc] already defined")); } - /** check that we can't write to _score, its read-only! */ + /** check that we can't write to doc, its read-only! */ public void testDocStore() { IllegalArgumentException expected = expectThrows(IllegalArgumentException.class, () -> { exec("doc = 5; return doc;"); }); assertTrue(expected.getMessage().contains("Variable [doc] is read-only")); } + + /** check that we can't declare a variable of ctx, its really reserved! */ + public void testCtxVar() { + IllegalArgumentException expected = expectThrows(IllegalArgumentException.class, () -> { + exec("int ctx = 5; return ctx;"); + }); + assertTrue(expected.getMessage().contains("Variable name [ctx] already defined")); + } + + /** check that we can't write to ctx, its read-only! */ + public void testCtxStore() { + IllegalArgumentException expected = expectThrows(IllegalArgumentException.class, () -> { + exec("ctx = 5; return ctx;"); + }); + assertTrue(expected.getMessage().contains("Variable [ctx] is read-only")); + } + + /** check that we can modify its contents though */ + public void testCtxStoreMap() { + assertEquals(5, exec("ctx.foo = 5; return ctx.foo;", Collections.singletonMap("ctx", new HashMap()))); + } } diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/plan_a/15_update.yaml b/modules/lang-painless/src/test/resources/rest-api-spec/test/plan_a/15_update.yaml index 0cd8b52a2bb..39cee4e82a3 100644 --- a/modules/lang-painless/src/test/resources/rest-api-spec/test/plan_a/15_update.yaml +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/plan_a/15_update.yaml @@ -18,7 +18,7 @@ script: "1" body: lang: painless - script: "input.ctx._source.foo = input.bar" + script: "ctx._source.foo = input.bar" params: { bar: 'xxx' } - match: { _index: test_1 } @@ -41,7 +41,7 @@ type: test id: 1 lang: painless - script: "input.ctx._source.foo = 'yyy'" + script: "ctx._source.foo = 'yyy'" - match: { _index: test_1 } - match: { _type: test } diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/plan_a/25_script_upsert.yaml b/modules/lang-painless/src/test/resources/rest-api-spec/test/plan_a/25_script_upsert.yaml index 6a0fef06e37..d4a447e98a7 100644 --- a/modules/lang-painless/src/test/resources/rest-api-spec/test/plan_a/25_script_upsert.yaml +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/plan_a/25_script_upsert.yaml @@ -7,7 +7,7 @@ type: test id: 1 body: - script: "input.ctx._source.foo = input.bar" + script: "ctx._source.foo = input.bar" lang: "painless" params: { bar: 'xxx' } upsert: { foo: baz } @@ -27,7 +27,7 @@ type: test id: 1 body: - script: "input.ctx._source.foo = input.bar" + script: "ctx._source.foo = input.bar" lang: "painless" params: { bar: 'xxx' } upsert: { foo: baz } @@ -46,7 +46,7 @@ type: test id: 2 body: - script: "input.ctx._source.foo = input.bar" + script: "ctx._source.foo = input.bar" lang: "painless" params: { bar: 'xxx' } upsert: { foo: baz }