-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Information schema fixups #2488
Conversation
log.debug( | ||
"Building information schema table (queryId=%s; catalog=%s; table=%s; prefixes=%s, limit=%s)", | ||
session.getQueryId(), | ||
informationSchemaTableHandle.getCatalogName(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we simply log informationSchemaTableHandle
?
@@ -13,6 +13,7 @@ | |||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmt msg:
Add logging...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was removed previously
@@ -111,6 +116,7 @@ public InformationSchemaPageSource( | |||
else { | |||
checkArgument(prefixes.equals(defaultPrefixes(catalogName)), "Catalog-wise tables have prefixes other than the default one"); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undo
@@ -101,6 +101,11 @@ public InformationSchemaPageSource( | |||
table = tableHandle.getTable(); | |||
prefixIterator = Suppliers.memoize(() -> { | |||
Set<QualifiedTablePrefix> prefixes = tableHandle.getPrefixes(); | |||
if (!tableHandle.getLimit().isPresent()) { | |||
// no limit is used, therefore it doesn't make sense to split information schema query into smaller ones |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Listing schemas explicitly here also helps return results to client sooner. And with respect to Hive Connector, it doesn't really increase the number of metadata calls to HMS. Having said that, I think it's fine to make the change as it may help in case of other connectors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And with respect to Hive Connector, it doesn't really increase the number of metadata calls to HMS. Having said that, I think it's fine to make the change as it may help in case of other connectors.
JDBC connectors can take advantage of single query. For example, they can query their own information_schema
, so instead of X requests you get one
When limit is not used, then listing schemas explicitly causes additional connector metadata queries. However, since all metadata rows need to be read, it doesn't make sense to do so.
3d6c905
to
3958333
Compare
No description provided.