fix: case insensitive names. Fixes #10

Julien Ruaux · Julien Ruaux · commit 8ad52fe5534c · 2023-03-29T12:41:55.000-07:00
diff --git a/src/main/java/com/redis/trino/RediSearchSession.java b/src/main/java/com/redis/trino/RediSearchSession.java
@@ -31,6 +31,7 @@
 import static java.lang.String.format;
 import static java.util.Locale.ENGLISH;
 import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toSet;
 
 import java.io.File;
 import java.util.Collections;
@@ -40,6 +41,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
@@ -165,7 +167,7 @@ public List<HostAddress> getAddresses() {
 		return Collections.emptyList();
 	}
 
-	public Set<String> getAllTables() throws SchemaNotFoundException {
+	private Set<String> listIndexNames() throws SchemaNotFoundException {
 		ImmutableSet.Builder<String> builder = ImmutableSet.builder();
 		builder.addAll(connection.sync().ftList());
 		return builder.build();
@@ -179,13 +181,17 @@ public Set<String> getAllTables() throws SchemaNotFoundException {
 	 */
 	public RediSearchTable getTable(SchemaTableName tableName) throws TableNotFoundException {
 		try {
-			return tableCache.getUnchecked(tableName);
-		} catch (UncheckedExecutionException e) {
+			return tableCache.get(tableName, () -> loadTableSchema(tableName));
+		} catch (ExecutionException | UncheckedExecutionException e) {
 			throwIfInstanceOf(e.getCause(), TrinoException.class);
-			throw e;
+			throw new RuntimeException(e);
 		}
 	}
 
+	public Set<String> getAllTables() {
+		return listIndexNames().stream().collect(toSet());
+	}
+
 	@SuppressWarnings("unchecked")
 	public void createTable(SchemaTableName schemaTableName, List<RediSearchColumnHandle> columns) {
 		String index = index(schemaTableName);
@@ -214,7 +220,7 @@ private String toRemoteTableName(String tableName) {
 		if (!config.isCaseInsensitiveNames()) {
 			return tableName;
 		}
-		for (String remoteTableName : getAllTables()) {
+		for (String remoteTableName : listIndexNames()) {
 			if (tableName.equals(remoteTableName.toLowerCase(ENGLISH))) {
 				return remoteTableName;
 			}
@@ -233,7 +239,7 @@ public void dropColumn(SchemaTableName schemaTableName, String columnName) {
 	 * @throws TableNotFoundException if no index by that name was found
 	 */
 	private RediSearchTable loadTableSchema(SchemaTableName schemaTableName) throws TableNotFoundException {
-		String index = schemaTableName.getTableName();
+		String index = toRemoteTableName(schemaTableName.getTableName());
 		Optional<IndexInfo> indexInfoOptional = indexInfo(index);
 		if (indexInfoOptional.isEmpty()) {
 			throw new TableNotFoundException(schemaTableName, format("Index '%s' not found", index), null);
diff --git a/src/test/java/com/redis/trino/RediSearchLoader.java b/src/test/java/com/redis/trino/RediSearchLoader.java
@@ -20,8 +20,9 @@
 import com.redis.lettucemod.api.StatefulRedisModulesConnection;
 import com.redis.lettucemod.search.CreateOptions;
 import com.redis.lettucemod.search.Field;
-import com.redis.testcontainers.junit.RedisTestContext;
+import com.redis.lettucemod.util.RedisModulesUtils;
 
+import io.lettuce.core.AbstractRedisClient;
 import io.lettuce.core.LettuceFutures;
 import io.lettuce.core.RedisFuture;
 import io.trino.Session;
@@ -35,15 +36,16 @@
 import io.trino.testing.ResultsSession;
 
 public class RediSearchLoader extends AbstractTestingTrinoClient<Void> {
+
 	private final String tableName;
-	private final RedisTestContext context;
+	private final StatefulRedisModulesConnection<String, String> connection;
 
-	public RediSearchLoader(RedisTestContext context, String tableName, TestingTrinoServer trinoServer,
+	public RediSearchLoader(AbstractRedisClient client, String tableName, TestingTrinoServer trinoServer,
 			Session defaultSession) {
 		super(trinoServer, defaultSession);
-
+		requireNonNull(client, "client is null");
+		this.connection = RedisModulesUtils.connection(client);
 		this.tableName = requireNonNull(tableName, "tableName is null");
-		this.context = requireNonNull(context, "client is null");
 	}
 
 	@Override
@@ -52,6 +54,12 @@ public ResultsSession<Void> getResultSession(Session session) {
 		return new RediSearchLoadingSession();
 	}
 
+	@Override
+	public void close() {
+		connection.close();
+		super.close();
+	}
+
 	private class RediSearchLoadingSession implements ResultsSession<Void> {
 
 		private final AtomicReference<List<Type>> types = new AtomicReference<>();
@@ -71,17 +79,16 @@ public void addResults(QueryStatusInfo statusInfo, QueryData data) {
 			}
 			checkState(types.get() != null, "Type information is missing");
 			List<Column> columns = statusInfo.getColumns();
-			if (!context.sync().ftList().contains(tableName)) {
+			if (!connection.sync().ftList().contains(tableName)) {
 				List<Field<String>> schema = new ArrayList<>();
 				for (int i = 0; i < columns.size(); i++) {
 					Type type = types.get().get(i);
 					schema.add(field(columns.get(i).getName(), type));
 				}
-				context.sync().ftCreate(tableName,
+				connection.sync().ftCreate(tableName,
 						CreateOptions.<String, String>builder().prefix(tableName + ":").build(),
 						schema.toArray(Field[]::new));
 			}
-			StatefulRedisModulesConnection<String, String> connection = context.getConnection();
 			connection.setAutoFlushCommands(false);
 			try {
 				UlidFactory factory = UlidFactory.newInstance(new Random());
@@ -94,10 +101,10 @@ public void addResults(QueryStatusInfo statusInfo, QueryData data) {
 						String value = convertValue(fields.get(i), type);
 						map.put(columns.get(i).getName(), value);
 					}
-					futures.add(context.async().hset(key, map));
+					futures.add(connection.async().hset(key, map));
 				}
 				connection.flushCommands();
-				LettuceFutures.awaitAll(context.getConnection().getTimeout(), futures.toArray(new RedisFuture[0]));
+				LettuceFutures.awaitAll(connection.getTimeout(), futures.toArray(new RedisFuture[0]));
 			} finally {
 				connection.setAutoFlushCommands(true);
 			}
diff --git a/src/test/java/com/redis/trino/RediSearchQueryRunner.java b/src/test/java/com/redis/trino/RediSearchQueryRunner.java
@@ -88,19 +88,20 @@ public static <T extends Throwable> T closeAllSuppress(T rootCause, AutoCloseabl
 	private static void installRediSearchPlugin(RediSearchServer server, QueryRunner queryRunner,
 			RediSearchConnectorFactory factory, Map<String, String> extraConnectorProperties) {
 		queryRunner.installPlugin(new RediSearchPlugin(factory));
-		Map<String, String> config = ImmutableMap.<String, String>builder()
-				.put("redisearch.uri", server.getTestContext().getRedisURI()).put("redisearch.default-limit", "100000")
-				.put("redisearch.default-schema-name", TPCH_SCHEMA).putAll(extraConnectorProperties).build();
+		Map<String, String> config = ImmutableMap.<String, String>builder().put("redisearch.uri", server.getRedisURI())
+				.put("redisearch.default-limit", "100000").put("redisearch.default-schema-name", TPCH_SCHEMA)
+				.putAll(extraConnectorProperties).build();
 		queryRunner.createCatalog("redisearch", "redisearch", config);
 	}
 
 	private static void loadTpchTopic(RediSearchServer server, TestingTrinoClient trinoClient, TpchTable<?> table) {
 		long start = System.nanoTime();
 		LOG.info("Running import for %s", table.getTableName());
-		RediSearchLoader loader = new RediSearchLoader(server.getTestContext(),
-				table.getTableName().toLowerCase(ENGLISH), trinoClient.getServer(), trinoClient.getDefaultSession());
-		loader.execute(format("SELECT * from %s",
-				new QualifiedObjectName(TPCH_SCHEMA, TINY_SCHEMA_NAME, table.getTableName().toLowerCase(ENGLISH))));
+		try (RediSearchLoader loader = new RediSearchLoader(server.getClient(),
+				table.getTableName().toLowerCase(ENGLISH), trinoClient.getServer(), trinoClient.getDefaultSession())) {
+			loader.execute(format("SELECT * from %s",
+					new QualifiedObjectName(TPCH_SCHEMA, TINY_SCHEMA_NAME, table.getTableName().toLowerCase(ENGLISH))));
+		}
 		LOG.info("Imported %s in %s s", table.getTableName(), Duration.ofNanos(System.nanoTime() - start).toSeconds());
 	}
 
diff --git a/src/test/java/com/redis/trino/RediSearchServer.java b/src/test/java/com/redis/trino/RediSearchServer.java
@@ -2,29 +2,49 @@
 
 import java.io.Closeable;
 
+import org.testcontainers.utility.DockerImageName;
+
+import com.redis.lettucemod.api.StatefulRedisModulesConnection;
+import com.redis.lettucemod.util.ClientBuilder;
+import com.redis.lettucemod.util.RedisModulesUtils;
 import com.redis.testcontainers.RedisStackContainer;
-import com.redis.testcontainers.junit.RedisTestContext;
+
+import io.lettuce.core.AbstractRedisClient;
+import io.lettuce.core.RedisURI;
 
 public class RediSearchServer implements Closeable {
 
-	private final RedisStackContainer dockerContainer;
-	private final RedisTestContext context;
+	private static final DockerImageName DOCKER_IMAGE_NAME = RedisStackContainer.DEFAULT_IMAGE_NAME
+			.withTag(RedisStackContainer.DEFAULT_TAG);
+	private final RedisStackContainer container = new RedisStackContainer(DOCKER_IMAGE_NAME).withEnv("REDISEARCH_ARGS",
+			"MAXAGGREGATERESULTS -1");
+	private final AbstractRedisClient client;
+	private final StatefulRedisModulesConnection<String, String> connection;
 
 	public RediSearchServer() {
-		this.dockerContainer = new RedisStackContainer(
-				RedisStackContainer.DEFAULT_IMAGE_NAME.withTag(RedisStackContainer.DEFAULT_TAG));
-		this.dockerContainer.withEnv("REDISEARCH_ARGS", "MAXAGGREGATERESULTS -1");
-		this.dockerContainer.start();
-		this.context = new RedisTestContext(dockerContainer);
+		this.container.start();
+		this.client = ClientBuilder.create(RedisURI.create(container.getRedisURI())).cluster(container.isCluster())
+				.build();
+		this.connection = RedisModulesUtils.connection(client);
+	}
+
+	public String getRedisURI() {
+		return container.getRedisURI();
+	}
+
+	public AbstractRedisClient getClient() {
+		return client;
 	}
 
-	public RedisTestContext getTestContext() {
-		return context;
+	public StatefulRedisModulesConnection<String, String> getConnection() {
+		return connection;
 	}
 
 	@Override
 	public void close() {
-		context.close();
-		dockerContainer.close();
+		connection.close();
+		client.shutdown();
+		client.getResources().shutdown();
+		container.close();
 	}
 }
diff --git a/src/test/java/com/redis/trino/TestConfig.java b/src/test/java/com/redis/trino/TestConfig.java
@@ -12,7 +12,7 @@
 
 import io.airlift.configuration.ConfigurationFactory;
 
-public class TestRediSearchConfig {
+public class TestConfig {
 
 	@Test
 	public void testDefaults() {
diff --git a/src/test/java/com/redis/trino/TestConnectorSmokeTest.java b/src/test/java/com/redis/trino/TestConnectorSmokeTest.java
@@ -20,10 +20,12 @@
 
 import com.google.common.base.Throwables;
 import com.redis.lettucemod.Beers;
+import com.redis.lettucemod.api.StatefulRedisModulesConnection;
 import com.redis.lettucemod.api.sync.RedisModulesCommands;
 import com.redis.lettucemod.search.CreateOptions;
 import com.redis.lettucemod.search.CreateOptions.DataType;
 import com.redis.lettucemod.search.Field;
+import com.redis.lettucemod.util.RedisModulesUtils;
 
 import io.airlift.log.Logger;
 import io.trino.spi.TrinoException;
@@ -33,9 +35,9 @@
 import io.trino.testing.TestingConnectorBehavior;
 import io.trino.testing.sql.TestTable;
 
-public class TestRediSearchConnectorSmokeTest extends BaseConnectorSmokeTest {
+public class TestConnectorSmokeTest extends BaseConnectorSmokeTest {
 
-	private static final Logger log = Logger.get(TestRediSearchConnectorSmokeTest.class);
+	private static final Logger log = Logger.get(TestConnectorSmokeTest.class);
 
 	private RediSearchServer redisearch;
 
@@ -46,7 +48,10 @@ protected QueryRunner createQueryRunner() throws Exception {
 	}
 
 	private void populateBeers() throws IOException, InterruptedException {
-		Beers.populateIndex(redisearch.getTestContext().getConnection());
+		try (StatefulRedisModulesConnection<String, String> connection = RedisModulesUtils
+				.connection(redisearch.getClient())) {
+			Beers.populateIndex(connection);
+		}
 	}
 
 	@Override
@@ -121,14 +126,14 @@ public void testRediSearchFields() throws IOException, InterruptedException {
 	public void testCountEmptyIndex() throws IOException, InterruptedException {
 		String index = "emptyidx";
 		CreateOptions<String, String> options = CreateOptions.<String, String>builder().prefix(index + ":").build();
-		redisearch.getTestContext().sync().ftCreate(index, options, Field.tag("field1").build());
+		redisearch.getConnection().sync().ftCreate(index, options, Field.tag("field1").build());
 		assertQuery("SELECT count(*) FROM " + index, "VALUES 0");
 	}
 
 	@SuppressWarnings("unchecked")
 	@Test
 	public void testJsonSearch() throws IOException {
-		RedisModulesCommands<String, String> sync = redisearch.getTestContext().getConnection().sync();
+		RedisModulesCommands<String, String> sync = redisearch.getConnection().sync();
 		sync.ftCreate("jsontest", CreateOptions.<String, String>builder().on(DataType.JSON).build(),
 				Field.tag("$.id").as("id").build(), Field.text("$.message").as("message").build());
 		sync.jsonSet("doc:1", "$", "{\"id\": \"1\", \"message\": \"this is a test\"}");
@@ -152,11 +157,11 @@ public void testInsertIndex() throws IOException, InterruptedException {
 		String index = "insertidx";
 		String prefix = index + ":";
 		CreateOptions<String, String> options = CreateOptions.<String, String>builder().prefix(prefix).build();
-		redisearch.getTestContext().sync().ftCreate(index, options, Field.tag("id").build(), Field.tag("name").build());
+		redisearch.getConnection().sync().ftCreate(index, options, Field.tag("id").build(), Field.tag("name").build());
 		assertUpdate(String.format("INSERT INTO %s (id, name) VALUES ('abc', 'mybeer')", index), 1);
 		assertThat(query(String.format("SELECT id, name FROM %s", index)))
 				.matches("VALUES (VARCHAR 'abc', VARCHAR 'mybeer')");
-		List<String> keys = redisearch.getTestContext().sync().keys(prefix + "*");
+		List<String> keys = redisearch.getConnection().sync().keys(prefix + "*");
 		assertEquals(keys.size(), 1);
 		assertTrue(keys.get(0).startsWith(prefix));
 	}
@@ -167,8 +172,8 @@ public final void destroy() {
 	}
 
 	static RuntimeException getTrinoExceptionCause(Throwable e) {
-		return Throwables.getCausalChain(e).stream().filter(TestRediSearchConnectorSmokeTest::isTrinoException)
-				.findFirst().map(RuntimeException.class::cast)
+		return Throwables.getCausalChain(e).stream().filter(TestConnectorSmokeTest::isTrinoException).findFirst()
+				.map(RuntimeException.class::cast)
 				.orElseThrow(() -> new IllegalArgumentException("Exception does not have TrinoException cause", e));
 	}
 
diff --git a/src/test/java/com/redis/trino/TestPlugin.java b/src/test/java/com/redis/trino/TestPlugin.java
@@ -13,7 +13,7 @@
 import io.trino.spi.connector.ConnectorFactory;
 import io.trino.testing.TestingConnectorContext;
 
-public class TestRediSearchPlugin {
+public class TestPlugin {
 
 	private RediSearchServer server;
 
@@ -27,8 +27,7 @@ public void testCreateConnector() {
 		RediSearchPlugin plugin = new RediSearchPlugin();
 
 		ConnectorFactory factory = getOnlyElement(plugin.getConnectorFactories());
-		Connector connector = factory.create("test",
-				ImmutableMap.of("redisearch.uri", server.getTestContext().getRedisURI()),
+		Connector connector = factory.create("test", ImmutableMap.of("redisearch.uri", server.getRedisURI()),
 				new TestingConnectorContext());
 
 		assertFalse(plugin.getTypes().iterator().hasNext());
diff --git a/src/test/java/com/redis/trino/TestQueryBuilder.java b/src/test/java/com/redis/trino/TestQueryBuilder.java
@@ -20,7 +20,7 @@
 import io.trino.spi.predicate.ValueSet;
 import io.trino.spi.type.DoubleType;
 
-public class TestRediSearchQueryBuilder {
+public class TestQueryBuilder {
 
 	private static final RediSearchColumnHandle COL1 = new RediSearchColumnHandle("col1", BIGINT, Field.Type.NUMERIC,
 			false, true);
diff --git a/src/test/java/com/redis/trino/TestTableHandle.java b/src/test/java/com/redis/trino/TestTableHandle.java
@@ -7,7 +7,8 @@
 import io.airlift.json.JsonCodec;
 import io.trino.spi.connector.SchemaTableName;
 
-public class TestRediSearchTableHandle {
+public class TestTableHandle {
+
 	private final JsonCodec<RediSearchTableHandle> codec = JsonCodec.jsonCodec(RediSearchTableHandle.class);
 
 	@Test
