Skip to content

Commit

Permalink
Fix trino-cli to display plans for EXPLAIN ANALYZE
Browse files Browse the repository at this point in the history
  • Loading branch information
@homar @findepi
homar authored and findepi committed Sep 27, 2022
1 parent 0ece498 commit bc794cd
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 34 deletions.
17 changes: 14 additions & 3 deletions client/trino-cli/src/main/java/io/trino/cli/Query.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,12 @@ private boolean renderQueryOutput(Terminal terminal, PrintStream out, PrintStrea
if (client.isRunning() || (client.isFinished() && client.finalStatusInfo().getError() == null)) {
QueryStatusInfo results = client.isRunning() ? client.currentStatusInfo() : client.finalStatusInfo();
if (results.getUpdateType() != null) {
renderUpdate(errorChannel, results);
renderUpdate(errorChannel, results, outputFormat, usePager);
}
// TODO once https://github.com/trinodb/trino/issues/14253 is done this else here should be needed
// and should be replaced with just simple:
// if there is updateCount print it
// if there are columns(resultSet) then print it
else if (results.getColumns() == null) {
errorChannel.printf("Query %s has no columns\n", results.getId());
return false;
Expand Down Expand Up @@ -220,14 +224,21 @@ private void processInitialStatusUpdates(WarningsPrinter warningsPrinter)
warningsPrinter.print(warnings, false, true);
}

private void renderUpdate(PrintStream out, QueryStatusInfo results)
private void renderUpdate(PrintStream out, QueryStatusInfo results, OutputFormat outputFormat, boolean usePager)
{
String status = results.getUpdateType();
if (results.getUpdateCount() != null) {
long count = results.getUpdateCount();
status += format(": %s row%s", count, (count != 1) ? "s" : "");
out.println(status);
}
else if (results.getColumns() != null && !results.getColumns().isEmpty()) {
out.println(status);
renderResults(out, outputFormat, usePager, results.getColumns());
}
else {
out.println(status);
}
out.println(status);
discardResults();
}

Expand Down
5 changes: 1 addition & 4 deletions core/trino-main/src/main/java/io/trino/server/protocol/Query.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
import io.trino.spi.block.BlockEncodingSerde;
import io.trino.spi.exchange.ExchangeId;
import io.trino.spi.security.SelectedRole;
import io.trino.spi.type.BooleanType;
import io.trino.spi.type.Type;
import io.trino.transaction.TransactionId;

Expand Down Expand Up @@ -507,13 +506,11 @@ private synchronized QueryResults getNextResult(long token, UriInfo uriInfo, Dat

private synchronized QueryResultRows removePagesFromExchange(QueryInfo queryInfo, long targetResultBytes)
{
// For queries with no output, return a fake boolean result for clients that require it.
if (!resultsConsumed && queryInfo.getOutputStage().isEmpty()) {
return queryResultRowsBuilder(session)
.withSingleBooleanValue(createColumn("result", BooleanType.BOOLEAN, supportsParametricDateTime), true)
.withColumnsAndTypes(ImmutableList.of(), ImmutableList.of())
.build();
}

// Remove as many pages as possible from the exchange until just greater than DESIRED_RESULT_BYTES
// NOTE: it is critical that query results are created for the pages removed from the exchange
// client while holding the lock because the query may transition to the finished state when the
Expand Down
35 changes: 23 additions & 12 deletions plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import io.trino.testing.MaterializedResult;
import io.trino.testing.MaterializedRow;
import io.trino.testing.QueryRunner;
import org.intellij.lang.annotations.Language;
import org.testng.annotations.AfterTest;
import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;
Expand Down Expand Up @@ -75,8 +76,8 @@ public void testCreateSchema()
assertEquals(queryRunner.execute("SHOW SCHEMAS FROM blackhole").getRowCount(), 3);
assertThatQueryReturnsValue("CREATE TABLE test.test_schema as SELECT * FROM tpch.tiny.region", 5L);

assertThatQueryReturnsValue("DROP TABLE test_schema", true);
assertThatQueryReturnsValue("DROP TABLE test.test_schema", true);
assertThatQueryDoesNotReturnValues("DROP TABLE test_schema");
assertThatQueryDoesNotReturnValues("DROP TABLE test.test_schema");
}

@Test
Expand All @@ -87,7 +88,7 @@ public void createTableWhenTableIsAlreadyCreated()
assertThatThrownBy(() -> queryRunner.execute(createTableSql))
.isInstanceOf(RuntimeException.class)
.hasMessage("line 1:1: Destination table 'blackhole.default.nation' already exists");
assertThatQueryReturnsValue("DROP TABLE nation", true);
assertThatQueryDoesNotReturnValues("DROP TABLE nation");
}

@Test
Expand All @@ -105,7 +106,7 @@ public void blackHoleConnectorUsage()

assertThatQueryReturnsValue("SELECT count(*) FROM nation", 0L);

assertThatQueryReturnsValue("DROP TABLE nation", true);
assertThatQueryDoesNotReturnValues("DROP TABLE nation");
}

@Test
Expand All @@ -125,7 +126,7 @@ public void createTableWithDistribution()
assertThatQueryReturnsValue(
"CREATE TABLE distributed_test WITH ( distributed_on = array['orderkey'] ) AS SELECT * FROM tpch.tiny.orders",
15000L);
assertThatQueryReturnsValue("DROP TABLE distributed_test", true);
assertThatQueryDoesNotReturnValues("DROP TABLE distributed_test");
}

