Skip to content

Rename listTables to listRelations to avoid confusion #19920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
findepi wants to merge 4 commits into from
Closed

Rename listTables to listRelations to avoid confusion #19920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d86f047
Rename listTables to listRelations
findepi Nov 20, 2023
107504d
Rename listTables to listRelations in Hive
findepi Nov 20, 2023
89a2bef
empty: roll the dice 🎲
findepi Nov 28, 2023
920834e
empty: roll the dice 🎲
findepi Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions client/trino-jdbc/src/test/java/io/trino/jdbc/TestTrinoDatabaseMetaData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1330,7 +1330,7 @@ public void testGetColumnsMetadataCalls()
list(list(COUNTING_CATALOG, "test_schema1", "test_table1", "column_17", "varchar")),
ImmutableMultiset.<String>builder()
.add("ConnectorMetadata.listSchemaNames")
.add("ConnectorMetadata.listTables(schema=test_schema1)")
.add("ConnectorMetadata.listRelations(schema=test_schema1)")
.addCopies("ConnectorMetadata.getSystemTable(schema=test_schema1, table=test_table1)", 4)
.add("ConnectorMetadata.getMaterializedView(schema=test_schema1, table=test_table1)")
.add("ConnectorMetadata.getView(schema=test_schema1, table=test_table1)")
Expand All @@ -1350,7 +1350,7 @@ public void testGetColumnsMetadataCalls()
.collect(toImmutableList()),
ImmutableMultiset.<String>builder()
.add("ConnectorMetadata.listSchemaNames")
.add("ConnectorMetadata.listTables(schema=test_schema1)")
.add("ConnectorMetadata.listRelations(schema=test_schema1)")
.addCopies("ConnectorMetadata.getSystemTable(schema=test_schema1, table=test_table1)", 4)
.add("ConnectorMetadata.getMaterializedView(schema=test_schema1, table=test_table1)")
.add("ConnectorMetadata.getView(schema=test_schema1, table=test_table1)")
Expand Down Expand Up @@ -1388,10 +1388,10 @@ public void testGetColumnsMetadataCalls()
.collect(toImmutableList()),
ImmutableMultiset.<String>builder()
.addCopies("ConnectorMetadata.listSchemaNames", 5)
.add("ConnectorMetadata.listTables(schema=test_schema1)")
.add("ConnectorMetadata.listTables(schema=test_schema2)")
.add("ConnectorMetadata.listTables(schema=test_schema3_empty)")
.add("ConnectorMetadata.listTables(schema=test_schema4_empty)")
.add("ConnectorMetadata.listRelations(schema=test_schema1)")
.add("ConnectorMetadata.listRelations(schema=test_schema2)")
.add("ConnectorMetadata.listRelations(schema=test_schema3_empty)")
.add("ConnectorMetadata.listRelations(schema=test_schema4_empty)")
.addCopies("ConnectorMetadata.getSystemTable(schema=test_schema1, table=test_table1)", 20)
.addCopies("ConnectorMetadata.getMaterializedView(schema=test_schema1, table=test_table1)", 5)
.addCopies("ConnectorMetadata.getView(schema=test_schema1, table=test_table1)", 5)
Expand Down
2 changes: 1 addition & 1 deletion ...no-main/src/main/java/io/trino/connector/informationschema/InformationSchemaMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ private Set<QualifiedTablePrefix> calculatePrefixesWithTableName(
}

