You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/11/29 18:22:06 UTC
[GitHub] [drill] dzamo opened a new pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
dzamo opened a new pull request #2388:
URL: https://github.com/apache/drill/pull/2388
# [DRILL-8057](https://issues.apache.org/jira/browse/DRILL-8057): INFORMATION_SCHEMA filter push down is inefficient
## Description
WHERE clauses in queries against INFORMATION_SCHEMA do not stop Drill from fetching a schema hierarchy from all enabled storage configs. This results in abysmal performance when unresponsive data sources are enabled, as reported by users in the Apache Drill Slack channels. This PR teaches info schema to prune irrelevant schema subtrees from the search tree.
## Documentation
No user-visible change
## Testing
Existing info schema unit tests + one addition for IN operator
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on a change in pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2388:
URL: https://github.com/apache/drill/pull/2388#discussion_r780266603
##########
File path: exec/java-exec/src/main/java/org/apache/calcite/jdbc/DynamicRootSchema.java
##########
@@ -154,10 +157,25 @@ private void loadSchemaFactory(String schemaName, boolean caseSensitive) {
}
}
- static class RootSchema extends AbstractSchema {
+ public static class RootSchema extends AbstractSchema {
- public RootSchema() {
+ private StoragePluginRegistry storages;
+
+ private DynamicRootSchema dynRootSchema;
+
+ public RootSchema(StoragePluginRegistry storages) {
super(Collections.emptyList(), ROOT_SCHEMA_NAME);
+ this.storages = storages;
+ }
+
+ @Override
+ public Set<String> getSubSchemaNames() {
+ return storages.availablePlugins();
+ }
+
+ @Override
+ public Schema getSubSchema(String name) {
Review comment:
Could you please explain why we should override this method and delegate calls to `DynamicRootSchema`?
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilter.java
##########
@@ -226,6 +281,10 @@ private Result evaluateHelperFunction(Map<String, String> recordValues, Function
}
case "in": {
+ // I don't believe that this case will ever run. The IN operator is compiled either
Review comment:
I think it is possible when a user submits plan instead of query.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] dzamo commented on pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
dzamo commented on pull request #2388:
URL: https://github.com/apache/drill/pull/2388#issuecomment-982678036
@cgivre let's hold off I think. While the change here would reasonably be mergeable (once checkstyle was satisfied), it does not bring any meaningful benefit if the earlier plugin scanning stays the way it is. I'm still hopeful that something can be done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on a change in pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
jnturton commented on a change in pull request #2388:
URL: https://github.com/apache/drill/pull/2388#discussion_r783077559
##########
File path: exec/java-exec/src/main/java/org/apache/calcite/jdbc/DynamicRootSchema.java
##########
@@ -154,10 +157,25 @@ private void loadSchemaFactory(String schemaName, boolean caseSensitive) {
}
}
- static class RootSchema extends AbstractSchema {
+ public static class RootSchema extends AbstractSchema {
- public RootSchema() {
+ private StoragePluginRegistry storages;
+
+ private DynamicRootSchema dynRootSchema;
+
+ public RootSchema(StoragePluginRegistry storages) {
super(Collections.emptyList(), ROOT_SCHEMA_NAME);
+ this.storages = storages;
+ }
+
+ @Override
+ public Set<String> getSubSchemaNames() {
+ return storages.availablePlugins();
+ }
+
+ @Override
+ public Schema getSubSchema(String name) {
Review comment:
@vvysotskyi good catch, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on a change in pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2388:
URL: https://github.com/apache/drill/pull/2388#discussion_r758726205
##########
File path: exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestInfoSchema.java
##########
@@ -64,13 +64,13 @@ public static void setupFiles() {
@Test
public void selectFromAllTables() throws Exception{
- test("select * from INFORMATION_SCHEMA.SCHEMATA");
- test("select * from INFORMATION_SCHEMA.CATALOGS");
- test("select * from INFORMATION_SCHEMA.VIEWS");
- test("select * from INFORMATION_SCHEMA.`TABLES`");
- test("select * from INFORMATION_SCHEMA.COLUMNS");
- test("select * from INFORMATION_SCHEMA.`FILES`");
- test("select * from INFORMATION_SCHEMA.`PARTITIONS`");
+// test("select * from INFORMATION_SCHEMA.SCHEMATA");
+// test("select * from INFORMATION_SCHEMA.CATALOGS");
+// test("select * from INFORMATION_SCHEMA.VIEWS");
+ test("select * from INFORMATION_SCHEMA.`TABLES` where table_schema = 'cp.default'");
+// test("select * from INFORMATION_SCHEMA.COLUMNS");
+// test("select * from INFORMATION_SCHEMA.`FILES`");
+// test("select * from INFORMATION_SCHEMA.`PARTITIONS`");
Review comment:
Please add a new test instead of changing the existing one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2388:
URL: https://github.com/apache/drill/pull/2388#issuecomment-982670650
@dzamo Thanks for submitting this. Should we merge this as is, or do you think it is worth tackling some of the hackery in the StoragePluginRegistryImpl?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton merged pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
jnturton merged pull request #2388:
URL: https://github.com/apache/drill/pull/2388
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] lgtm-com[bot] commented on pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #2388:
URL: https://github.com/apache/drill/pull/2388#issuecomment-1013197378
This pull request **introduces 1 alert** when merging 8579a1ef1d27786907ac7ac141670986245552bd into bd0458e027bc90c1332fa1d5376d9616b720213a - [view on LGTM.com](https://lgtm.com/projects/g/apache/drill/rev/pr-e5a608d80b17cb6b79e854ec9b67040e7ed56862)
**new alerts:**
* 1 for Spurious Javadoc @param tags
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
jnturton commented on pull request #2388:
URL: https://github.com/apache/drill/pull/2388#issuecomment-1013124268
I've added @paul-rogers to the reviewers since he's familiar with the storage plugin registry and might spot gotchas that were missed in this less-than-obvious change set.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on a change in pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
jnturton commented on a change in pull request #2388:
URL: https://github.com/apache/drill/pull/2388#discussion_r784968419
##########
File path: exec/java-exec/src/main/java/org/apache/calcite/jdbc/DynamicRootSchema.java
##########
@@ -150,6 +150,11 @@ public String getTypeName() {
DataContext.ROOT,
BuiltInMethod.DATA_CONTEXT_GET_ROOT_SCHEMA.method);
}
+
+ @Override
+ public boolean showInInformationSchema() {
+ return false;
+ }
}
Review comment:
@cgivre here's the description on this method in AbstractSchema. I believe it was added when the information_schema db was originally developed.
```
/**
* Reports whether to show items from this schema in INFORMATION_SCHEMA
* tables.
* (Controls ... TODO: Doc.: Mention what this typically controls or
* affects.)
* <p>
* This base implementation returns {@code true}.
* </p>
*/
public boolean showInInformationSchema() {
return true;
}
```
The override in this PR says "don't show the root schema in query results against information_schema".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on a change in pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2388:
URL: https://github.com/apache/drill/pull/2388#discussion_r781256339
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/store/SchemaTreeProvider.java
##########
@@ -154,17 +156,23 @@ public SchemaPlus createFullRootSchema(SchemaConfig schemaConfig) {
.build(logger);
}
}
+ */
@Override
public void close() throws Exception {
+ //TODO jnturton: clean up
+ /*
List<AutoCloseable> toClose = Lists.newArrayList();
for(SchemaPlus tree : schemaTreesToClose) {
addSchemasToCloseList(tree, toClose);
}
AutoCloseables.close(toClose);
+ */
}
+ //TODO jnturton: clean up
+ /*
private static void addSchemasToCloseList(final SchemaPlus tree, final List<AutoCloseable> toClose) {
for(String subSchemaName : tree.getSubSchemaNames()) {
addSchemasToCloseList(tree.getSubSchema(subSchemaName), toClose);
Review comment:
Yes, initially I had doubts about this change, but for now, I propose to delete this method. For the case when someone would need to add some special logic under it, this change might be reconsidered. But we should be sure that changes in this PR are covered by tests, so breaking this optimization would be easily detected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on a change in pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2388:
URL: https://github.com/apache/drill/pull/2388#discussion_r781253084
##########
File path: exec/java-exec/src/main/java/org/apache/calcite/jdbc/DynamicRootSchema.java
##########
@@ -154,10 +157,25 @@ private void loadSchemaFactory(String schemaName, boolean caseSensitive) {
}
}
- static class RootSchema extends AbstractSchema {
+ public static class RootSchema extends AbstractSchema {
- public RootSchema() {
+ private StoragePluginRegistry storages;
+
+ private DynamicRootSchema dynRootSchema;
+
+ public RootSchema(StoragePluginRegistry storages) {
super(Collections.emptyList(), ROOT_SCHEMA_NAME);
+ this.storages = storages;
+ }
+
+ @Override
+ public Set<String> getSubSchemaNames() {
+ return storages.availablePlugins();
+ }
+
+ @Override
+ public Schema getSubSchema(String name) {
Review comment:
I think the case of `DynamicRootSchema` is the special one, since it overrides the parent method `getImplicitSubSchema()` that uses the `Schema.getSubSchema()` method in the parent class. I think it was done intentionally, in the opposite case all the logic related to loading schemas simple would be placed in the `RootSchema`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on a change in pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
jnturton commented on a change in pull request #2388:
URL: https://github.com/apache/drill/pull/2388#discussion_r784968419
##########
File path: exec/java-exec/src/main/java/org/apache/calcite/jdbc/DynamicRootSchema.java
##########
@@ -150,6 +150,11 @@ public String getTypeName() {
DataContext.ROOT,
BuiltInMethod.DATA_CONTEXT_GET_ROOT_SCHEMA.method);
}
+
+ @Override
+ public boolean showInInformationSchema() {
+ return false;
+ }
}
Review comment:
@cgivre here's the description on this method in AbstractSchema. I believe it was added when the information_schema db was originally developed.
```
/**
* Reports whether to show items from this schema in INFORMATION_SCHEMA
* tables.
* (Controls ... TODO: Doc.: Mention what this typically controls or
* affects.)
* <p>
* This base implementation returns {@code true}.
* </p>
*/
public boolean showInInformationSchema() {
return true;
}
```
The override in this PR says "don't show the root schema in query results from information_schema".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] lgtm-com[bot] commented on pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #2388:
URL: https://github.com/apache/drill/pull/2388#issuecomment-1013328835
This pull request **introduces 1 alert** when merging d029bf133deba496772dffa4e4c5680e2d3bc1b6 into bd0458e027bc90c1332fa1d5376d9616b720213a - [view on LGTM.com](https://lgtm.com/projects/g/apache/drill/rev/pr-70a5b8fc109f90fe37c7f12ae6ff506dfc15a5d2)
**new alerts:**
* 1 for Spurious Javadoc @param tags
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on a change in pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2388:
URL: https://github.com/apache/drill/pull/2388#discussion_r780266603
##########
File path: exec/java-exec/src/main/java/org/apache/calcite/jdbc/DynamicRootSchema.java
##########
@@ -154,10 +157,25 @@ private void loadSchemaFactory(String schemaName, boolean caseSensitive) {
}
}
- static class RootSchema extends AbstractSchema {
+ public static class RootSchema extends AbstractSchema {
- public RootSchema() {
+ private StoragePluginRegistry storages;
+
+ private DynamicRootSchema dynRootSchema;
+
+ public RootSchema(StoragePluginRegistry storages) {
super(Collections.emptyList(), ROOT_SCHEMA_NAME);
+ this.storages = storages;
+ }
+
+ @Override
+ public Set<String> getSubSchemaNames() {
+ return storages.availablePlugins();
+ }
+
+ @Override
+ public Schema getSubSchema(String name) {
Review comment:
Could you please explain why we should override this method and delegate calls to `DynamicRootSchema`?
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilter.java
##########
@@ -226,6 +281,10 @@ private Result evaluateHelperFunction(Map<String, String> recordValues, Function
}
case "in": {
+ // I don't believe that this case will ever run. The IN operator is compiled either
Review comment:
I think it is possible when a user submits plan instead of query.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] dzamo commented on a change in pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
dzamo commented on a change in pull request #2388:
URL: https://github.com/apache/drill/pull/2388#discussion_r758981355
##########
File path: exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestInfoSchema.java
##########
@@ -64,13 +64,13 @@ public static void setupFiles() {
@Test
public void selectFromAllTables() throws Exception{
- test("select * from INFORMATION_SCHEMA.SCHEMATA");
- test("select * from INFORMATION_SCHEMA.CATALOGS");
- test("select * from INFORMATION_SCHEMA.VIEWS");
- test("select * from INFORMATION_SCHEMA.`TABLES`");
- test("select * from INFORMATION_SCHEMA.COLUMNS");
- test("select * from INFORMATION_SCHEMA.`FILES`");
- test("select * from INFORMATION_SCHEMA.`PARTITIONS`");
+// test("select * from INFORMATION_SCHEMA.SCHEMATA");
+// test("select * from INFORMATION_SCHEMA.CATALOGS");
+// test("select * from INFORMATION_SCHEMA.VIEWS");
+ test("select * from INFORMATION_SCHEMA.`TABLES` where table_schema = 'cp.default'");
+// test("select * from INFORMATION_SCHEMA.COLUMNS");
+// test("select * from INFORMATION_SCHEMA.`FILES`");
+// test("select * from INFORMATION_SCHEMA.`PARTITIONS`");
Review comment:
@vvysotskyi thanks, this was definitely not supposed to be included in what I pushed. Reverted this test, another new one is already present.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] lgtm-com[bot] commented on pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #2388:
URL: https://github.com/apache/drill/pull/2388#issuecomment-1004908325
This pull request **introduces 5 alerts** when merging ce046faa50fa78857640ca7546336e733cf9d965 into fa2cb0f4937c0d8e797a675d8d6c13c316e48d4c - [view on LGTM.com](https://lgtm.com/projects/g/apache/drill/rev/pr-de882f8198d8945d6a7c877a69fe541f9266856f)
**new alerts:**
* 5 for Spurious Javadoc @param tags
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
jnturton commented on pull request #2388:
URL: https://github.com/apache/drill/pull/2388#issuecomment-1005646342
@vvysotskyi one specific question that arises in this PR is that of closing schema objects. Before this PR, `SchemaTreeProvider#close()` would recursively access the entire schema tree in order to call close on each schema object. This resulted in a lookup of every storage plugin and then a call to `registerSchemas()` on it, even if the plugin did not participate in the query being run. To avoid this I've made `SchemaTreeProvider#close()` a no-op but I'm not sure what the consequences of this might be. IIRC the one implementation of `SchemaPlus#close()` I did check in Calcite was itself a no-op, making this change harmless. But there could be other implementations of `SchemaPlus#close()`...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on a change in pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
jnturton commented on a change in pull request #2388:
URL: https://github.com/apache/drill/pull/2388#discussion_r781226257
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/store/SchemaTreeProvider.java
##########
@@ -154,17 +156,23 @@ public SchemaPlus createFullRootSchema(SchemaConfig schemaConfig) {
.build(logger);
}
}
+ */
@Override
public void close() throws Exception {
+ //TODO jnturton: clean up
+ /*
List<AutoCloseable> toClose = Lists.newArrayList();
for(SchemaPlus tree : schemaTreesToClose) {
addSchemasToCloseList(tree, toClose);
}
AutoCloseables.close(toClose);
+ */
}
+ //TODO jnturton: clean up
+ /*
private static void addSchemasToCloseList(final SchemaPlus tree, final List<AutoCloseable> toClose) {
for(String subSchemaName : tree.getSubSchemaNames()) {
addSchemasToCloseList(tree.getSubSchema(subSchemaName), toClose);
Review comment:
@vvysotskyi under this PR, the construction of the schema tree becomes lazy since createFullRootSchema is avoided. The code at this line unfortunately undoes the benefits by eagerly constructing the entire schema tree only in order to be able to close schema objects. I am not happy about skipping calls to `close()` on schemas, do you think I should look to add code to DynamicRootSchemas that returns only schemas that have already been constructed and initialised so that the code here has a suitable collection to close? E.g. the `getSubSchemaNames()`, which returns every storage config name, gets replaced with something like `getInitialisedSubSchemas()`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] jnturton commented on a change in pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
jnturton commented on a change in pull request #2388:
URL: https://github.com/apache/drill/pull/2388#discussion_r781220453
##########
File path: exec/java-exec/src/main/java/org/apache/calcite/jdbc/DynamicRootSchema.java
##########
@@ -154,10 +157,25 @@ private void loadSchemaFactory(String schemaName, boolean caseSensitive) {
}
}
- static class RootSchema extends AbstractSchema {
+ public static class RootSchema extends AbstractSchema {
- public RootSchema() {
+ private StoragePluginRegistry storages;
+
+ private DynamicRootSchema dynRootSchema;
+
+ public RootSchema(StoragePluginRegistry storages) {
super(Collections.emptyList(), ROOT_SCHEMA_NAME);
+ this.storages = storages;
+ }
+
+ @Override
+ public Set<String> getSubSchemaNames() {
+ return storages.availablePlugins();
+ }
+
+ @Override
+ public Schema getSubSchema(String name) {
Review comment:
@vvysotskyi I aimed here to work according to the existing design as I saw it. DynamicRootSchema is a subtype of CalciteSchema which is a wrapper around a Schema stored in a member variable. In the case of CalciteSchema it is the wrapped Schema that implements getSubSchema and getSubSchemaNames, not the CalciteSchema wrapper. So I reasoned that for the case of DynamicRootSchema, the wrapped RootSchema should implement, indeed override, the mentioned methods.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on a change in pull request #2388: DRILL-8057: INFORMATION_SCHEMA filter push down is inefficient
Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2388:
URL: https://github.com/apache/drill/pull/2388#discussion_r758710483
##########
File path: exec/java-exec/src/main/java/org/apache/calcite/jdbc/DynamicRootSchema.java
##########
@@ -150,6 +150,11 @@ public String getTypeName() {
DataContext.ROOT,
BuiltInMethod.DATA_CONTEXT_GET_ROOT_SCHEMA.method);
}
+
+ @Override
+ public boolean showInInformationSchema() {
+ return false;
+ }
}
Review comment:
What is the purpose of this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org