Skip to content

Commit

Permalink
Fixed Functional Test Failures (prestodb#24123)
Browse files Browse the repository at this point in the history
Fix functional test failures

Fixed mount issues and docker host for running in pipeline
  • Loading branch information
abevk2023 authored Dec 16, 2024
1 parent 2183c0a commit 33d7c03
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 30 deletions.
6 changes: 6 additions & 0 deletions presto-native-execution/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,12 @@
<artifactId>presto-jdbc</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
<version>3.14.0</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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);
Expand All @@ -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)));
}
Expand All @@ -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));
}

Expand Down Expand Up @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand All @@ -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);
Expand All @@ -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<TypeSignatureParameter> 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
Expand Down

0 comments on commit 33d7c03

Please sign in to comment.