@Test
Expand Down Expand Up @@ -170,7 +171,7 @@ public void dataGenerationUsage()
assertEquals(row.getField(2), 0L);
assertEquals(row.getField(3), "****************");

assertThatQueryReturnsValue("DROP TABLE nation", true);
assertThatQueryDoesNotReturnValues("DROP TABLE nation");
}

@Test
Expand Down Expand Up @@ -201,7 +202,7 @@ public void fieldLength()
assertEquals(row.getField(3), "********");
assertEquals(row.getField(4), "***"); // this one is shorter due to column type being VARCHAR(3)

assertThatQueryReturnsValue("DROP TABLE nation", true);
assertThatQueryDoesNotReturnValues("DROP TABLE nation");
}

@Test
Expand Down Expand Up @@ -261,7 +262,7 @@ public void testSelectWithUnenforcedConstraint()

private void createBlackholeAllTypesTable()
{
assertThatQueryReturnsValue(
assertThatQueryDoesNotReturnValues(
format("CREATE TABLE blackhole_all_types (" +
" _varchar VARCHAR(10)" +
", _bigint BIGINT" +
Expand All @@ -279,13 +280,12 @@ private void createBlackholeAllTypesTable()
") WITH ( %s = 1, %s = 1, %s = 1 ) ",
ROWS_PER_PAGE_PROPERTY,
PAGES_PER_SPLIT_PROPERTY,
SPLIT_COUNT_PROPERTY),
true);
SPLIT_COUNT_PROPERTY));
}

private void dropBlackholeAllTypesTable()
{
assertThatQueryReturnsValue("DROP TABLE IF EXISTS blackhole_all_types", true);
assertThatQueryDoesNotReturnValues("DROP TABLE IF EXISTS blackhole_all_types");
}

@Test
Expand Down Expand Up @@ -318,7 +318,7 @@ public void pageProcessingDelay()
stopwatch.stop();
assertGreaterThan(stopwatch.elapsed(MILLISECONDS), pageProcessingDelay.toMillis());

assertThatQueryReturnsValue("DROP TABLE nation", true);
assertThatQueryDoesNotReturnValues("DROP TABLE nation");
}

private void assertThatNoBlackHoleTableIsCreated()
Expand Down Expand Up @@ -346,4 +346,15 @@ private void assertThatQueryReturnsValue(String sql, Object expected, Session se
assertEquals(value, expected);
assertEquals(Iterables.getOnlyElement(rows).getFieldCount(), 1);
}

private void assertThatQueryDoesNotReturnValues(String sql)
{
assertThatQueryDoesNotReturnValues(queryRunner.getDefaultSession(), sql);
}

