Remove setConverted method usage #70
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Not sure why the version check is failing, but it seems to be a common problem in this repo. |
|
I think merging this in should be safe, since parent-pom points to 1.2 and after this is merged I can run an interproject build and try a |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Attempts resolution similar to the one in #27 but attempts to not run into the same types of failures seen previously. Digging into JOOQ's source, it looks like DSL::param returns a Val object, which inherits the implementation of method setConverted0 (which is called via the inherited method setConverted). This inherited
setConverted0method performs the conversion withgetDataType().convert(value), instead ofgetType().cast(value)used in #27. This should hopefully get us the behavior we want without the whole mutation business.#27 eventually was reverted in #28 due to failing builds because of failed casts. When I unreverted #27 to test it out, it also caused this more recent unit test to fail, also due to a failed cast.
After making this change I verified locally that both the newer unit test and the test referenced in the slack thread which led to the revert (I can provide more details about this in slack if desired, but I thought I'd leave the private links out of the public GitHub) no longer experience failed casting, so I think we may be more successful with this approach.