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.

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

Allow fetching relation names with their types (v2) #19863

Merged
merged 5 commits into from
Nov 24, 2023

Conversation

findepi
Copy link
Member

@findepi findepi commented Nov 22, 2023

This reduces number of Glue calls when reading from
information_schema.tables or system.jdbc.tables.

Like #19832 but avoids contentious rename (#19832 (comment)).

@findepi findepi requested a review from martint November 22, 2023 14:07
@cla-bot cla-bot bot added the cla-signed label Nov 22, 2023
@github-actions github-actions bot added jdbc Relates to Trino JDBC driver tests:hive hive Hive connector labels Nov 22, 2023
@findepi findepi force-pushed the findepi/relation-types-II branch from c1eb89d to 1007c1c Compare November 22, 2023 14:10
@findepi findepi force-pushed the findepi/relation-types-II branch from 1007c1c to 7239b11 Compare November 22, 2023 15:55
@findepi
Copy link
Member Author

findepi commented Nov 23, 2023

CI #19747

@findepi
Copy link
Member Author

findepi commented Nov 23, 2023

CI #16315

@findepi findepi force-pushed the findepi/relation-types-II branch from 7239b11 to 2a571f2 Compare November 23, 2023 12:14
@github-actions github-actions bot added the iceberg Iceberg connector label Nov 23, 2023
@findepi
Copy link
Member Author

findepi commented Nov 23, 2023

@losipiuk thank you for your review!

I noticed there was a would-be regression for Iceberg, so I added a preparatory commit (currently first) with a test case, that later shows a regression and later improvement. @losipiuk please take another look

Now this is an improvement for information_schema.tables and system.jdbc.tables for Iceberg with Glue as well.
This is however a regression for Iceberg and Hive with HMS (instead of 2 listing calls we do 3).
I think the Glue improvement is more important from performance sensitivity perspective, and I think we should go ahead with the change.

This reduces number of Glue calls when reading from
`information_schema.tables` or `system.jdbc.tables`.
This reduces number of Glue calls when reading from
`information_schema.tables` or `system.jdbc.tables`.
@findepi findepi force-pushed the findepi/relation-types-II branch from 2a571f2 to 3ebbbeb Compare November 24, 2023 08:51
@losipiuk
Copy link
Member

I think the Glue improvement is more important from performance sensitivity perspective, and I think we should go ahead with the change.

We chatted offline - and even though this is not an universal truth - it makes sense. The important point in the discussion was that HMS protocol is not suited for large deployments anyway; each call to HMS is bounded by request-timeout anyway so extra time is bounded.

There is no `HiveGlueMetastore`, but `GlueHiveMetastore` does exist.
@findepi findepi force-pushed the findepi/relation-types-II branch from 3ebbbeb to 131c790 Compare November 24, 2023 10:03
@findepi
Copy link
Member Author

findepi commented Nov 24, 2023

CI #19888

@findepi
Copy link
Member Author

findepi commented Nov 24, 2023

CI #19242

@findepi findepi merged commit d32454f into master Nov 24, 2023
98 of 100 checks passed
@findepi findepi deleted the findepi/relation-types-II branch November 24, 2023 15:50
@github-actions github-actions bot added this to the 434 milestone Nov 24, 2023
@mosabua
Copy link
Member

mosabua commented Nov 24, 2023

Do we need a release note for this .. something about performance improvement for Glue access in Iceberg, Delta Lake and Hive/

@mosabua
Copy link
Member

mosabua commented Nov 29, 2023

I am going to assume we dont need release notes entry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed hive Hive connector iceberg Iceberg connector jdbc Relates to Trino JDBC driver
Development

Successfully merging this pull request may close these issues.

3 participants