Set<QualifiedTablePrefix> tablePrefixes = prefixes.stream()
.flatMap(prefix -> metadata.listTables(session, prefix).stream())
.flatMap(prefix -> metadata.listRelations(session, prefix).stream())
.filter(objectName -> predicate.get().test(asFixedValues(objectName)))
.map(QualifiedObjectName::asQualifiedTablePrefix)
.distinct()
Expand Down
4 changes: 2 additions & 2 deletions ...-main/src/main/java/io/trino/connector/informationschema/InformationSchemaPageSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@
import static io.trino.connector.informationschema.InformationSchemaMetadata.isTablesEnumeratingTable;
import static io.trino.metadata.MetadataListing.getRelationTypes;
import static io.trino.metadata.MetadataListing.getViews;
import static io.trino.metadata.MetadataListing.listRelations;
import static io.trino.metadata.MetadataListing.listSchemas;
import static io.trino.metadata.MetadataListing.listTableColumns;
import static io.trino.metadata.MetadataListing.listTablePrivileges;
import static io.trino.metadata.MetadataListing.listTables;
import static io.trino.spi.security.PrincipalType.USER;
import static io.trino.spi.type.TypeUtils.writeNativeValue;
import static io.trino.type.TypeUtils.getDisplayLabel;
Expand Down Expand Up @@ -289,7 +289,7 @@ private void addTablesRecords(QualifiedTablePrefix prefix)
.collect(toImmutableSet());
}
else {
relations = listTables(session, metadata, accessControl, prefix);
relations = listRelations(session, metadata, accessControl, prefix);
views = Set.of();
}
// TODO (https://github.com/trinodb/trino/issues/8207) define a type for materialized views
Expand Down
6 changes: 3 additions & 3 deletions core/trino-main/src/main/java/io/trino/connector/system/jdbc/ColumnJdbcTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@
import static io.trino.connector.system.jdbc.FilterUtil.tablePrefix;
import static io.trino.connector.system.jdbc.FilterUtil.tryGetSingleVarcharValue;
import static io.trino.metadata.MetadataListing.listCatalogNames;
import static io.trino.metadata.MetadataListing.listRelations;
import static io.trino.metadata.MetadataListing.listSchemas;
import static io.trino.metadata.MetadataListing.listTableColumns;
import static io.trino.metadata.MetadataListing.listTables;
import static io.trino.metadata.MetadataUtil.TableMetadataBuilder.tableMetadataBuilder;
import static io.trino.spi.type.BigintType.BIGINT;
import static io.trino.spi.type.BooleanType.BOOLEAN;
Expand Down Expand Up @@ -211,7 +211,7 @@ TABLE_SCHEMA_COLUMN, toNullableValue(schemaName))))
QualifiedTablePrefix tablePrefix = tableFilter.isPresent()
? new QualifiedTablePrefix(schema.getCatalogName(), schema.getSchemaName(), tableFilter.get())
: new QualifiedTablePrefix(schema.getCatalogName(), schema.getSchemaName());
return listTables(session, metadata, accessControl, tablePrefix).stream()
return listRelations(session, metadata, accessControl, tablePrefix).stream()
.filter(schemaTableName -> predicate.test(ImmutableMap.of(
TABLE_CATALOG_COLUMN, toNullableValue(schema.getCatalogName()),
TABLE_SCHEMA_COLUMN, toNullableValue(schemaTableName.getSchemaName()),
Expand Down Expand Up @@ -278,7 +278,7 @@ public RecordCursor cursor(ConnectorTransactionHandle transactionHandle, Connect
QualifiedTablePrefix tablePrefix = tableFilter.isPresent()
? new QualifiedTablePrefix(catalog, schema, tableFilter.get())
: new QualifiedTablePrefix(catalog, schema);
Set<SchemaTableName> tables = listTables(session, metadata, accessControl, tablePrefix);
Set<SchemaTableName> tables = listRelations(session, metadata, accessControl, tablePrefix);
for (SchemaTableName schemaTableName : tables) {
String tableName = schemaTableName.getTableName();
if (!tableDomain.includesNullableValue(utf8Slice(tableName))) {
Expand Down
2 changes: 1 addition & 1 deletion core/trino-main/src/main/java/io/trino/execution/DropSchemaTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,6 @@ private static boolean isSchemaEmpty(Session session, CatalogSchemaName schema,
QualifiedTablePrefix tablePrefix = new QualifiedTablePrefix(schema.getCatalogName(), schema.getSchemaName());

// This is a best effort check that doesn't provide any guarantees against concurrent DDL operations
return metadata.listTables(session, tablePrefix).isEmpty();
return metadata.listRelations(session, tablePrefix).isEmpty();
}
}
2 changes: 1 addition & 1 deletion core/trino-main/src/main/java/io/trino/metadata/Metadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ Optional<TableExecuteHandle> getTableHandleForExecute(
* Get the relation names that match the specified table prefix (never null).
* This includes all relations (e.g. tables, views, materialized views).
*/
List<QualifiedObjectName> listTables(Session session, QualifiedTablePrefix prefix);
List<QualifiedObjectName> listRelations(Session session, QualifiedTablePrefix prefix);

/**
* Get the relation names that match the specified table prefix (never null).
Expand Down
8 changes: 4 additions & 4 deletions core/trino-main/src/main/java/io/trino/metadata/MetadataListing.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,19 +113,19 @@ private static SortedSet<String> doListSchemas(Session session, Metadata metadat
return ImmutableSortedSet.copyOf(accessControl.filterSchemas(session.toSecurityContext(), catalogName, schemaNames));
}

public static Set<SchemaTableName> listTables(Session session, Metadata metadata, AccessControl accessControl, QualifiedTablePrefix prefix)
public static Set<SchemaTableName> listRelations(Session session, Metadata metadata, AccessControl accessControl, QualifiedTablePrefix prefix)
Copy link
Copy Markdown
Member

@mosabua mosabua Nov 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At min you should add javadoc here .. but it also looks weird .. you are calling listRelations .. but getting back a set of SchemaTableName objects? Would you not have to rename SchemaTablenName to SchemaRelation as well then to be consistent?

Copy link
Copy Markdown
Member

@mosabua mosabua Nov 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other places we use QualifiedObjectName ..

{
try {
return doListTables(session, metadata, accessControl, prefix);
return doListRelations(session, metadata, accessControl, prefix);
}
catch (RuntimeException exception) {
throw handleListingException(exception, "tables", prefix.getCatalogName());
}
}

private static Set<SchemaTableName> doListTables(Session session, Metadata metadata, AccessControl accessControl, QualifiedTablePrefix prefix)
private static Set<SchemaTableName> doListRelations(Session session, Metadata metadata, AccessControl accessControl, QualifiedTablePrefix prefix)
{
Set<SchemaTableName> tableNames = metadata.listTables(session, prefix).stream()
Set<SchemaTableName> tableNames = metadata.listRelations(session, prefix).stream()
.map(QualifiedObjectName::asSchemaTableName)
.collect(toImmutableSet());

Expand Down
10 changes: 5 additions & 5 deletions core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ public ColumnMetadata getColumnMetadata(Session session, TableHandle tableHandle
}

@Override
public List<QualifiedObjectName> listTables(Session session, QualifiedTablePrefix prefix)
public List<QualifiedObjectName> listRelations(Session session, QualifiedTablePrefix prefix)
{
requireNonNull(prefix, "prefix is null");
if (cannotExist(prefix)) {
Expand All @@ -520,7 +520,7 @@ public List<QualifiedObjectName> listTables(Session session, QualifiedTablePrefi
}

Optional<CatalogMetadata> catalog = getOptionalCatalogMetadata(session, prefix.getCatalogName());
Set<QualifiedObjectName> tables = new LinkedHashSet<>();
Set<QualifiedObjectName> relations = new LinkedHashSet<>();
if (catalog.isPresent()) {
CatalogMetadata catalogMetadata = catalog.get();

Expand All @@ -530,13 +530,13 @@ public List<QualifiedObjectName> listTables(Session session, QualifiedTablePrefi
if (isExternalInformationSchema(catalogHandle, prefix.getSchemaName())) {
continue;
}
metadata.listTables(connectorSession, prefix.getSchemaName()).stream()
metadata.listRelations(connectorSession, prefix.getSchemaName()).stream()
.map(convertFromSchemaTableName(prefix.getCatalogName()))
.filter(table -> !isExternalInformationSchema(catalogHandle, table.getSchemaName()))
.forEach(tables::add);
.forEach(relations::add);
}
}
return ImmutableList.copyOf(tables);
return ImmutableList.copyOf(relations);
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,14 +810,14 @@ public TableHandle getTableHandle(String catalogName, String schemaName, String
}

@Override
public List<QualifiedObjectName> listTables(Session session, String catalog, String schema)
public List<QualifiedObjectName> listRelations(Session session, String catalog, String schema)
{
lock.readLock().lock();
try {
return transaction(transactionManager, plannerContext.getMetadata(), accessControl)
.readOnly()
.execute(session, transactionSession -> {
return getMetadata().listTables(transactionSession, new QualifiedTablePrefix(catalog, schema));
return getMetadata().listRelations(transactionSession, new QualifiedTablePrefix(catalog, schema));
});
}
finally {
Expand Down
2 changes: 1 addition & 1 deletion core/trino-main/src/main/java/io/trino/testing/QueryRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ default Plan createPlan(Session session, @Language("SQL") String sql)
throw new UnsupportedOperationException();
}

List<QualifiedObjectName> listTables(Session session, String catalog, String schema);
List<QualifiedObjectName> listRelations(Session session, String catalog, String schema);

boolean tableExists(Session session, String table);

Expand Down
2 changes: 1 addition & 1 deletion core/trino-main/src/main/java/io/trino/testing/TestingMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public Map<SchemaTableName, List<ColumnMetadata>> listTableColumns(ConnectorSess
requireNonNull(prefix, "prefix is null");

ImmutableMap.Builder<SchemaTableName, List<ColumnMetadata>> tableColumns = ImmutableMap.builder();
for (SchemaTableName tableName : listTables(session, prefix.getSchema())) {
for (SchemaTableName tableName : listRelations(session, prefix.getSchema())) {
ImmutableList.Builder<ColumnMetadata> columns = ImmutableList.builder();
for (ColumnMetadata column : tables.get(tableName).getColumns()) {
columns.add(new ColumnMetadata(column.getName(), column.getType()));
Expand Down
9 changes: 9 additions & 0 deletions core/trino-main/src/main/java/io/trino/tracing/TracingConnectorMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,15 @@ public Optional<Object> getInfo(ConnectorTableHandle table)
}
}

@Override
public List<SchemaTableName> listRelations(ConnectorSession session, Optional<String> schemaName)
{
Span span = startSpan("listRelations", schemaName);
try (var ignored = scopedSpan(span)) {
return delegate.listRelations(session, schemaName);
}
}

@Override
public List<SchemaTableName> listTables(ConnectorSession session, Optional<String> schemaName)
{
Expand Down
6 changes: 3 additions & 3 deletions core/trino-main/src/main/java/io/trino/tracing/TracingMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,11 @@ public TableStatistics getTableStatistics(Session session, TableHandle tableHand
}

@Override
public List<QualifiedObjectName> listTables(Session session, QualifiedTablePrefix prefix)
public List<QualifiedObjectName> listRelations(Session session, QualifiedTablePrefix prefix)
{
Span span = startSpan("listTables", prefix);
Span span = startSpan("listRelations", prefix);
try (var ignored = scopedSpan(span)) {
return delegate.listTables(session, prefix);
return delegate.listRelations(session, prefix);
}
}

Expand Down
2 changes: 1 addition & 1 deletion core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ public void dropSchema(Session session, CatalogSchemaName schema, boolean cascad
}

@Override
public List<QualifiedObjectName> listTables(Session session, QualifiedTablePrefix prefix)
public List<QualifiedObjectName> listRelations(Session session, QualifiedTablePrefix prefix)
{
List<QualifiedObjectName> tables = ImmutableList.<QualifiedObjectName>builder()
.addAll(this.tables.keySet().stream().map(table -> new QualifiedObjectName(catalogName, table.getSchemaName(), table.getTableName())).collect(toImmutableList()))
Expand Down
2 changes: 1 addition & 1 deletion core/trino-main/src/test/java/io/trino/metadata/AbstractMockMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public TableStatistics getTableStatistics(Session session, TableHandle tableHand
}

@Override
public List<QualifiedObjectName> listTables(Session session, QualifiedTablePrefix prefix)
public List<QualifiedObjectName> listRelations(Session session, QualifiedTablePrefix prefix)
{
throw new UnsupportedOperationException();
}
Expand Down
19 changes: 14 additions & 5 deletions core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,15 @@ default Optional<Object> getInfo(ConnectorTableHandle table)
* @see #listViews(ConnectorSession, Optional)
* @see #listMaterializedViews(ConnectorSession, Optional)
*/
default List<SchemaTableName> listRelations(ConnectorSession session, Optional<String> schemaName)
{
return listTables(session, schemaName);
}

/**
* @deprecated Use {@link #listRelations(ConnectorSession, Optional)} instead.
*/
@Deprecated(forRemoval = true)
default List<SchemaTableName> listTables(ConnectorSession session, Optional<String> schemaName)
{
return emptyList();
Expand Down Expand Up @@ -352,7 +361,7 @@ default Iterator<TableColumnsMetadata> streamTableColumns(ConnectorSession sessi

/**
* Gets columns for all relations (tables, views, materialized views), possibly filtered by schemaName.
* (e.g. for all relations that would be returned by {@link #listTables(ConnectorSession, Optional)}).
* (e.g. for all relations that would be returned by {@link #listRelations(ConnectorSession, Optional)}).
* Redirected table names are included, but the comment for them is not.
*/
@Experimental(eta = "2024-01-01")
Expand Down Expand Up @@ -391,7 +400,7 @@ default Iterator<RelationColumnsMetadata> streamRelationColumns(

/**
* Gets comments for all relations (tables, views, materialized views), possibly filtered by schemaName.
* (e.g. for all relations that would be returned by {@link #listTables(ConnectorSession, Optional)}).
* (e.g. for all relations that would be returned by {@link #listRelations(ConnectorSession, Optional)}).
* Redirected table names are included, but the comment for them is not.
*/
@Experimental(eta = "2024-01-01")
Expand All @@ -411,7 +420,7 @@ default Iterator<RelationCommentMetadata> streamRelationComments(ConnectorSessio
Set<SchemaTableName> mvAndViewNames = Stream.concat(mvNames.stream(), views.stream().map(RelationCommentMetadata::name))
.collect(toUnmodifiableSet());

List<RelationCommentMetadata> tables = listTables(session, schemaName).stream()
List<RelationCommentMetadata> tables = listRelations(session, schemaName).stream()
.filter(tableName -> !mvAndViewNames.contains(tableName))
.collect(collectingAndThen(toUnmodifiableSet(), relationFilter)).stream()
.map(tableName -> {
Expand Down Expand Up @@ -444,7 +453,7 @@ default Iterator<RelationCommentMetadata> streamRelationComments(ConnectorSessio
// getTableHandle or getTableMetadata failed call may fail if table disappeared during listing or is unsupported.
Helper.juliLogger.log(Level.WARNING, e, () -> "Failed to get metadata for table: " + tableName);
}
// Since the getTableHandle did not return null (i.e. succeeded or failed), we assume the table would be returned by listTables
// Since the getTableHandle did not return null (i.e. succeeded or failed), we assume the table would be returned by listRelations
return RelationCommentMetadata.forRelation(tableName, Optional.empty());
}
})
Expand Down Expand Up @@ -782,7 +791,7 @@ default ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, Co

/**
* Begin the atomic creation of a table with data.
*
* <p>
* <p/>
* If connector does not support execution with retries, the method should throw:
* <pre>
Expand Down
8 changes: 8 additions & 0 deletions ...lkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,14 @@ public Optional<Object> getInfo(ConnectorTableHandle table)
}
}

@Override
public List<SchemaTableName> listRelations(ConnectorSession session, Optional<String> schemaName)
{
try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) {
return delegate.listRelations(session, schemaName);
}
}

@Override
public List<SchemaTableName> listTables(ConnectorSession session, Optional<String> schemaName)
{
Expand Down
2 changes: 1 addition & 1 deletion plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ private void assertThatNoBlackHoleTableIsCreated()

private List<QualifiedObjectName> listBlackHoleTables()
{
return queryRunner.listTables(queryRunner.getDefaultSession(), "blackhole", "default");
return queryRunner.listRelations(queryRunner.getDefaultSession(), "blackhole", "default");
}

private void assertThatQueryReturnsValue(String sql, Object expected)
Expand Down
2 changes: 1 addition & 1 deletion .../main/java/io/trino/plugin/deltalake/metastore/HiveMetastoreBackedDeltaLakeMetastore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public List<String> getAllTables(String databaseName)
// it would be nice to filter out non-Delta tables; however, we can not call
// metastore.getTablesWithParameter(schema, TABLE_PROVIDER_PROP, TABLE_PROVIDER_VALUE), because that property
// contains a dot and must be compared case-insensitive
return delegate.getAllTables(databaseName);
return delegate.getRelations(databaseName);
}

@Override
Expand Down
Loading