private void assertThatQueryDoesNotReturnValues(Session session, @Language("SQL") String sql)
{
MaterializedResult rows = session == null ? queryRunner.execute(sql) : queryRunner.execute(session, sql);
assertEquals(rows.getRowCount(), 0);
}
}
2 changes: 1 addition & 1 deletion plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,7 @@ public void testCreatePartitionedTableWithQuotedIdentifierCasing(String columnNa
String tableName = "partitioning_" + randomTableSuffix();
@Language("SQL") String sql = format("CREATE TABLE %s (%s bigint) WITH (partitioning = ARRAY['%s'])", tableName, columnName, partitioningField);
if (success) {
assertThat(query(sql)).matches("VALUES (true)");
assertUpdate(sql);
dropTable(tableName);
}
else {
Expand Down
24 changes: 24 additions & 0 deletions testing/trino-product-tests/src/main/java/io/trino/tests/product/cli/TestTrinoCli.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,30 @@ public void testSetRole()
assertThat(trimLines(trino.readLinesUntilPrompt())).doesNotContain("admin");
}

@Test(groups = CLI, timeOut = TIMEOUT)
public void shouldPrintExplainAnalyzePlan()
throws Exception
{
launchTrinoCliWithServerArgument();
trino.waitForPrompt();
trino.getProcessInput().println("EXPLAIN ANALYZE CREATE TABLE iceberg.default.test_table AS SELECT * FROM hive.default.nation LIMIT 10;");
List<String> lines = trimLines(trino.readLinesUntilPrompt());
// TODO once https://github.com/trinodb/trino/issues/14253 is done this should be assertThat(lines).contains("CREATE TABLE: 1 row", "Query Plan");
assertThat(lines).contains("CREATE TABLE", "Query Plan");
// TODO once https://github.com/trinodb/trino/issues/14253 is done this should be assertThat(lines).contains("INSERT: 1 row", "Query Plan");
trino.getProcessInput().println("EXPLAIN ANALYZE INSERT INTO iceberg.default.test_table VALUES(100, 'URUGUAY', 3, 'test comment');");
lines = trimLines(trino.readLinesUntilPrompt());
assertThat(lines).contains("INSERT", "Query Plan");
// TODO once https://github.com/trinodb/trino/issues/14253 is done this should be assertThat(lines).contains("UPDATE: 1 row", "Query Plan");
trino.getProcessInput().println("EXPLAIN ANALYZE UPDATE iceberg.default.test_table SET n_comment = 'testValue 5' WHERE n_nationkey = 100;");
lines = trimLines(trino.readLinesUntilPrompt());
assertThat(lines).contains("UPDATE", "Query Plan");
// TODO once https://github.com/trinodb/trino/issues/14253 is done this should be assertThat(lines).contains("DELETE: 1 row", "Query Plan");
trino.getProcessInput().println("EXPLAIN ANALYZE DELETE FROM iceberg.default.test_table WHERE n_nationkey = 100;");
lines = trimLines(trino.readLinesUntilPrompt());
assertThat(lines).contains("DELETE", "Query Plan");
}

private void launchTrinoCliWithServerArgument(String... arguments)
throws IOException
{
Expand Down
26 changes: 16 additions & 10 deletions testing/trino-testing/src/main/java/io/trino/testing/AbstractTestEngineOnlyQueries.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5315,47 +5315,47 @@ public void testShowSession()
public void testSetSession()
{
MaterializedResult result = computeActual("SET SESSION test_string = 'bar'");
assertTrue((Boolean) getOnlyElement(result).getField(0));
assertNoRelationalResult(result);
assertEquals(result.getSetSessionProperties(), ImmutableMap.of("test_string", "bar"));

result = computeActual(format("SET SESSION %s.connector_long = 999", TESTING_CATALOG));
assertTrue((Boolean) getOnlyElement(result).getField(0));
assertNoRelationalResult(result);
assertEquals(result.getSetSessionProperties(), ImmutableMap.of(TESTING_CATALOG + ".connector_long", "999"));

result = computeActual(format("SET SESSION %s.connector_string = 'baz'", TESTING_CATALOG));
assertTrue((Boolean) getOnlyElement(result).getField(0));
assertNoRelationalResult(result);
assertEquals(result.getSetSessionProperties(), ImmutableMap.of(TESTING_CATALOG + ".connector_string", "baz"));

result = computeActual(format("SET SESSION %s.connector_string = 'ban' || 'ana'", TESTING_CATALOG));
assertTrue((Boolean) getOnlyElement(result).getField(0));
assertNoRelationalResult(result);
assertEquals(result.getSetSessionProperties(), ImmutableMap.of(TESTING_CATALOG + ".connector_string", "banana"));

result = computeActual(format("SET SESSION %s.connector_long = 444", TESTING_CATALOG));
assertTrue((Boolean) getOnlyElement(result).getField(0));
assertNoRelationalResult(result);
assertEquals(result.getSetSessionProperties(), ImmutableMap.of(TESTING_CATALOG + ".connector_long", "444"));

result = computeActual(format("SET SESSION %s.connector_long = 111 + 111", TESTING_CATALOG));
assertTrue((Boolean) getOnlyElement(result).getField(0));
assertNoRelationalResult(result);
assertEquals(result.getSetSessionProperties(), ImmutableMap.of(TESTING_CATALOG + ".connector_long", "222"));

result = computeActual(format("SET SESSION %s.connector_boolean = 111 < 3", TESTING_CATALOG));
assertTrue((Boolean) getOnlyElement(result).getField(0));
assertNoRelationalResult(result);
assertEquals(result.getSetSessionProperties(), ImmutableMap.of(TESTING_CATALOG + ".connector_boolean", "false"));

result = computeActual(format("SET SESSION %s.connector_double = 11.1", TESTING_CATALOG));
assertTrue((Boolean) getOnlyElement(result).getField(0));
assertNoRelationalResult(result);
assertEquals(result.getSetSessionProperties(), ImmutableMap.of(TESTING_CATALOG + ".connector_double", "11.1"));
}

@Test
public void testResetSession()
{
MaterializedResult result = computeActual(getSession(), "RESET SESSION test_string");
assertTrue((Boolean) getOnlyElement(result).getField(0));
assertNoRelationalResult(result);
assertEquals(result.getResetSessionProperties(), ImmutableSet.of("test_string"));

result = computeActual(getSession(), format("RESET SESSION %s.connector_string", TESTING_CATALOG));
assertTrue((Boolean) getOnlyElement(result).getField(0));
assertNoRelationalResult(result);
assertEquals(result.getResetSessionProperties(), ImmutableSet.of(TESTING_CATALOG + ".connector_string"));
}

Expand Down Expand Up @@ -6535,4 +6535,10 @@ private static ZonedDateTime zonedDateTime(String value)
{
return ZONED_DATE_TIME_FORMAT.parse(value, ZonedDateTime::from);
}

private void assertNoRelationalResult(MaterializedResult result)
{
assertThat(result.getMaterializedRows()).isEmpty();
assertThat(result.getTypes()).isEmpty(); // i.e. no columns
}
}
4 changes: 2 additions & 2 deletions testing/trino-tests/src/test/java/io/trino/execution/TestGrantOnSchema.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ public void testValidGrantWithGrantOption(String privilege)
queryRunner.execute(admin, format("GRANT %s ON SCHEMA default TO %s WITH GRANT OPTION", privilege, username));

assertThat(assertions.query(user, "SHOW SCHEMAS FROM local")).matches("VALUES (VARCHAR 'information_schema'), (VARCHAR 'default')");
assertThat(assertions.query(user, format("GRANT %s ON SCHEMA default TO %s", privilege, randomUsername()))).matches("VALUES (BOOLEAN 'TRUE')");
assertThat(assertions.query(user, format("GRANT %s ON SCHEMA default TO %s WITH GRANT OPTION", privilege, randomUsername()))).matches("VALUES (BOOLEAN 'TRUE')");
assertions.query(user, format("GRANT %s ON SCHEMA default TO %s", privilege, randomUsername()));
assertions.query(user, format("GRANT %s ON SCHEMA default TO %s WITH GRANT OPTION", privilege, randomUsername()));
}

