From 33d7c035be62a4aca32e8eb5ef0ecb16f8234a6c Mon Sep 17 00:00:00 2001 From: Abe Varghese <148206484+abevk2023@users.noreply.github.com> Date: Tue, 17 Dec 2024 02:36:56 +0530 Subject: [PATCH] Fixed Functional Test Failures (#24123) Fix functional test failures Fixed mount issues and docker host for running in pipeline --- presto-native-execution/pom.xml | 6 +++ .../nativeworker/ContainerQueryRunner.java | 39 ++++++++++--------- .../ContainerQueryRunnerUtils.java | 25 ++++++------ 3 files changed, 40 insertions(+), 30 deletions(-) diff --git a/presto-native-execution/pom.xml b/presto-native-execution/pom.xml index 3dcfe4a02a1d2..79271d2bb3cad 100644 --- a/presto-native-execution/pom.xml +++ b/presto-native-execution/pom.xml @@ -244,6 +244,12 @@ presto-jdbc test + + org.apache.commons + commons-lang3 + 3.14.0 + test + diff --git a/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/ContainerQueryRunner.java b/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/ContainerQueryRunner.java index d32eea55c3929..9cd88e2ade8db 100644 --- a/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/ContainerQueryRunner.java +++ b/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/ContainerQueryRunner.java @@ -30,14 +30,14 @@ import com.facebook.presto.testing.QueryRunner; import com.facebook.presto.testing.TestingAccessControlManager; import com.facebook.presto.transaction.TransactionManager; -import org.testcontainers.containers.BindMode; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.Network; import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.utility.MountableFile; import java.io.IOException; import java.sql.Connection; -import java.sql.DriverManager; +import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; import java.time.Duration; @@ -50,6 +50,7 @@ import java.util.logging.Logger; import static com.facebook.presto.testing.TestingSession.testSessionBuilder; +import static java.sql.DriverManager.getConnection; public class ContainerQueryRunner implements QueryRunner @@ -72,7 +73,6 @@ public class ContainerQueryRunner private final String schema; private final int numberOfWorkers; private Connection connection; - private Statement statement; public ContainerQueryRunner() throws InterruptedException, IOException @@ -97,19 +97,20 @@ public ContainerQueryRunner(int coordinatorPort, String catalog, String schema, coordinator.start(); workers.forEach(GenericContainer::start); - logger.info("Presto UI is accessible at http://localhost:" + coordinator.getMappedPort(coordinatorPort)); - TimeUnit.SECONDS.sleep(5); - String url = String.format("jdbc:presto://localhost:%s/%s/%s?%s", + String dockerHostIp = coordinator.getHost(); + logger.info("Presto UI is accessible at http://" + dockerHostIp + ":" + coordinator.getMappedPort(coordinatorPort)); + + String url = String.format("jdbc:presto://%s:%s/%s/%s?%s", + dockerHostIp, coordinator.getMappedPort(coordinatorPort), catalog, schema, "timeZoneId=UTC"); try { - Connection connection = DriverManager.getConnection(url, "test", null); - statement = connection.createStatement(); + connection = getConnection(url, "test", null); } catch (SQLException e) { throw new RuntimeException(e); @@ -134,9 +135,10 @@ private GenericContainer createCoordinator() return new GenericContainer<>(PRESTO_COORDINATOR_IMAGE) .withExposedPorts(coordinatorPort) - .withNetwork(network).withNetworkAliases("presto-coordinator") - .withFileSystemBind(BASE_DIR + "/testcontainers/coordinator/etc", "/opt/presto-server/etc", BindMode.READ_WRITE) - .withFileSystemBind(BASE_DIR + "/testcontainers/coordinator/entrypoint.sh", "/opt/entrypoint.sh", BindMode.READ_ONLY) + .withNetwork(network) + .withNetworkAliases("presto-coordinator") + .withCopyFileToContainer(MountableFile.forHostPath(BASE_DIR + "/testcontainers/coordinator/etc"), "/opt/presto-server/etc") + .withCopyFileToContainer(MountableFile.forHostPath(BASE_DIR + "/testcontainers/coordinator/entrypoint.sh"), "/opt/entrypoint.sh") .waitingFor(Wait.forLogMessage(".*======== SERVER STARTED ========.*", 1)) .withStartupTimeout(Duration.ofSeconds(Long.parseLong(CONTAINER_TIMEOUT))); } @@ -151,9 +153,10 @@ private GenericContainer createNativeWorker(int port, String nodeId) ContainerQueryRunnerUtils.createNativeWorkerVeloxProperties(nodeId); return new GenericContainer<>(PRESTO_WORKER_IMAGE) .withExposedPorts(port) - .withNetwork(network).withNetworkAliases(nodeId) - .withFileSystemBind(BASE_DIR + "/testcontainers/" + nodeId + "/etc", "/opt/presto-server/etc", BindMode.READ_ONLY) - .withFileSystemBind(BASE_DIR + "/testcontainers/" + nodeId + "/entrypoint.sh", "/opt/entrypoint.sh", BindMode.READ_ONLY) + .withNetwork(network) + .withNetworkAliases(nodeId) + .withCopyFileToContainer(MountableFile.forHostPath(BASE_DIR + "/testcontainers/" + nodeId + "/etc"), "/opt/presto-server/etc") + .withCopyFileToContainer(MountableFile.forHostPath(BASE_DIR + "/testcontainers/" + nodeId + "/entrypoint.sh"), "/opt/entrypoint.sh") .waitingFor(Wait.forLogMessage(".*Announcement succeeded: HTTP 202.*", 1)); } @@ -297,12 +300,12 @@ public Session getDefaultSession() public MaterializedResult execute(Session session, String sql) { try { - return ContainerQueryRunnerUtils - .toMaterializedResult( - statement.executeQuery(sql)); + Statement statement = connection.createStatement(); + ResultSet resultSet = statement.executeQuery(sql); + return ContainerQueryRunnerUtils.toMaterializedResult(resultSet); } catch (SQLException e) { - throw new RuntimeException(e); + throw new RuntimeException("Error executing query: " + sql, e); } } } diff --git a/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/ContainerQueryRunnerUtils.java b/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/ContainerQueryRunnerUtils.java index 6f29f57b3da9c..3a6a98da1ab89 100644 --- a/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/ContainerQueryRunnerUtils.java +++ b/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/ContainerQueryRunnerUtils.java @@ -28,6 +28,8 @@ import com.facebook.presto.common.type.TimestampWithTimeZoneType; import com.facebook.presto.common.type.TinyintType; import com.facebook.presto.common.type.Type; +import com.facebook.presto.common.type.TypeSignature; +import com.facebook.presto.common.type.TypeSignatureParameter; import com.facebook.presto.common.type.UnknownType; import com.facebook.presto.common.type.VarbinaryType; import com.facebook.presto.common.type.VarcharType; @@ -204,10 +206,6 @@ public static void createPropertiesFile(String filePath, Properties properties) parentDir.mkdirs(); } - if (file.exists()) { - throw new IOException("File exists: " + filePath); - } - try (OutputStreamWriter writer = new OutputStreamWriter(new FileOutputStream(file), StandardCharsets.UTF_8)) { for (String key : properties.stringPropertyNames()) { writer.write(key + "=" + properties.getProperty(key) + "\n"); @@ -224,10 +222,6 @@ public static void createScriptFile(String filePath, String scriptContent) parentDir.mkdirs(); } - if (file.exists()) { - throw new IOException("File exists: " + filePath); - } - try (OutputStream output = new FileOutputStream(file); OutputStreamWriter writer = new OutputStreamWriter(output, StandardCharsets.UTF_8)) { writer.write(scriptContent); @@ -251,10 +245,17 @@ public static MaterializedResult toMaterializedResult(ResultSet resultSet) String typeName = metaData.getColumnTypeName(i); if (sqlType == java.sql.Types.ARRAY) { - // Get the base type of the array elements - String arrayElementTypeName = metaData.getColumnTypeName(i); - Type elementType = mapSqlTypeNameToType(arrayElementTypeName); - types.add(new ArrayType(elementType)); + TypeSignature typeSignature = TypeSignature.parseTypeSignature(typeName); + List parameters = typeSignature.getParameters(); + + if (parameters.size() == 1) { + TypeSignature baseTypeSignature = parameters.get(0).getTypeSignature(); + Type elementType = mapSqlTypeNameToType(baseTypeSignature.toString()); + types.add(new ArrayType(elementType)); + } + else { + throw new UnsupportedOperationException("Unsupported ARRAY type with multiple parameters: " + typeName); + } } else if (sqlType == java.sql.Types.STRUCT) { // Handle STRUCT types if necessary