wnob commented on code in PR #3421:
URL: https://github.com/apache/calcite/pull/3421#discussion_r1323688831
##########
core/src/main/java/org/apache/calcite/jdbc/CalciteConnectionImpl.java:
##########
@@ -190,6 +191,23 @@ void init() {
return super.unwrap(iface);
}
+ /** If a subclass is passed in for CalciteMetaTable or MetaColumn for
+ * {@code CalciteConnectionProperty.META_TABLE_CLASS} or
+ * {@code CalciteConnectionProperty.META_COLUMN_CLASS} when connecting,
calls to getTables()
+ * or getColumns() will create an enumerable using the subclass, otherwise it
+ * will default to creating CaliteMetaTable / MetaColumn. */
+ public Class<?> getMetaClass(CalciteConnectionProperty connectionProperty,
Review Comment:
See if you can use Java's type system to enforce inheritance here:
https://stackoverflow.com/questions/897935/when-do-java-generics-require-extends-t-instead-of-t-and-is-there-any-down
So the second parameter could be `Class<T> defaultClass` and the method
would return a `Class<? extends T>` or something like that. I also think the
javadoc is a bit awkwardly worded, and it should make use of the `{@link
CalciteMetaTable}` syntax which will hook into intellij's index for linking and
refactoring etc.
##########
core/src/main/java/org/apache/calcite/jdbc/CalciteMetaImpl.java:
##########
@@ -89,14 +93,35 @@
*/
public class CalciteMetaImpl extends MetaImpl {
static final Driver DRIVER = new Driver();
+ private final Class metaTableClass;
Review Comment:
`Class<? extends CalciteMetaTable>` here too, and similar for columns. That
will also obviate your `isAssignableFrom` assertions.
##########
core/src/main/java/org/apache/calcite/jdbc/CalciteMetaImpl.java:
##########
@@ -263,23 +288,30 @@ private static ImmutableMap.Builder<DatabaseProperty,
Object> addProperty(
typeFilter = v1 -> typeList.contains(v1.tableType);
}
final Predicate1<MetaSchema> schemaMatcher = namedMatcher(schemaPattern);
- return createResultSet(schemas(catalog)
- .where(schemaMatcher)
- .selectMany(schema -> tables(schema, matcher(tableNamePattern)))
- .where(typeFilter),
- MetaTable.class,
- "TABLE_CAT",
- "TABLE_SCHEM",
- "TABLE_NAME",
- "TABLE_TYPE",
- "REMARKS",
- "TYPE_CAT",
- "TYPE_SCHEM",
- "TYPE_NAME",
- "SELF_REFERENCING_COL_NAME",
- "REF_GENERATION");
+ Enumerable<MetaTable> tables = schemas(catalog)
+ .where(schemaMatcher)
+ .selectMany(schema -> tables(schema, matcher(tableNamePattern)))
+ .where(typeFilter);
+ String[] columnNames = getColumnNames(this.metaTableClass);
+ return createResultSet(tables,
+ this.metaTableClass,
+ columnNames);
+ }
+
+ /** The provided subclass needs to overload getColumnNames() with the
expected columns in the
+ * enumerable.
+ * */
+ private String[] getColumnNames(Class<?> clazz) {
+ try {
+ Method m = clazz.getMethod("getColumnNames");
Review Comment:
No need for dynamic method lookup with the type changes.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
