Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[jdbc] Upgrade MySQL JDBC driver to 8.0.30 #13242

Merged
merged 1 commit into from
Aug 31, 2022

Conversation

jlaur
Copy link
Contributor

@jlaur jlaur commented Aug 10, 2022

See

Tested by installing compiled bundle as well as mysql-connector into addons directory:

openhabian@openhabian:/usr/share/openhab/addons $ ll *mysql* *jdbc*
-rw-r--r-- 1 openhabian openhab 2.4M Jul  1 05:18 mysql-connector-java.jar
-rw-r--r-- 1 openhabian openhab 296K Aug 10 22:26 org.openhab.persistence.jdbc-3.4.0-SNAPSHOT.jar

Then checking in database that items are persisted after the upgrade.

@jlaur jlaur requested a review from a team as a code owner August 10, 2022 20:49
@jlaur jlaur marked this pull request as draft August 10, 2022 21:17
@jlaur
Copy link
Contributor Author

jlaur commented Aug 10, 2022

Investigating rule errors that started to show:
2022-08-10 23:15:26.875 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'danfoss-7' failed: class java.time.LocalDateTime cannot be cast to class java.sql.Timestamp (java.time.LocalDateTime is in module java.base of loader 'bootstrap'; java.sql.Timestamp is in module java.sql of loader 'platform') in danfoss

It might be related to the way I installed the bundle, so will now try to install 8.0.28 the same way.

@jlaur jlaur marked this pull request as ready for review August 10, 2022 21:25
@jlaur
Copy link
Contributor Author

jlaur commented Aug 10, 2022

It might be related to the way I installed the bundle, so will now try to install 8.0.28 the same way.

It happens also when using version from main branch with connector 8.0.28 installed the same way. So something is wrong with the way I'm testing it (on a 3.3 system). Good news is that so far I have not found any issues (besides this one), bad news is that I have not been able to fully test this upgrade. Items are persisted when updated and restored on startup - that much I was able to verify.

@jlaur
Copy link
Contributor Author

jlaur commented Aug 19, 2022

@kaikreuzer - should mysql-connector-java also be uploaded to jfrog for this upgrade to work?

@kaikreuzer
Copy link
Member

@jlaur I guess so, yes, and I just uploaded it.
What is weird, though, is the fact that 8.0.28 wasn't uploaded before, I can only see version 8.0.15.

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think your errors are related to this upgrade.
Moving two digits higher on the patch level should hopefully not have any negative impact, so I'd suggest to merge.

@kaikreuzer kaikreuzer merged commit dddf463 into openhab:main Aug 31, 2022
@kaikreuzer kaikreuzer added this to the 3.4 milestone Aug 31, 2022
@kaikreuzer kaikreuzer added the enhancement An enhancement or new feature for an existing add-on label Aug 31, 2022
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Sep 3, 2022
@jlaur
Copy link
Contributor Author

jlaur commented Sep 11, 2022

@kaikreuzer - unfortunately I'm now seeing this after upgrading to M2:

2022-09-11 22:35:02.679 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'danfoss-7' failed: class java.time.LocalDateTime cannot be cast to class java.sql.Timestamp (java.time.LocalDateTime is in module java.base of loader 'bootstrap'; java.sql.Timestamp is in module java.sql of loader 'platform') in danfoss

I'll need some time to understand this problem. Any ideas would be much appreciated.

DSL rule line triggering this problem:

var hasChanged = Anyone_Home.changedSince(now.minusMinutes(29))

Persistence table for item Anyone_Home is defined with Time column of type DATETIME.

@kaikreuzer
Copy link
Member

@jlaur I just had a look in the code and the only place where there is a cast to Timestamp seems to be

protected ZonedDateTime objectAsDate(Object v) {
if (v instanceof java.lang.String) {
return ZonedDateTime.ofInstant(java.sql.Timestamp.valueOf(v.toString()).toInstant(),
ZoneId.systemDefault());
}
return ZonedDateTime.ofInstant(((java.sql.Timestamp) v).toInstant(), ZoneId.systemDefault());
}

Maybe the lib changed to now return LocalDateTime instead of Timestamp? Possibly, adding another if statement in the method above for dealing with LocalDateTime instances could fix the issue?

@jlaur
Copy link
Contributor Author

jlaur commented Sep 12, 2022

Thank you @kaikreuzer for pointing to this place in the code, it looks quite related indeed. I'll try to fix and test this in the evening. I also found this article: https://dev.mysql.com/blog-archive/support-for-date-time-types-in-connector-j-8-0/ which perhaps provides some more context. The POM upgrade was 8.0.28 to 8.0.30, but actually the feature.xml was at 8.0.22, so before 8.0.23.

@jlaur
Copy link
Contributor Author

jlaur commented Sep 12, 2022

@kaikreuzer - you hit the nail on the head. Thanks a lot! I have created #13382 for fixing this problem.

@kaikreuzer
Copy link
Member

Excellent - glad I could help!

leifbladt pushed a commit to leifbladt/openhab-addons that referenced this pull request Oct 15, 2022
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
Signed-off-by: Jacob Laursen <[email protected]>
Signed-off-by: Andras Uhrin <[email protected]>
marcelGoerentz pushed a commit to marcelGoerentz/openhab-addons that referenced this pull request Nov 14, 2022
@jlaur jlaur deleted the jdbc-mysql-upgrade branch December 17, 2022 16:40
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants