vvysotskyi commented on a change in pull request #2305:
URL: https://github.com/apache/calcite/pull/2305#discussion_r549816599
##########
File path: core/src/test/java/org/apache/calcite/test/TableInRootSchemaTest.java
##########
@@ -97,7 +102,8 @@
private Class[] columnTypes = { String.class, Integer.class };
private Object[][] rows = new Object[3][];
- SimpleTable() {
+ // CHECKSTYLE: IGNORE 1
Review comment:
We ignore the check for "Redundant 'public' modifier", but the
constructor should be public to be able to use it from the generated code. I've
just added a corresponding comment above this line.
##########
File path: core/src/test/java/org/apache/calcite/test/TableInRootSchemaTest.java
##########
@@ -179,5 +185,21 @@ public RelNode toRel(RelOptTable.ToRelContext context,
RelOptTable relOptTable) {
return EnumerableTableScan.create(context.getCluster(), relOptTable);
}
+
+ @Override public Expression getExpression(SchemaPlus schema, String
tableName, Class clazz) {
+ try {
+ MethodCallExpression queryableExpression =
+
Expressions.call(Expressions.new_(SimpleTable.class.getConstructor()),
+ QueryableTable.class.getMethod(
+ "asQueryable", QueryProvider.class, SchemaPlus.class,
String.class),
Review comment:
Thanks, used the constant from `BuiltInMethod`.
##########
File path: core/src/test/java/org/apache/calcite/test/JdbcTest.java
##########
@@ -461,6 +462,34 @@ private void addTableMacro(Connection connection, Method
method) throws SQLExcep
connection.close();
}
+ @Test void testQueryableTableWithTableMacro() throws SQLException {
+ try (Connection connection =
+ DriverManager.getConnection("jdbc:calcite:")) {
+
+ CalciteConnection calciteConnection =
+ connection.unwrap(CalciteConnection.class);
+ SchemaPlus rootSchema = calciteConnection.getRootSchema();
+ SchemaPlus schema = rootSchema.add("s", new AbstractSchema());
+ schema.add("simple", new TableMacro() {
Review comment:
Thanks, moved it there as a nested class.
##########
File path: core/src/test/java/org/apache/calcite/test/JdbcTest.java
##########
@@ -461,6 +462,34 @@ private void addTableMacro(Connection connection, Method
method) throws SQLExcep
connection.close();
}
+ @Test void testQueryableTableWithTableMacro() throws SQLException {
Review comment:
Thanks, moved it to the `TableFunctionTest` class.
##########
File path: core/src/main/java/org/apache/calcite/schema/Schemas.java
##########
@@ -183,6 +183,33 @@ public static Expression tableExpression(SchemaPlus
schema, Type elementType,
return EnumUtils.convert(expression, clazz);
}
+ /**
+ * Generates an expression with which table can be referenced in
+ * generated code.
+ *
+ * @param schema Schema
+ * @param tableName Table name (unique within schema)
+ * @param table table to be referenced
+ * @param clazz The desired collection class; for example {@code
Queryable}.
Review comment:
Thanks for pointing to this, updated the comment to clarify that class
may or may not be the class of the table, and added a more specific example.
##########
File path: core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java
##########
@@ -118,9 +115,9 @@ public static RelOptTableImpl create(
RelDataType rowType,
List<String> names,
Table table,
- Expression expression) {
+ Function<Class, Expression> expressionFunction) {
return new RelOptTableImpl(schema, rowType, names, table,
- c -> expression, table.getStatistic().getRowCount());
+ expressionFunction, table.getStatistic().getRowCount());
Review comment:
Agree. I've introduced the `TableExpressionFactory` interface and use it
instead of the previous `expressionFunction`.
##########
File path: core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java
##########
@@ -163,22 +160,7 @@ public RelOptTableImpl copy(RelDataType newRowType) {
private static Function<Class, Expression> getClassExpressionFunction(
Review comment:
Thanks, inlined the code and removed it.
##########
File path: core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java
##########
@@ -118,9 +115,9 @@ public static RelOptTableImpl create(
RelDataType rowType,
List<String> names,
Table table,
- Expression expression) {
+ Function<Class, Expression> expressionFunction) {
Review comment:
Added the method with the previous signature, marked it as deprecated
and added JavaDoc for a new 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.
For queries about this service, please contact Infrastructure at:
[email protected]
