julianhyde commented on code in PR #3084:
URL: https://github.com/apache/calcite/pull/3084#discussion_r1117979248
##########
core/src/main/java/org/apache/calcite/jdbc/Driver.java:
##########
@@ -64,6 +64,15 @@ public Driver() {
this.prepareFactory = createPrepareFactory();
}
+ private Driver(Function0<CalcitePrepare> prepareFactory) {
+ super();
+ this.prepareFactory = prepareFactory;
+ }
+ public Driver withPrepareFactory(Function0<CalcitePrepare> prepareFactory) {
Review Comment:
add newline and javadoc
##########
core/src/main/java/org/apache/calcite/prepare/CalciteSqlValidator.java:
##########
@@ -22,10 +22,12 @@
import org.apache.calcite.sql.SqlOperatorTable;
import org.apache.calcite.sql.validate.SqlValidatorImpl;
-/** Validator. */
-class CalciteSqlValidator extends SqlValidatorImpl {
+/** Validator.
+ * Public access-level to allow overriding and custom validation logic.
+ * */
Review Comment:
Similar comments to previous javadoc. Second sentence should be a paragraph.
Also a sentence.
##########
core/src/main/java/org/apache/calcite/jdbc/Driver.java:
##########
@@ -64,6 +64,15 @@ public Driver() {
this.prepareFactory = createPrepareFactory();
}
+ private Driver(Function0<CalcitePrepare> prepareFactory) {
Review Comment:
unify constructors. all constructors except one should call other
constructors.
##########
core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java:
##########
@@ -514,14 +514,35 @@ <T> CalciteSignature<T> prepare_(
throw new AssertionError("factory returned null planner");
}
try {
+ CalcitePreparingStmt preparingStmt = getPreparingStmt(
+ context, elementType, catalogReader, planner);
return prepare2_(context, query, elementType, maxRowCount,
- catalogReader, planner);
+ catalogReader, preparingStmt);
} catch (RelOptPlanner.CannotPlanException e) {
exception = e;
}
}
throw exception;
}
+ protected CalcitePreparingStmt getPreparingStmt(
Review Comment:
add newline and javadoc
##########
core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java:
##########
@@ -937,8 +944,10 @@ public <R> R perform(CalciteServerStatement statement,
prepareContext.getRootSchema().plus(), statement);
}
- /** Holds state for the process of preparing a SQL statement. */
- static class CalcitePreparingStmt extends Prepare
+ /** Holds state for the process of preparing a SQL statement.
+ * Overload this class and {@link #createSqlValidator} to supply custom
validation logic.
+ * */
+ public static class CalcitePreparingStmt extends Prepare
Review Comment:
improve javadoc formatting. second sentence should be a new paragraph. use
`**/` rather than `* */`.
##########
core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java:
##########
@@ -590,21 +611,8 @@ <T> CalciteSignature<T> prepare2_(
Type elementType,
long maxRowCount,
CalciteCatalogReader catalogReader,
- RelOptPlanner planner) {
+ CalcitePreparingStmt preparingStmt) {
Review Comment:
Is CalcitePreparingStmt required? Or could parameter be a superclass?
--
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]
