Skip to content

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

Merged
findepi merged 5 commits intomasterfrom
findepi/relation-types-II
Nov 24, 2023
Merged

Allow fetching relation names with their types (v2)#19863
findepi merged 5 commits intomasterfrom
findepi/relation-types-II

Conversation

@findepi
Copy link
Copy Markdown
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
Copy Markdown
Member Author

findepi commented Nov 23, 2023

CI #19747

@findepi
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

findepi commented Nov 24, 2023

CI #19888

@findepi
Copy link
Copy Markdown
Member Author

findepi commented Nov 24, 2023

CI #19242

@findepi findepi merged commit d32454f into master Nov 24, 2023
@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
Copy Markdown
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
Copy Markdown
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