From 3872065693a4a3e94c8267940cea67746aaa2960 Mon Sep 17 00:00:00 2001 From: Ouertani Date: Sat, 22 Jun 2019 01:19:08 +0300 Subject: [PATCH 1/9] Fixes #2974 Arrays should be indexable from the end too --- .../ksql/codegen/SqlToJavaVisitor.java | 33 ++++++++++++++----- .../ksql/codegen/SqlToJavaVisitorTest.java | 12 +++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/ksql-engine/src/main/java/io/confluent/ksql/codegen/SqlToJavaVisitor.java b/ksql-engine/src/main/java/io/confluent/ksql/codegen/SqlToJavaVisitor.java index 977c134d146e..2f761554ccbf 100644 --- a/ksql-engine/src/main/java/io/confluent/ksql/codegen/SqlToJavaVisitor.java +++ b/ksql-engine/src/main/java/io/confluent/ksql/codegen/SqlToJavaVisitor.java @@ -633,15 +633,30 @@ protected Pair visitSubscriptExpression( SchemaUtil.getJavaType(internalSchema).getCanonicalName(); switch (internalSchema.type()) { case ARRAY: - return new Pair<>( - String.format("((%s) ((%s)%s).get((int)(%s)))", - SchemaUtil.getJavaType(internalSchema.valueSchema()).getSimpleName(), - internalSchemaJavaType, - process(node.getBase(), context).getLeft(), - process(node.getIndex(), context).getLeft() - ), - internalSchema.valueSchema() - ); + final String listName = process(node.getBase(), context).getLeft(); + if (node.getIndex().toString().startsWith("-")) { + return new Pair<>( + String.format("((%s) ((%s)%s).get((int)((%s)%s).size()%s))", + SchemaUtil.getJavaType(internalSchema.valueSchema()).getSimpleName(), + internalSchemaJavaType, + listName, + internalSchemaJavaType, + listName, + process(node.getIndex(), context).getLeft() + ), + internalSchema.valueSchema() + ); + } else { + return new Pair<>( + String.format("((%s) ((%s)%s).get((int)(%s)))", + SchemaUtil.getJavaType(internalSchema.valueSchema()).getSimpleName(), + internalSchemaJavaType, + listName, + process(node.getIndex(), context).getLeft() + ), + internalSchema.valueSchema() + ); + } case MAP: return new Pair<>( String.format("((%s) ((%s)%s).get(%s))", diff --git a/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java b/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java index 4867d91cc1f0..915af43ebb09 100644 --- a/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java +++ b/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java @@ -107,6 +107,18 @@ public void shouldProcessArrayExpressionCorrectly() { equalTo("((Double) ((java.util.List)TEST1_COL4).get((int)(Integer.parseInt(\"0\"))))")); } + @Test + public void shouldProcessArrayNegativeIndexExpressionCorrectly() { + final String simpleQuery = "SELECT col4[-1] FROM test1 WHERE col0 > 100;"; + final Analysis analysis = analyzeQuery(simpleQuery, metaStore); + + final String javaExpression = sqlToJavaVisitor + .process(analysis.getSelectExpressions().get(0)); + + assertThat(javaExpression, + equalTo("((Double) ((java.util.List)TEST1_COL4).get((int)((java.util.List)TEST1_COL4).size()-Integer.parseInt(\"1\")))")); + } + @Test public void shouldProcessMapExpressionCorrectly() { final String simpleQuery = "SELECT col5['key1'] FROM test1 WHERE col0 > 100;"; From ef7a82a0c8dcd51bbbc37a602cb74f3f41596ae2 Mon Sep 17 00:00:00 2001 From: Ouertani Date: Fri, 28 Jun 2019 14:33:38 +0300 Subject: [PATCH 2/9] fixes #2974 Arrays should be indexable from the end too --- .../ksql/codegen/SqlToJavaVisitor.java | 36 +++++--------- .../ksql/codegen/SqlToJavaVisitorTest.java | 2 +- .../query-validation-tests/arrayindex.json | 47 +++++++++++++++++++ 3 files changed, 61 insertions(+), 24 deletions(-) create mode 100644 ksql-functional-tests/src/test/resources/query-validation-tests/arrayindex.json diff --git a/ksql-engine/src/main/java/io/confluent/ksql/codegen/SqlToJavaVisitor.java b/ksql-engine/src/main/java/io/confluent/ksql/codegen/SqlToJavaVisitor.java index 2f761554ccbf..1b64767c0751 100644 --- a/ksql-engine/src/main/java/io/confluent/ksql/codegen/SqlToJavaVisitor.java +++ b/ksql-engine/src/main/java/io/confluent/ksql/codegen/SqlToJavaVisitor.java @@ -634,29 +634,19 @@ protected Pair visitSubscriptExpression( switch (internalSchema.type()) { case ARRAY: final String listName = process(node.getBase(), context).getLeft(); - if (node.getIndex().toString().startsWith("-")) { - return new Pair<>( - String.format("((%s) ((%s)%s).get((int)((%s)%s).size()%s))", - SchemaUtil.getJavaType(internalSchema.valueSchema()).getSimpleName(), - internalSchemaJavaType, - listName, - internalSchemaJavaType, - listName, - process(node.getIndex(), context).getLeft() - ), - internalSchema.valueSchema() - ); - } else { - return new Pair<>( - String.format("((%s) ((%s)%s).get((int)(%s)))", - SchemaUtil.getJavaType(internalSchema.valueSchema()).getSimpleName(), - internalSchemaJavaType, - listName, - process(node.getIndex(), context).getLeft() - ), - internalSchema.valueSchema() - ); - } + final String suppliedIdx = process(node.getIndex(), context).getLeft(); + final String trueIdx = node.getIndex().toString().startsWith("-") + ? String.format("((%s)%s).size()%s", internalSchemaJavaType, + listName, suppliedIdx) + : suppliedIdx; + return new Pair<>( + String.format("((%s) ((%s)%s).get((int)%s))", + SchemaUtil.getJavaType(internalSchema.valueSchema()).getSimpleName(), + internalSchemaJavaType, + listName, + trueIdx), + internalSchema.valueSchema() + ); case MAP: return new Pair<>( String.format("((%s) ((%s)%s).get(%s))", diff --git a/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java b/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java index 915af43ebb09..3da2df364223 100644 --- a/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java +++ b/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java @@ -104,7 +104,7 @@ public void shouldProcessArrayExpressionCorrectly() { .process(analysis.getSelectExpressions().get(0)); assertThat(javaExpression, - equalTo("((Double) ((java.util.List)TEST1_COL4).get((int)(Integer.parseInt(\"0\"))))")); + equalTo("((Double) ((java.util.List)TEST1_COL4).get((int)Integer.parseInt(\"0\")))")); } @Test diff --git a/ksql-functional-tests/src/test/resources/query-validation-tests/arrayindex.json b/ksql-functional-tests/src/test/resources/query-validation-tests/arrayindex.json new file mode 100644 index 000000000000..848802aeba8e --- /dev/null +++ b/ksql-functional-tests/src/test/resources/query-validation-tests/arrayindex.json @@ -0,0 +1,47 @@ +{ + "comments": [ + "Arrays should be indexable from the end too", + "https://github.com/confluentinc/ksql/issues/2974" + ], + "tests": [ + + { + "name": "select the first element of an Array", + "statements": [ + "CREATE STREAM test (colors ARRAY) WITH (kafka_topic='test_topic', value_format='JSON');", + "CREATE STREAM OUTPUT AS SELECT colors[0] as C FROM test;" + ], + "inputs": [ + {"topic": "test_topic", "key": 1, "value": {"colors": ["Red", "Green"]}, "timestamp": 0}, + {"topic": "test_topic", "key": 1, "value": {"colors": ["Black"]}, "timestamp": 0}, + {"topic": "test_topic", "key": 1, "value": {"colors": ["Pink", "Yellow", "Pink"]}, "timestamp": 0} + ], + "outputs": [ + {"topic": "OUTPUT", "key": 1, "value": {"C": "Red"}, "timestamp": 0}, + {"topic": "OUTPUT", "key": 1, "value": {"C": "Black"}, "timestamp": 0}, + {"topic": "OUTPUT", "key": 1, "value": {"C": "Pink"}, "timestamp": 0} + ] + }, + { + "name": "select the last element of an Array (-1)", + "statements": [ + "CREATE STREAM test (colors ARRAY) WITH (kafka_topic='test_topic', value_format='JSON');", + "CREATE STREAM OUTPUT AS SELECT colors[-1] as C FROM test;" + ], + "inputs": [ + {"topic": "test_topic", "key": 1, "value": {"colors": ["Red", "Green"]}, "timestamp": 0}, + {"topic": "test_topic", "key": 1, "value": {"colors": ["Black"]}, "timestamp": 0}, + {"topic": "test_topic", "key": 1, "value": {"colors": ["Pink", "Yellow", "Pink"]}, "timestamp": 0}, + {"topic": "test_topic", "key": 1, "value": {"colors": ["White", "Pink"]}, "timestamp": 0}, + {"topic": "test_topic", "key": 1, "value": {"colors": ["Pink", null]}, "timestamp": 0} + ], + "outputs": [ + {"topic": "OUTPUT", "key": 1, "value": {"C": "Green"}, "timestamp": 0}, + {"topic": "OUTPUT", "key": 1, "value": {"C": "Black"}, "timestamp": 0}, + {"topic": "OUTPUT", "key": 1, "value": {"C": "Pink"}, "timestamp": 0}, + {"topic": "OUTPUT", "key": 1, "value": {"C": "Pink"}, "timestamp": 0}, + {"topic": "OUTPUT", "key": 1, "value": {"C": null}, "timestamp": 0} + ] + } + ] +} \ No newline at end of file From 63a3ebd6772090485ba0c2c5a170f6cc554bd57d Mon Sep 17 00:00:00 2001 From: Andy Coates <8012398+big-andy-coates@users.noreply.github.com> Date: Fri, 28 Jun 2019 19:35:35 +0100 Subject: [PATCH 3/9] Fix indentation --- .../ksql/codegen/SqlToJavaVisitor.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/ksql-engine/src/main/java/io/confluent/ksql/codegen/SqlToJavaVisitor.java b/ksql-engine/src/main/java/io/confluent/ksql/codegen/SqlToJavaVisitor.java index 202c6001c147..8c7e8d9d1092 100644 --- a/ksql-engine/src/main/java/io/confluent/ksql/codegen/SqlToJavaVisitor.java +++ b/ksql-engine/src/main/java/io/confluent/ksql/codegen/SqlToJavaVisitor.java @@ -655,17 +655,17 @@ protected Pair visitSubscriptExpression( final String listName = process(node.getBase(), context).getLeft(); final String suppliedIdx = process(node.getIndex(), context).getLeft(); final String trueIdx = node.getIndex().toString().startsWith("-") - ? String.format("((%s)%s).size()%s", internalSchemaJavaType, - listName, suppliedIdx) - : suppliedIdx; - return new Pair<>( - String.format("((%s) ((%s)%s).get((int)%s))", - SchemaUtil.getJavaType(internalSchema.valueSchema()).getSimpleName(), - internalSchemaJavaType, - listName, - trueIdx), - internalSchema.valueSchema() - ); + ? String.format("((%s)%s).size()%s", internalSchemaJavaType, listName, suppliedIdx) + : suppliedIdx; + + final String code = format("((%s) ((%s)%s).get((int)%s))", + SchemaUtil.getJavaType(internalSchema.valueSchema()).getSimpleName(), + internalSchemaJavaType, + listName, + trueIdx); + + return new Pair<>(code, internalSchema.valueSchema()); + case MAP: return new Pair<>( String.format("((%s) ((%s)%s).get(%s))", @@ -944,4 +944,4 @@ private CaseWhenProcessed( } } -} \ No newline at end of file +} From 2da5e36fdbc4dc3bb2ec5979aef0226f71dc1940 Mon Sep 17 00:00:00 2001 From: Andy Coates <8012398+big-andy-coates@users.noreply.github.com> Date: Fri, 28 Jun 2019 19:36:35 +0100 Subject: [PATCH 4/9] Fix my merge error --- .../java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java b/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java index e9c2504c8713..3a276523de87 100644 --- a/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java +++ b/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java @@ -104,7 +104,7 @@ public void shouldProcessArrayExpressionCorrectly() { .process(analysis.getSelectExpressions().get(0)); assertThat(javaExpression, - equalTo("((Double) ((java.util.List)TEST1_COL4).get((int)(0)))")); + equalTo("((Double) ((java.util.List)TEST1_COL4).get((int)0))")); } @Test @@ -663,4 +663,4 @@ public void shouldThrowOnNotIn() { // When: sqlToJavaVisitor.process(decimal); } -} \ No newline at end of file +} From ef7406fb239beed986dde7a6b69c9ec62a28e03d Mon Sep 17 00:00:00 2001 From: Andy Coates <8012398+big-andy-coates@users.noreply.github.com> Date: Fri, 28 Jun 2019 19:40:34 +0100 Subject: [PATCH 5/9] Fix indentation, add test cases to cover out of bounds --- .../query-validation-tests/arrayindex.json | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/ksql-functional-tests/src/test/resources/query-validation-tests/arrayindex.json b/ksql-functional-tests/src/test/resources/query-validation-tests/arrayindex.json index 848802aeba8e..effebbac04ce 100644 --- a/ksql-functional-tests/src/test/resources/query-validation-tests/arrayindex.json +++ b/ksql-functional-tests/src/test/resources/query-validation-tests/arrayindex.json @@ -12,14 +12,16 @@ "CREATE STREAM OUTPUT AS SELECT colors[0] as C FROM test;" ], "inputs": [ - {"topic": "test_topic", "key": 1, "value": {"colors": ["Red", "Green"]}, "timestamp": 0}, - {"topic": "test_topic", "key": 1, "value": {"colors": ["Black"]}, "timestamp": 0}, - {"topic": "test_topic", "key": 1, "value": {"colors": ["Pink", "Yellow", "Pink"]}, "timestamp": 0} + {"topic": "test_topic", "value": {"colors": ["Red", "Green"]}}, + {"topic": "test_topic", "value": {"colors": ["Black"]}}, + {"topic": "test_topic", "value": {"colors": [null, "Yellow", "Pink"]}}, + {"topic": "test_topic", "value": {"colors": []}} ], "outputs": [ - {"topic": "OUTPUT", "key": 1, "value": {"C": "Red"}, "timestamp": 0}, - {"topic": "OUTPUT", "key": 1, "value": {"C": "Black"}, "timestamp": 0}, - {"topic": "OUTPUT", "key": 1, "value": {"C": "Pink"}, "timestamp": 0} + {"topic": "OUTPUT", "value": {"C": "Red"}}, + {"topic": "OUTPUT", "value": {"C": "Black"}}, + {"topic": "OUTPUT", "value": {"C": null}}, + {"topic": "OUTPUT", "value": {"C": null}} ] }, { @@ -29,19 +31,21 @@ "CREATE STREAM OUTPUT AS SELECT colors[-1] as C FROM test;" ], "inputs": [ - {"topic": "test_topic", "key": 1, "value": {"colors": ["Red", "Green"]}, "timestamp": 0}, - {"topic": "test_topic", "key": 1, "value": {"colors": ["Black"]}, "timestamp": 0}, - {"topic": "test_topic", "key": 1, "value": {"colors": ["Pink", "Yellow", "Pink"]}, "timestamp": 0}, - {"topic": "test_topic", "key": 1, "value": {"colors": ["White", "Pink"]}, "timestamp": 0}, - {"topic": "test_topic", "key": 1, "value": {"colors": ["Pink", null]}, "timestamp": 0} - ], - "outputs": [ - {"topic": "OUTPUT", "key": 1, "value": {"C": "Green"}, "timestamp": 0}, - {"topic": "OUTPUT", "key": 1, "value": {"C": "Black"}, "timestamp": 0}, - {"topic": "OUTPUT", "key": 1, "value": {"C": "Pink"}, "timestamp": 0}, - {"topic": "OUTPUT", "key": 1, "value": {"C": "Pink"}, "timestamp": 0}, - {"topic": "OUTPUT", "key": 1, "value": {"C": null}, "timestamp": 0} - ] - } + {"topic": "test_topic", "value": {"colors": ["Red", "Green"]}}, + {"topic": "test_topic", "value": {"colors": ["Black"]}}, + {"topic": "test_topic", "value": {"colors": ["Pink", "Yellow", "Pink"]}}, + {"topic": "test_topic", "value": {"colors": ["White", "Pink"]}}, + {"topic": "test_topic", "value": {"colors": ["Pink", null]}}, + {"topic": "test_topic", "value": {"colors": []}} + ], + "outputs": [ + {"topic": "OUTPUT", "value": {"C": "Green"}}, + {"topic": "OUTPUT", "value": {"C": "Black"}}, + {"topic": "OUTPUT", "value": {"C": "Pink"}}, + {"topic": "OUTPUT", "value": {"C": "Pink"}}, + {"topic": "OUTPUT", "value": {"C": null}}, + {"topic": "OUTPUT", "value": {"C": null}} + ] + } ] -} \ No newline at end of file +} From 7069f369f7c3d739f0c9b7d28b2446b171b51033 Mon Sep 17 00:00:00 2001 From: Ouertani Date: Sat, 29 Jun 2019 11:28:46 +0300 Subject: [PATCH 6/9] Fixes #2974 remove complication from test --- .../java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java b/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java index 3da2df364223..bc547791730a 100644 --- a/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java +++ b/ksql-engine/src/test/java/io/confluent/ksql/codegen/SqlToJavaVisitorTest.java @@ -109,7 +109,7 @@ public void shouldProcessArrayExpressionCorrectly() { @Test public void shouldProcessArrayNegativeIndexExpressionCorrectly() { - final String simpleQuery = "SELECT col4[-1] FROM test1 WHERE col0 > 100;"; + final String simpleQuery = "SELECT col4[-1] FROM test1;"; final Analysis analysis = analyzeQuery(simpleQuery, metaStore); final String javaExpression = sqlToJavaVisitor From 8becd5b1103c3a7851283fb9f3fbb56aa4b2964d Mon Sep 17 00:00:00 2001 From: Ouertani Date: Wed, 3 Jul 2019 19:13:02 +0300 Subject: [PATCH 7/9] docs: documentation updated --- .../query-with-arrays-and-maps.rst | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/docs/developer-guide/query-with-arrays-and-maps.rst b/docs/developer-guide/query-with-arrays-and-maps.rst index a1afca1ce265..e44d5e525822 100644 --- a/docs/developer-guide/query-with-arrays-and-maps.rst +++ b/docs/developer-guide/query-with-arrays-and-maps.rst @@ -104,6 +104,35 @@ Your output should resemble: Press Ctrl+C to terminate the query. +Arrays elements can be accessible using positive or negative index. +For instance, to get the user last interest run the following select: + +.. code:: sql + + SELECT interests[-1] AS last_interest, + userid, + gender, + regionid + FROM users_extended; + +Your output should resemble: + +:: + + Travel | User_9 | OTHER | Region_6 + Travel | User_2 | FEMALE | Region_5 + Sport | User_3 | FEMALE | Region_8 + Movies | User_5 | OTHER | Region_9 + Movies | User_8 | MALE | Region_1 + Movies | User_1 | MALE | Region_6 + News | User_4 | MALE | Region_9 + Movies | User_7 | OTHER | Region_1 + Sport | User_6 | FEMALE | Region_5 + ^CQuery terminated + + +Press Ctrl+C to terminate the query. + Next Steps ********** From 88f4f1bcb8e05cb38ebbfd8b41a789fc55c11512 Mon Sep 17 00:00:00 2001 From: slim Date: Thu, 4 Jul 2019 20:53:33 +0300 Subject: [PATCH 8/9] Update docs/developer-guide/query-with-arrays-and-maps.rst Co-Authored-By: Jim Galasyn --- docs/developer-guide/query-with-arrays-and-maps.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer-guide/query-with-arrays-and-maps.rst b/docs/developer-guide/query-with-arrays-and-maps.rst index e44d5e525822..63d845ab239b 100644 --- a/docs/developer-guide/query-with-arrays-and-maps.rst +++ b/docs/developer-guide/query-with-arrays-and-maps.rst @@ -104,7 +104,7 @@ Your output should resemble: Press Ctrl+C to terminate the query. -Arrays elements can be accessible using positive or negative index. +You can access array elements by using positive or negative index values. For instance, to get the user last interest run the following select: .. code:: sql From 756ec092f0b6ccae9d107273c550643c25897668 Mon Sep 17 00:00:00 2001 From: slim Date: Thu, 4 Jul 2019 20:53:56 +0300 Subject: [PATCH 9/9] Update docs/developer-guide/query-with-arrays-and-maps.rst Co-Authored-By: Jim Galasyn --- docs/developer-guide/query-with-arrays-and-maps.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer-guide/query-with-arrays-and-maps.rst b/docs/developer-guide/query-with-arrays-and-maps.rst index 63d845ab239b..dfffad6e638a 100644 --- a/docs/developer-guide/query-with-arrays-and-maps.rst +++ b/docs/developer-guide/query-with-arrays-and-maps.rst @@ -105,7 +105,7 @@ Your output should resemble: Press Ctrl+C to terminate the query. You can access array elements by using positive or negative index values. -For instance, to get the user last interest run the following select: +For example, to get the user's last interest run the following SELECT statement: .. code:: sql