Skip to content
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

Jump to bottom

Filter information_schema within engine #16080

Merged
merged 1 commit into from
Mar 8, 2023
Merged

Filter information_schema within engine #16080

merged 1 commit into from
Mar 8, 2023

Conversation

ebyhr
Copy link
Member

@ebyhr ebyhr commented Feb 13, 2023

Description

Fixes #1559

Additional context and related issues

Release notes

(x) This is not user-visible or docs only and no release notes are required.

@cla-bot cla-bot bot added the cla-signed label Feb 13, 2023
@ebyhr ebyhr marked this pull request as draft February 13, 2023 04:06
@ebyhr ebyhr force-pushed the ebi/core-filter-information-schema branch 2 times, most recently from 9e5f500 to 26da64e Compare February 13, 2023 05:53
@github-actions github-actions bot added the jdbc Relates to Trino JDBC driver label Feb 13, 2023
@ebyhr ebyhr force-pushed the ebi/core-filter-information-schema branch from 26da64e to 7df9deb Compare February 13, 2023 07:11
@ebyhr ebyhr marked this pull request as ready for review February 13, 2023 09:45
@ebyhr
Copy link
Member Author

ebyhr commented Feb 13, 2023

CI hit #13166 & #13199

@findinpath findinpath self-requested a review February 13, 2023 10:06
@ebyhr ebyhr requested review from martint and electrum February 13, 2023 10:06
@ebyhr ebyhr force-pushed the ebi/core-filter-information-schema branch from 7df9deb to 1c9ec05 Compare February 13, 2023 12:37
@ebyhr ebyhr force-pushed the ebi/core-filter-information-schema branch 2 times, most recently from 4784c9a to 6db2f57 Compare February 20, 2023 00:08
@ebyhr
Copy link
Member Author

ebyhr commented Feb 20, 2023

web-ui-checks hit #16179

@ebyhr ebyhr self-assigned this Feb 20, 2023
@ebyhr ebyhr force-pushed the ebi/core-filter-information-schema branch 2 times, most recently from 0b398f9 to b8c7c1c Compare February 27, 2023 07:05
@ebyhr ebyhr force-pushed the ebi/core-filter-information-schema branch from b8c7c1c to 84887e4 Compare March 7, 2023 07:52
@github-actions github-actions bot added bigquery BigQuery connector delta-lake Delta Lake connector hive Hive connector iceberg Iceberg connector labels Mar 7, 2023
@ebyhr ebyhr requested a review from findepi March 7, 2023 08:24
findepi
findepi approved these changes Mar 7, 2023
View reviewed changes
core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java Outdated
@@ -1431,6 +1466,16 @@ public void setMaterializedViewProperties(Session session, QualifiedObjectName v
metadata.setMaterializedViewProperties(session.toConnectorSession(catalogHandle), viewName.asSchemaTableName(), properties);
}

private static boolean isExternalInformationSchema(CatalogHandle catalogHandle, Optional<String> schemaName)
{
return isExternalInformationSchema(catalogHandle, schemaName.orElse(null));
Copy link
Member

Choose a reason for hiding this comment

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

return schemaName.isPresent() && ...

so that the other method doesn't have to have @Nullable

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java Outdated
String schemaName = schema.toLowerCase(ENGLISH);
return !(schemaName.equals("information_schema") || schemaName.equals("sys"));
})
.filter(schema -> !schema.toLowerCase(ENGLISH).equals("sys"))
Copy link
Member

Choose a reason for hiding this comment

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

equalsIgnoreCase?

plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveUtil.java
@@ -1098,11 +1098,6 @@ public static String sortingColumnToString(SortingColumn column)

public static boolean isHiveSystemSchema(String schemaName)
{
if ("information_schema".equals(schemaName)) {
Copy link
Member

Choose a reason for hiding this comment

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

isHiveSystemSchema("information_schema") should still return true, because "information_schema" is indeed a Hive system schema.

we should reword the comment though

testing/trino-tests/src/test/java/io/trino/tests/TestInformationSchemaConnector.java
@@ -153,7 +153,7 @@ public void testMetadataCalls()
"VALUES 2",
new MetadataCallsCount()
.withListSchemasCount(1)
.withListTablesCount(3));
.withListTablesCount(2));
Copy link
Member

Choose a reason for hiding this comment

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

nice

@ebyhr ebyhr force-pushed the ebi/core-filter-information-schema branch from 84887e4 to 88e66a2 Compare March 7, 2023 23:43
@ebyhr ebyhr force-pushed the ebi/core-filter-information-schema branch from 88e66a2 to ff8876b Compare March 8, 2023 05:10
@ebyhr
Copy link
Member Author

ebyhr commented Mar 8, 2023

CI hit #16406

@ebyhr ebyhr merged commit b360596 into master Mar 8, 2023
@ebyhr ebyhr deleted the ebi/core-filter-information-schema branch March 8, 2023 07:01
@github-actions github-actions bot added this to the 410 milestone Mar 8, 2023
@colebow colebow mentioned this pull request Mar 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@findinpath findinpath findinpath left review comments

@chenjian2664 chenjian2664 chenjian2664 left review comments

@findepi findepi findepi approved these changes

@martint martint Awaiting requested review from martint

@electrum electrum Awaiting requested review from electrum

Assignees

@ebyhr ebyhr

Labels
bigquery BigQuery connector cla-signed delta-lake Delta Lake connector hive Hive connector iceberg Iceberg connector jdbc Relates to Trino JDBC driver
Milestone
410
Development

Successfully merging this pull request may close these issues.

information_schema should be handled by the engine fully
4 participants
@ebyhr @findepi @chenjian2664 @findinpath