@Test(dataProvider = "privileges")
Expand Down
7 changes: 5 additions & 2 deletions testing/trino-tests/src/test/java/io/trino/execution/TestGrantOnTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,12 @@
import org.testng.annotations.Test;

import java.util.EnumSet;
import java.util.Optional;
import java.util.OptionalLong;

import static io.trino.common.Randoms.randomUsername;
import static io.trino.spi.security.PrincipalType.USER;
import static io.trino.testing.QueryAssertions.assertUpdate;
import static io.trino.testing.TestingSession.testSessionBuilder;
import static java.lang.String.format;
import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -105,8 +108,8 @@ public void testValidGrantWithGrantOption(String privilege)
queryRunner.execute(admin, format("GRANT %s ON TABLE table_one TO %s WITH GRANT OPTION", privilege, username));

assertThat(assertions.query(user, "SHOW TABLES FROM default")).matches("VALUES (VARCHAR 'table_one')");
assertThat(assertions.query(user, format("GRANT %s ON TABLE table_one TO %s", privilege, randomUsername()))).matches("VALUES (BOOLEAN 'TRUE')");
assertThat(assertions.query(user, format("GRANT %s ON TABLE table_one TO %s WITH GRANT OPTION", privilege, randomUsername()))).matches("VALUES (BOOLEAN 'TRUE')");
assertUpdate(queryRunner, user, format("GRANT %s ON TABLE table_one TO %s", privilege, randomUsername()), OptionalLong.empty(), Optional.empty());
assertUpdate(queryRunner, user, format("GRANT %s ON TABLE table_one TO %s WITH GRANT OPTION", privilege, randomUsername()), OptionalLong.empty(), Optional.empty());
}

@Test(dataProvider = "privileges")
Expand Down

0 comments on commit bc794cd

Please sign in to comment.