hito4t
commented
5 years ago @hiroyuki-sato
Thank you for the information!
It seems that com.mysql.jdbc.TimeUtil was moved to com.mysql.cj.util.TimeUtil .
Open hiroyuki-sato opened 5 years ago
hito4t
commented
5 years ago @hiroyuki-sato
Thank you for the information!
It seems that com.mysql.jdbc.TimeUtil was moved to com.mysql.cj.util.TimeUtil .
hiroyuki-sato
commented
5 years ago Hello, @hito4t
Thank you for your comment. I just wanted to share these issues. I don't have a strong motivation to support Connector/J v8 yet.
I got it. It seems we need to switch com.mysql.jdbc.TimeUtil and com.mysql.cj.util.TimeUtil versions.
t-yuki
commented
3 years ago It seems to be MySQL 8.x Server requires Connector/J v8 because current driver doesn't support caching_sha2_password auth option.
When I try to connect an MySQL 8.x Server, Error: java.lang.RuntimeException: java.sql.SQLException: Unable to load authentication plugin 'caching_sha2_password'. occurs.
hiroyuki-sato
commented
3 years ago Hello, @t-yuki Thank you for your report.
mysql_native_password instead of caching_sha2_password../gradlew gem It will create embulk-input-mysql-0.11.0-java.gem in the ./embulk-input-mysql/build/gems directory.diff --git a/embulk-input-mysql/build.gradle b/embulk-input-mysql/build.gradle
index 93a93b5..e4603ef 100644
--- a/embulk-input-mysql/build.gradle
+++ b/embulk-input-mysql/build.gradle
@@ -1,11 +1,11 @@
dependencies {
compile(project(path: ":embulk-input-jdbc", configuration: "runtimeElements"))
- compileOnly "mysql:mysql-connector-java:5.1.44"
- defaultJdbcDriver 'mysql:mysql-connector-java:5.1.44'
+ compileOnly "mysql:mysql-connector-java:8.0.23"
+ defaultJdbcDriver 'mysql:mysql-connector-java:8.0.23'
testCompile 'org.embulk:embulk-standards:0.9.23'
- testCompile "mysql:mysql-connector-java:5.1.44"
+ testCompile "mysql:mysql-connector-java:8.0.23"
}
embulkPlugin {
diff --git a/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java b/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java
index 04f1bca..0d14d50 100644
--- a/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java
+++ b/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java
@@ -158,7 +158,7 @@ public class MySQLInputPlugin
// Here implements a workaround as as workaround.
Field f = null;
try {
- Class<?> timeUtilClass = Class.forName("com.mysql.jdbc.TimeUtil");
+ Class<?> timeUtilClass = Class.forName("com.mysql.cj.util.TimeUtil");
f = timeUtilClass.getDeclaredField("timeZoneMappings");
f.setAccessible(true);
@@ -166,7 +166,7 @@ public class MySQLInputPlugin
if (timeZoneMappings == null) {
timeZoneMappings = new Properties();
synchronized (timeUtilClass) {
- timeZoneMappings.load(this.getClass().getResourceAsStream("/com/mysql/jdbc/TimeZoneMapping.properties"));
+ timeZoneMappings.load(this.getClass().getResourceAsStream("/com/mysql/cj/util/TimeZoneMapping.properties"));
}
f.set(null, timeZoneMappings);
}@hiroyuki-sato I'm looking forward to your PR!
It needs to support the MySQL 5.x and 8.x.
We can detect the driver version by the following code.
Driver driver = DriverManager.getDriver(url);
int majorVersion = driver.getMajorVersion();
hiroyuki-sato
commented
3 years ago @hito4t Thank you for your comment. I created #200. @dmikurube advised me about this implementation. twitter. So I don't use version checking. Do you have any concerns? Let's discuss this on #200
hito4t
commented
3 years ago @dmikurube @hiroyuki-sato We can get the major version number as integer (for example, 5 or 8). I think it is simpler than try-catch. What do you think?
hiroyuki-sato
commented
3 years ago @hito4t Thank you for your comment.
The proposed way is simpler than the current way.
@dmikurube What do you think?
diff --git a/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java b/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java
index 04f1bca..b333f91 100644
--- a/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java
+++ b/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java
@@ -2,6 +2,7 @@ package org.embulk.input;
import java.io.IOException;
import java.lang.reflect.Field;
+import java.sql.Driver;
import java.util.Properties;
import java.sql.Connection;
import java.sql.DriverManager;
@@ -68,6 +69,7 @@ public class MySQLInputPlugin
@Override
protected MySQLInputConnection newConnection(PluginTask task) throws SQLException
{
+ Driver driver;
MySQLPluginTask t = (MySQLPluginTask) task;
loadDriver("com.mysql.jdbc.Driver", t.getDriverPath());
@@ -123,8 +125,9 @@ public class MySQLInputPlugin
props.putAll(t.getOptions());
logConnectionProperties(url, props);
+ driver = DriverManager.getDriver(url);
// load timezone mappings
- loadTimeZoneMappings();
+ loadTimeZoneMappings(driver.getMajorVersion());
Connection con = DriverManager.getConnection(url, props);
try {
@@ -144,8 +147,18 @@ public class MySQLInputPlugin
return new MySQLColumnGetterFactory(pageBuilder, dateTimeZone);
}
- private void loadTimeZoneMappings()
+ private void loadTimeZoneMappings(int version)
{
+ String timeUtilClassName;
+ String timeZonePropName;
+
+ if ( version < 8 ) {
+ timeUtilClassName = "com.mysql.jdbc.TimeUtil";
+ timeZonePropName = "/com/mysql/jdbc/TimeZoneMapping.properties";
+ } else {
+ timeUtilClassName = "com.mysql.cj.util.TimeUtil";
+ timeZonePropName = "/com/mysql/cj/util/TimeZoneMapping.properties";
+ }
// Here initializes com.mysql.jdbc.TimeUtil.timeZoneMappings static field by calling
// static timeZoneMappings method using reflection.
// The field is usually initialized when Driver#connect method is called. But the field
@@ -156,9 +169,11 @@ public class MySQLInputPlugin
// that loaded com.mysql.jdbc.TimeUtil class rather than system class loader to read the
// property file because the file should be in the same classpath with the class.
// Here implements a workaround as as workaround.
+
+
Field f = null;
try {
- Class<?> timeUtilClass = Class.forName("com.mysql.jdbc.TimeUtil");
+ Class<?> timeUtilClass = Class.forName(timeUtilClassName);
f = timeUtilClass.getDeclaredField("timeZoneMappings");
f.setAccessible(true);
@@ -166,7 +181,7 @@ public class MySQLInputPlugin
if (timeZoneMappings == null) {
timeZoneMappings = new Properties();
synchronized (timeUtilClass) {
- timeZoneMappings.load(this.getClass().getResourceAsStream("/com/mysql/jdbc/TimeZoneMapping.properties"));
+ timeZoneMappings.load(this.getClass().getResourceAsStream(timeZonePropName));
}
f.set(null, timeZoneMappings);
}
dmikurube
commented
3 years ago @hito4t @hiroyuki-sato The integer getMajorVersion() does look simpler than parsing a version string, but to be honest, class loading seems simpler to me, because of those questions below:
5 uses com.mysql.jdbc.TimeUtil and 8 uses com.mysql.cj.util.TimeUtil?Considering these cases, the version if-statements will grow complicatedly. Rather than that, making practical decisions based on the "actual classes/resources found" would make things simpler.
hiroyuki-sato
commented
3 years ago @dmikurube @hito4t
Here is my current opinion.
getMajorVersion() is betterThere are two reasons.
getMajorVersion() can switch the class name com.mysql.jdbc.Driver and com.mysql.cj.jdbc.Driver easily If we supports MySQL only.TimeZoneMapping.properties and TimeUtil. So, It seems requires additional rework.I don't modify the following code yet, current implementation load driver like the following.
loadDriver("com.mysql.jdbc.Driver", t.getDriverPath());When I use this code with MySQL Connector/J v8, It outputs the following warning.
Loading class `com.mysql.jdbc.Driver'. This is deprecated. The new driver class is `com.mysql.cj.jdbc.Driver'. The driver is automatically registered via the SPI and manual loading of the driver class is generally unnecessary.If we support MySQL 5/8 driver only, We can switch the class name easily.
if ( t.getDriverPath() < 8 ) {
loadDriver("com.mysql.jdbc.Driver", t.getDriverPath());
} else {
loadDriver("com.mysql.cj.jdbc.Driver", t.getDriverPath());
}But It is hard to switch if we use MariaDB Connector/J. and I don't konw how to write is using classForName
It seems that the current input-mysql implementation hard to support MariaDB Connector/J(Because it doesn't have TimeZoneMapping.properties and TimeUtil) It's need more reworks. If we supports MySQL Connector/J only, getMajorVersion seems simpler way.
I have no idea how to write loadDriver() part using classForName yet.
hiroyuki-sato
commented
3 years ago How does this work for MariaDB JDBC, and, their future updates? Does MariaDB maintain their "major version" strategy discriminable from MySQL's one?
Probably, the latest Mariadb Connecotr/J getMajorVersion returns 2.
So, getMajorVersion() can't use MySQL/MariaDB version checking if we support MariaDB Connector/J
https://github.com/mariadb-corporation/mariadb-connector-j/blob/master/src/main/java/org/mariadb/jdbc/Driver.java#L146-L148 https://github.com/mariadb-corporation/mariadb-connector-j/blob/master/src/main/java/org/mariadb/jdbc/internal/util/constant/Version.java
Is it documented and guaranteed that 5 uses com.mysql.jdbc.TimeUtil and 8 uses com.mysql.cj.util.TimeUtil?
I can't find any documentation about TimeUtil.
rajyan
commented
1 month ago @hiroyuki-sato
Is there anything I can do to help move this forward? What was the problem for merging https://github.com/embulk/embulk-input-jdbc/pull/200 ?
hiroyuki-sato
commented
1 month ago Hello, @rajyan
Please check this discussion. https://github.com/orgs/embulk/discussions/19
We need to understand the following.
So, We need a summary of What Connector/J changed in the newer version and What changes should be made in the Embulk plugin.
rajyan
commented
1 month ago @hiroyuki-sato
Hi, thank you for your swift response! Yeah, updating default JDBC driver seems a tough task, there are a lot of breaking changes...
Is there way to avoid this timeUtil error at least? Then we can try embulk with Connector/J 8.x (using driver_path) and test to search for the breaking changes.
I'll contribute back to embulk when I have some time to help upgrading the default jdbc driver versions.
For example, this PR https://github.com/embulk/embulk-output-jdbc/pull/343/files (still in progress, II'll look into it later) is from my colleague which is trying to solve the incompatibility of nullCatalogMeansCurrent defaults (ref https://dev.mysql.com/doc/connectors/en/connector-j-upgrading-to-8.0.html).
hiroyuki-sato
commented
1 month ago @rajyan I'll ask other maintainers about your proposal. It is a good start for me. Please wait.
hiroyuki-sato
commented
1 month ago @rajyan I revised #259 and am waiting for the review.
It seems that
com.mysql.jdbc.TimeUtilremoved.