From 497e3747770dd2c13281c66e40a03b12fe61b01d Mon Sep 17 00:00:00 2001 From: lukasz-stec Date: Wed, 8 Jan 2025 11:25:57 +0100 Subject: [PATCH 1/2] Add executeWithoutResults to StandaloneQueryRunner It is needed to test the query execution without reading the final query info which triggers the QueryCompletedEvent --- .../java/io/trino/testing/StandaloneQueryRunner.java | 10 ++++++++-- .../io/trino/testing/TestingDirectTrinoClient.java | 7 ++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/testing/StandaloneQueryRunner.java b/core/trino-main/src/main/java/io/trino/testing/StandaloneQueryRunner.java index 1bbeda0f0a2b..a93411a976f0 100644 --- a/core/trino-main/src/main/java/io/trino/testing/StandaloneQueryRunner.java +++ b/core/trino-main/src/main/java/io/trino/testing/StandaloneQueryRunner.java @@ -105,14 +105,20 @@ public List getSpans() @Override public MaterializedResult execute(Session session, @Language("SQL") String sql) { - return executeInternal(session, sql).result(); + return executeInternal(session, sql).result().get(); } @Override public MaterializedResultWithPlan executeWithPlan(Session session, String sql) { TestingDirectTrinoClient.Result result = executeInternal(session, sql); - return new MaterializedResultWithPlan(result.queryId(), server.getQueryPlan(result.queryId()), result.result()); + MaterializedResult materializedRows = result.result().get(); + return new MaterializedResultWithPlan(result.queryId(), server.getQueryPlan(result.queryId()), materializedRows); + } + + public TestingDirectTrinoClient.Result executeWithoutResults(Session session, String sql) + { + return executeInternal(session, sql); } private TestingDirectTrinoClient.Result executeInternal(Session session, @Language("SQL") String sql) diff --git a/core/trino-main/src/main/java/io/trino/testing/TestingDirectTrinoClient.java b/core/trino-main/src/main/java/io/trino/testing/TestingDirectTrinoClient.java index c73a3d1ca3f1..34ea8aa09588 100644 --- a/core/trino-main/src/main/java/io/trino/testing/TestingDirectTrinoClient.java +++ b/core/trino-main/src/main/java/io/trino/testing/TestingDirectTrinoClient.java @@ -37,6 +37,7 @@ import java.util.List; import java.util.Optional; import java.util.OptionalLong; +import java.util.function.Supplier; import static com.google.common.base.Preconditions.checkArgument; import static io.trino.execution.QueryState.FINISHED; @@ -63,7 +64,7 @@ public Result execute(SessionContext sessionContext, @Language("SQL") String sql { MaterializedQueryResultsListener queryResultsListener = new MaterializedQueryResultsListener(); DispatchQuery dispatchQuery = directTrinoClient.execute(sessionContext, sql, queryResultsListener); - return new Result(dispatchQuery.getQueryId(), toMaterializedRows(dispatchQuery, queryResultsListener.columnTypes(), queryResultsListener.columnNames(), queryResultsListener.pages())); + return new Result(dispatchQuery.getQueryId(), () -> toMaterializedRows(dispatchQuery, queryResultsListener.columnTypes(), queryResultsListener.columnNames(), queryResultsListener.pages())); } private static MaterializedResult toMaterializedRows(DispatchQuery dispatchQuery, List columnTypes, List columnNames, List pages) @@ -137,9 +138,9 @@ private static List toMaterializedRows(ConnectorSession session return rows.build(); } - record Result(QueryId queryId, MaterializedResult result) + public record Result(QueryId queryId, Supplier result) { - Result + public Result { requireNonNull(queryId, "queryId is null"); requireNonNull(result, "result is null"); From 95f4b16412ce8aa622fc78654dd434a1a179eefb Mon Sep 17 00:00:00 2001 From: lukasz-stec Date: Wed, 8 Jan 2025 11:30:05 +0100 Subject: [PATCH 2/2] Always fire QueryCompletedEvent for DDL queries Previously the event was fired because client protocol is reading the final query info. That is brittle and theoretically could be removed making DDL queries fail to trigger query completed event. --- .../execution/DataDefinitionExecution.java | 6 ++ .../execution/TestQueryCompletedEvent.java | 91 +++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 testing/trino-tests/src/test/java/io/trino/execution/TestQueryCompletedEvent.java diff --git a/core/trino-main/src/main/java/io/trino/execution/DataDefinitionExecution.java b/core/trino-main/src/main/java/io/trino/execution/DataDefinitionExecution.java index 1e8e43829335..3e8d7ad36e88 100644 --- a/core/trino-main/src/main/java/io/trino/execution/DataDefinitionExecution.java +++ b/core/trino-main/src/main/java/io/trino/execution/DataDefinitionExecution.java @@ -69,6 +69,12 @@ private DataDefinitionExecution( this.stateMachine = requireNonNull(stateMachine, "stateMachine is null"); this.parameters = parameters; this.warningCollector = requireNonNull(warningCollector, "warningCollector is null"); + stateMachine.addStateChangeListener(state -> { + if (state.isDone() && stateMachine.getFinalQueryInfo().isEmpty()) { + // make sure the final query info is set and listeners are triggered + stateMachine.updateQueryInfo(Optional.empty()); + } + }); } @Override diff --git a/testing/trino-tests/src/test/java/io/trino/execution/TestQueryCompletedEvent.java b/testing/trino-tests/src/test/java/io/trino/execution/TestQueryCompletedEvent.java new file mode 100644 index 000000000000..8d2ea9f5e537 --- /dev/null +++ b/testing/trino-tests/src/test/java/io/trino/execution/TestQueryCompletedEvent.java @@ -0,0 +1,91 @@ +/* + * 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.execution; + +import com.google.common.collect.ImmutableList; +import io.trino.dispatcher.DispatchQuery; +import io.trino.plugin.memory.MemoryPlugin; +import io.trino.spi.Plugin; +import io.trino.spi.eventlistener.EventListener; +import io.trino.spi.eventlistener.EventListenerFactory; +import io.trino.spi.eventlistener.QueryCompletedEvent; +import io.trino.testing.StandaloneQueryRunner; +import io.trino.testing.TestingDirectTrinoClient; +import org.junit.jupiter.api.Test; + +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import static io.trino.execution.QueryState.FINISHED; +import static io.trino.testing.TestingSession.testSession; +import static io.trino.testing.assertions.Assert.assertEventually; +import static java.util.Collections.synchronizedSet; +import static org.assertj.core.api.Assertions.assertThat; + +public class TestQueryCompletedEvent +{ + @Test + public void testQueryCompletedEventIssued() + { + try (StandaloneQueryRunner queryRunner = new StandaloneQueryRunner(testSession())) { + queryRunner.installPlugin(new MemoryPlugin()); + queryRunner.createCatalog("memory", "memory"); + Set queryCompletedQueryIds = synchronizedSet(new HashSet<>()); + EventListener listener = new EventListener() + { + @Override + public void queryCompleted(QueryCompletedEvent queryCompletedEvent) + { + queryCompletedQueryIds.add(queryCompletedEvent.getMetadata().getQueryId()); + } + }; + queryRunner.installPlugin(new Plugin() + { + @Override + public Iterable getEventListenerFactories() + { + return ImmutableList.of(new EventListenerFactory() + { + @Override + public String getName() + { + return "testQueryCompletedEventIssued"; + } + + @Override + public EventListener create(Map config, EventListenerContext context) + { + return listener; + } + }); + } + }); + + assertQueryCompletedIssued(queryRunner, queryCompletedQueryIds, "SELECT 1"); + assertQueryCompletedIssued(queryRunner, queryCompletedQueryIds, "CREATE SCHEMA memory.test_schema"); + assertQueryCompletedIssued(queryRunner, queryCompletedQueryIds, "DROP SCHEMA memory.test_schema"); + } + } + + private static void assertQueryCompletedIssued(StandaloneQueryRunner queryRunner, Set queryCompletedQueryIds, String sql) + { + TestingDirectTrinoClient.Result result = queryRunner.executeWithoutResults(testSession(), sql); + DispatchQuery query = queryRunner.getCoordinator().getDispatchManager().getQuery(result.queryId()); + assertThat(query.getState()).isEqualTo(FINISHED); + assertEventually(() -> assertThat(queryCompletedQueryIds) + .describedAs("query '%s'", sql) + .contains(result.queryId().getId())); + } +}