From 17df4298f92009d0f165bdfe8dbb909c757f3d07 Mon Sep 17 00:00:00 2001 From: Han Weng Date: Wed, 16 Dec 2020 14:54:06 -0500 Subject: [PATCH] Fix drop index failure and issues with execution plan cache --- .../io/hetu/core/heuristicindex/FileIndexWriter.java | 5 ++--- .../hetu/core/heuristicindex/HeuristicIndexClient.java | 5 ----- .../core/heuristicindex/TestIndexRecordManager.java | 10 +++++----- .../java/io/prestosql/plugin/hive/util/IndexCache.java | 2 +- .../java/io/prestosql/heuristicindex/IndexCache.java | 2 +- .../io/prestosql/query/CachedSqlQueryExecution.java | 3 ++- .../io/prestosql/security/IndexAccessControlRule.java | 1 + .../java/io/prestosql/testing/NoOpIndexWriter.java | 1 + .../prestosql/spi/connector/CreateIndexMetadata.java | 3 ++- .../io/prestosql/spi/heuristicindex/IndexRecord.java | 3 ++- 10 files changed, 17 insertions(+), 18 deletions(-) diff --git a/hetu-heuristic-index/src/main/java/io/hetu/core/heuristicindex/FileIndexWriter.java b/hetu-heuristic-index/src/main/java/io/hetu/core/heuristicindex/FileIndexWriter.java index 9a37ab9ae..5c37c2412 100644 --- a/hetu-heuristic-index/src/main/java/io/hetu/core/heuristicindex/FileIndexWriter.java +++ b/hetu-heuristic-index/src/main/java/io/hetu/core/heuristicindex/FileIndexWriter.java @@ -39,7 +39,6 @@ import java.util.Collections; import java.util.Comparator; import java.util.LinkedList; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; @@ -213,13 +212,13 @@ public class FileIndexWriter } // Create index and put values - try (Index index = HeuristicIndexFactory.createIndex(createIndexMetadata.getIndexType().toLowerCase(Locale.ENGLISH))) { + try (Index index = HeuristicIndexFactory.createIndex(createIndexMetadata.getIndexType())) { index.setProperties(createIndexMetadata.getProperties()); index.setExpectedNumOfEntries(expectedNumEntries); index.addValues(stripeData); // Persist one index (e.g. 3.bloom) - String indexFileName = offset + "." + index.getId().toLowerCase(Locale.ENGLISH); + String indexFileName = offset + "." + index.getId(); try (OutputStream os = LOCAL_FS_CLIENT.newOutputStream(tmpPath.resolve(indexFileName))) { index.serialize(os); } diff --git a/hetu-heuristic-index/src/main/java/io/hetu/core/heuristicindex/HeuristicIndexClient.java b/hetu-heuristic-index/src/main/java/io/hetu/core/heuristicindex/HeuristicIndexClient.java index a00972e56..b25e41500 100644 --- a/hetu-heuristic-index/src/main/java/io/hetu/core/heuristicindex/HeuristicIndexClient.java +++ b/hetu-heuristic-index/src/main/java/io/hetu/core/heuristicindex/HeuristicIndexClient.java @@ -17,8 +17,6 @@ package io.hetu.core.heuristicindex; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import io.airlift.log.Logger; -import io.hetu.core.filesystem.HetuLocalFileSystemClient; -import io.hetu.core.filesystem.LocalConfig; import io.hetu.core.heuristicindex.util.IndexConstants; import io.hetu.core.plugin.heuristicindex.index.btree.BTreeIndex; import io.prestosql.spi.connector.CreateIndexMetadata; @@ -42,7 +40,6 @@ import java.util.ArrayList; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.Properties; import java.util.concurrent.locks.Lock; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -58,8 +55,6 @@ import static java.util.Objects.requireNonNull; public class HeuristicIndexClient implements IndexClient { - private static final HetuFileSystemClient LOCAL_FS_CLIENT = new HetuLocalFileSystemClient( - new LocalConfig(new Properties()), Paths.get("/")); private static final Logger LOG = Logger.get(HeuristicIndexClient.class); private HetuFileSystemClient fs; diff --git a/hetu-heuristic-index/src/test/java/io/hetu/core/heuristicindex/TestIndexRecordManager.java b/hetu-heuristic-index/src/test/java/io/hetu/core/heuristicindex/TestIndexRecordManager.java index f10e094cf..995986e24 100644 --- a/hetu-heuristic-index/src/test/java/io/hetu/core/heuristicindex/TestIndexRecordManager.java +++ b/hetu-heuristic-index/src/test/java/io/hetu/core/heuristicindex/TestIndexRecordManager.java @@ -216,12 +216,12 @@ public class TestIndexRecordManager @Test public void testAddAndLookUp() - throws IOException, IllegalAccessException + throws IOException { - testIndexRecordAddLookUpHelper("testName", "testUser", "testTable", new String[] {"testColumn"}, "minmax", Collections.emptyList(), Collections.emptyList()); - testIndexRecordAddLookUpHelper("testName", "testUser", "testTable", new String[] {"testColumn", "testColumn2"}, "minmax", Collections.emptyList(), Collections.emptyList()); - testIndexRecordAddLookUpHelper("testName", "testUser", "testTable", new String[] {"testColumn"}, "minmax", Collections.emptyList(), ImmutableList.of("12")); - testIndexRecordAddLookUpHelper("testName", "testUser", "testTable", new String[] {"testColumn"}, "minmax", Collections.emptyList(), ImmutableList.of("12", "123")); + testIndexRecordAddLookUpHelper("testName", "testUser", "testTable", new String[] {"testColumn"}, "MINMAX", Collections.emptyList(), Collections.emptyList()); + testIndexRecordAddLookUpHelper("testName", "testUser", "testTable", new String[] {"testColumn", "testColumn2"}, "MINMAX", Collections.emptyList(), Collections.emptyList()); + testIndexRecordAddLookUpHelper("testName", "testUser", "testTable", new String[] {"testColumn"}, "MINMAX", Collections.emptyList(), ImmutableList.of("12")); + testIndexRecordAddLookUpHelper("testName", "testUser", "testTable", new String[] {"testColumn"}, "MINMAX", Collections.emptyList(), ImmutableList.of("12", "123")); } private void testIndexRecordAddLookUpHelper(String name, String user, String table, String[] columns, String indexType, List indexProperties, List partitions) diff --git a/presto-hive/src/main/java/io/prestosql/plugin/hive/util/IndexCache.java b/presto-hive/src/main/java/io/prestosql/plugin/hive/util/IndexCache.java index ea5db84f3..5baf8d83f 100644 --- a/presto-hive/src/main/java/io/prestosql/plugin/hive/util/IndexCache.java +++ b/presto-hive/src/main/java/io/prestosql/plugin/hive/util/IndexCache.java @@ -52,7 +52,7 @@ public class IndexCache { private static final Logger LOG = Logger.get(IndexCache.class); private static final ThreadFactory threadFactory = new ThreadFactoryBuilder().setNameFormat("Hive-IndexCache-pool-%d").setDaemon(true).build(); - protected static final List INDEX_TYPES = ImmutableList.of("minmax", "bloom", "bitmap"); + protected static final List INDEX_TYPES = ImmutableList.of("MINMAX", "BLOOM", "BITMAP"); private static ScheduledExecutorService executor; diff --git a/presto-main/src/main/java/io/prestosql/heuristicindex/IndexCache.java b/presto-main/src/main/java/io/prestosql/heuristicindex/IndexCache.java index 2772cd4b7..df74bd835 100644 --- a/presto-main/src/main/java/io/prestosql/heuristicindex/IndexCache.java +++ b/presto-main/src/main/java/io/prestosql/heuristicindex/IndexCache.java @@ -49,7 +49,7 @@ public class IndexCache { private static final Logger LOG = Logger.get(IndexCache.class); private static final ThreadFactory threadFactory = new ThreadFactoryBuilder().setNameFormat("Main-IndexCache-pool-%d").setDaemon(true).build(); - protected static final List INDEX_TYPES = ImmutableList.of("bloom", "minmax"); + protected static final List INDEX_TYPES = ImmutableList.of("BLOOM", "MINMAX"); private static ScheduledExecutorService executor; diff --git a/presto-main/src/main/java/io/prestosql/query/CachedSqlQueryExecution.java b/presto-main/src/main/java/io/prestosql/query/CachedSqlQueryExecution.java index 1910f3652..31458b236 100644 --- a/presto-main/src/main/java/io/prestosql/query/CachedSqlQueryExecution.java +++ b/presto-main/src/main/java/io/prestosql/query/CachedSqlQueryExecution.java @@ -157,7 +157,8 @@ public class CachedSqlQueryExecution isExecutionPlanCacheEnabled(session) && analysis.getParameters().isEmpty() && validateAndExtractTableAndColumns(analysis, metadata, session, tableNames, tableStatistics, columnTypes) && - isCacheable(statement); + isCacheable(statement) && + (!(analysis.getOriginalStatement() instanceof CreateIndex)); // create index should not be cached cacheable = cacheable && !tableNames.isEmpty(); if (!cacheable) { diff --git a/presto-main/src/main/java/io/prestosql/security/IndexAccessControlRule.java b/presto-main/src/main/java/io/prestosql/security/IndexAccessControlRule.java index c485c1266..b47073a67 100644 --- a/presto-main/src/main/java/io/prestosql/security/IndexAccessControlRule.java +++ b/presto-main/src/main/java/io/prestosql/security/IndexAccessControlRule.java @@ -1,4 +1,5 @@ /* + * Copyright (C) 2018-2020. Huawei Technologies Co., Ltd. All rights reserved. * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at diff --git a/presto-main/src/main/java/io/prestosql/testing/NoOpIndexWriter.java b/presto-main/src/main/java/io/prestosql/testing/NoOpIndexWriter.java index 80444336c..e4e6bacb7 100644 --- a/presto-main/src/main/java/io/prestosql/testing/NoOpIndexWriter.java +++ b/presto-main/src/main/java/io/prestosql/testing/NoOpIndexWriter.java @@ -1,4 +1,5 @@ /* + * Copyright (C) 2018-2020. Huawei Technologies Co., Ltd. All rights reserved. * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at diff --git a/presto-spi/src/main/java/io/prestosql/spi/connector/CreateIndexMetadata.java b/presto-spi/src/main/java/io/prestosql/spi/connector/CreateIndexMetadata.java index a62049c15..b6dd4ecbe 100644 --- a/presto-spi/src/main/java/io/prestosql/spi/connector/CreateIndexMetadata.java +++ b/presto-spi/src/main/java/io/prestosql/spi/connector/CreateIndexMetadata.java @@ -20,6 +20,7 @@ import io.prestosql.spi.heuristicindex.Index; import io.prestosql.spi.type.Type; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Properties; @@ -54,7 +55,7 @@ public class CreateIndexMetadata { this.indexName = checkNotEmpty(indexName, "indexName"); this.tableName = requireNonNull(tableName, "tableName is null"); - this.indexType = requireNonNull(indexType, "indexType is null"); + this.indexType = requireNonNull(indexType, "indexType is null").toUpperCase(Locale.ENGLISH); this.indexColumns = indexColumns; this.partitions = partitions; this.properties = properties; diff --git a/presto-spi/src/main/java/io/prestosql/spi/heuristicindex/IndexRecord.java b/presto-spi/src/main/java/io/prestosql/spi/heuristicindex/IndexRecord.java index 42a35fa2e..d6d86d291 100644 --- a/presto-spi/src/main/java/io/prestosql/spi/heuristicindex/IndexRecord.java +++ b/presto-spi/src/main/java/io/prestosql/spi/heuristicindex/IndexRecord.java @@ -16,6 +16,7 @@ package io.prestosql.spi.heuristicindex; import java.util.Arrays; import java.util.List; +import java.util.Locale; import java.util.Objects; import java.util.stream.Collectors; @@ -39,7 +40,7 @@ public class IndexRecord this.user = user == null ? "" : user; this.table = table; this.columns = columns; - this.indexType = indexType; + this.indexType = indexType.toUpperCase(Locale.ENGLISH); this.properties = properties; this.partitions = partitions; this.lastModifiedTime = System.currentTimeMillis();