damondouglas commented on code in PR #30024:
URL: https://github.com/apache/beam/pull/30024#discussion_r1456157850
##########
sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/JdbcReadSchemaTransformProvider.java:
##########
@@ -68,8 +73,29 @@ public
JdbcReadSchemaTransform(JdbcReadSchemaTransformConfiguration config) {
}
protected JdbcIO.DataSourceConfiguration dataSourceConfiguration() {
+ String driverClassName = config.getDriverClassName();
+
+ if (Strings.isNullOrEmpty(driverClassName)) {
+ switch (Objects.requireNonNull(config.getJdbcType()).toLowerCase()) {
+ case "mysql":
+ driverClassName = "com.mysql.cj.jdbc.Driver";
+ break;
+ case "postgres":
+ driverClassName = "org.postgresql.Driver";
+ break;
+ case "oracle":
+ driverClassName = "oracle.jdbc.driver.OracleDriver";
+ break;
+ case "mssql":
+ driverClassName = "com.microsoft.sqlserver.jdbc.SQLServerDriver";
+ break;
Review Comment:
What if we combined this association of the driver name i.e. "mysql",
"postgres", etc with the JDBCTypes above as a Map<String, String> so that
future changes need not coordinate JDBCTypes above with the switch statements
here. Then perhaps in this method one could do
`jdbcTypesMap.contains(config.getJdbcType().toLowerCase())`.
##########
sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/JdbcReadSchemaTransformProvider.java:
##########
@@ -164,12 +194,17 @@ public abstract static class
JdbcReadSchemaTransformConfiguration implements Ser
public abstract String getDriverJars();
public void validate() throws IllegalArgumentException {
- if (Strings.isNullOrEmpty(getDriverClassName())) {
- throw new IllegalArgumentException("JDBC Driver class name cannot be
blank.");
+ if (Strings.isNullOrEmpty(getDriverClassName()) &&
Strings.isNullOrEmpty(getJdbcType())) {
Review Comment:
If `getDriverClassName()` is required, why is it declared `@Nullable`?
--
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]
