From be2dcc867f245f719ff3cf6f7026b75b6a8d0cf7 Mon Sep 17 00:00:00 2001 From: Jayaprakash Sivaprasad Date: Tue, 6 Aug 2024 19:31:54 -0700 Subject: [PATCH] Avoid pushdown of negative position for element_at for array --- .../hive/TestHivePushdownFilterQueries.java | 57 +++++++++++++++++++ .../optimizations/PushdownSubfields.java | 6 ++ 2 files changed, 63 insertions(+) diff --git a/presto-hive/src/test/java/com/facebook/presto/hive/TestHivePushdownFilterQueries.java b/presto-hive/src/test/java/com/facebook/presto/hive/TestHivePushdownFilterQueries.java index 023c2ad35ff11..5b0844835b8af 100644 --- a/presto-hive/src/test/java/com/facebook/presto/hive/TestHivePushdownFilterQueries.java +++ b/presto-hive/src/test/java/com/facebook/presto/hive/TestHivePushdownFilterQueries.java @@ -37,6 +37,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import static com.facebook.presto.SystemSessionProperties.PUSHDOWN_SUBFIELDS_ENABLED; import static com.facebook.presto.common.type.StandardTypes.BIGINT; import static com.facebook.presto.common.type.StandardTypes.BOOLEAN; import static com.facebook.presto.common.type.StandardTypes.DATE; @@ -1131,6 +1132,54 @@ public void testOptimizeWithWarningMessage() ImmutableSet.of(HIVE_TABLESCAN_CONVERTED_TO_VALUESNODE.toWarningCode())); } + //Test for issue https://github.com/prestodb/presto/issues/22690 + //Avoid negative index pushdown + @Test + public void testArraySubscriptPushdown() + { + Session session = enablePushdownFilterAndSubfield(getQueryRunner().getDefaultSession()); + getQueryRunner().execute(session, + "CREATE TABLE test_neg_array_sub_pushdown AS \n" + + "select ARRAY[10,20,30,40] numbers"); + + try { + assertQuery("select element_at(numbers,1) as number from test_neg_array_sub_pushdown", "SELECT 10"); + assertQuery("select element_at(numbers,2) as number from test_neg_array_sub_pushdown", "SELECT 20"); + assertQuery("select element_at(numbers,3) as number from test_neg_array_sub_pushdown", "SELECT 30"); + assertQuery("select element_at(numbers,-1) as number from test_neg_array_sub_pushdown", "SELECT 40"); + assertQuery("select element_at(numbers,-2) as number from test_neg_array_sub_pushdown", "SELECT 30"); + assertQuery("select element_at(numbers,-3) as number from test_neg_array_sub_pushdown", "SELECT 20"); + assertQueryFails("select element_at(numbers,0) as number from test_neg_array_sub_pushdown", "SQL array indices start at 1"); + + assertQuery("select numbers[1] as number from test_neg_array_sub_pushdown", "SELECT 10"); + assertQuery("select numbers[2] as number from test_neg_array_sub_pushdown", "SELECT 20"); + assertQuery("select numbers[3 ] as number from test_neg_array_sub_pushdown", "SELECT 30"); + assertQueryFails("select numbers[-1] as number from test_neg_array_sub_pushdown", "Array subscript is negative"); + assertQueryFails("select numbers[-2] as number from test_neg_array_sub_pushdown", "Array subscript is negative"); + assertQueryFails("select numbers[0] as number from test_neg_array_sub_pushdown", "SQL array indices start at 1"); + } + finally { + getQueryRunner().execute("DROP TABLE test_neg_array_sub_pushdown"); + } + } + + @Test + public void testArraySubscriptPushdownEmptyArray() + { + Session session = enablePushdownFilterAndSubfield(getQueryRunner().getDefaultSession()); + getQueryRunner().execute(session, + "CREATE TABLE test_neg_array_sub_pushdown ( numbers array(integer))"); + assertUpdate("INSERT into test_neg_array_sub_pushdown (numbers) values (ARRAY[])", 1); + + try { + assertQuery("select element_at(numbers,1) as number from test_neg_array_sub_pushdown", "SELECT NULL"); + assertQuery("select element_at(numbers,-2) as number from test_neg_array_sub_pushdown", "SELECT NULL"); + } + finally { + getQueryRunner().execute("DROP TABLE test_neg_array_sub_pushdown"); + } + } + private Path getPartitionDirectory(String tableName, String partitionClause) { String filePath = ((String) computeActual(noPushdownFilter(getSession()), format("SELECT \"$path\" FROM %s WHERE %s LIMIT 1", tableName, partitionClause)).getOnlyValue()) @@ -1218,4 +1267,12 @@ private static Session noPartialAggregationPushdown(Session session) .setCatalogSessionProperty(HIVE_CATALOG, PARTIAL_AGGREGATION_PUSHDOWN_ENABLED, "false") .build(); } + + private static Session enablePushdownFilterAndSubfield(Session session) + { + return Session.builder(session) + .setCatalogSessionProperty(HIVE_CATALOG, PUSHDOWN_FILTER_ENABLED, "true") + .setSystemProperty(PUSHDOWN_SUBFIELDS_ENABLED, "true") + .build(); + } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PushdownSubfields.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PushdownSubfields.java index ae3e869ec07a9..302aaefd19076 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PushdownSubfields.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/PushdownSubfields.java @@ -609,6 +609,12 @@ private static Optional toSubfield( return Optional.empty(); } if (index instanceof Number) { + //Fix for issue https://github.com/prestodb/presto/issues/22690 + //Avoid negative index pushdown + if (((Number) index).longValue() < 0) { + return Optional.empty(); + } + elements.add(new Subfield.LongSubscript(((Number) index).longValue())); expression = arguments.get(0); continue;