Airblader commented on a change in pull request #17264:
URL: https://github.com/apache/flink/pull/17264#discussion_r712737082
##########
File path:
flink-connectors/flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/AbstractDialect.java
##########
@@ -18,80 +18,252 @@
package org.apache.flink.connector.jdbc.dialect;
-import org.apache.flink.table.api.TableSchema;
+import org.apache.flink.annotation.PublicEvolving;
import org.apache.flink.table.api.ValidationException;
import org.apache.flink.table.types.DataType;
import org.apache.flink.table.types.logical.DecimalType;
import org.apache.flink.table.types.logical.LogicalTypeRoot;
+import org.apache.flink.table.types.logical.RowType;
import org.apache.flink.table.types.logical.TimestampType;
import org.apache.flink.table.types.logical.VarBinaryType;
+import org.apache.flink.util.Preconditions;
-import java.util.List;
+import java.util.Arrays;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
-abstract class AbstractDialect implements JdbcDialect {
+import static java.lang.String.format;
+
+/**
+ * Base class for {@link JdbcDialect JdbcDialects} that implements basic data
type validation and
+ * the construction of basic {@code INSERT}, {@code UPDATE}, {@code DELETE},
and {@code SELECT}
+ * statements.
+ *
+ * <p>Implementors should be careful to check the default SQL statements are
performant for their
+ * specific dialect and override them if necessary.
+ */
+@PublicEvolving
+public abstract class AbstractDialect implements JdbcDialect {
Review comment:
I don't have a better suggestion of representing this idea if we insist
on keeping it in, but also e.g. `Range` feels out of place for a class like
this. I think we're missing a broader concept in the type system here to
express "type ranges", and we're working around it by introducing localized,
highly specific – but public – APIs for it. But before we start beating a dead
horse, it's also public-evolving, implementing a custom JDBC dialect will be a
fairly rare occurrence and if we ever have something better to switch this to,
the migration path would likely be trivial, so my concern with it is limited;
so let's consider this point closed.
--
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]
