HHH-16858 improve typechecking for comparisons/assignments #6910
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
In particular, correctly typecheck comparisons between enums and other enums, and literal integers / strings. Actually I'm not a great fan of comparing enums with int/string literals but since we used to support it in 5, and kinda mostly support it in earlier releases of 6, on balance we might as well continue to allow it.
- use the known JdbcType which previously we didn't have proper access to - and accidentally fix HHH-16859 by side-effect (I didn't really want to fix that one, but it was easier to fix it than to unfix it.)
gavinking
force-pushed
the
comparison-typechecking
branch
from
1a5b75f
to
0e28b24
Compare
|
Thanks for your pull request! This pull request does not follow the contribution rules. Could you have a look? ❌ All commit messages should start with a JIRA issue key matching pattern › This message was automatically generated. |
gavinking
force-pushed
the
comparison-typechecking
branch
from
7f91d54
to
395259b
Compare
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.
In particular, correctly type check comparisons between enums and other enums, literal integers, and literal strings.
Actually I'm not a great fan of comparing enums with int/string literals but since we used to support it in 5, and kinda mostly support it in earlier releases of 6, on balance we might as well continue to allow it. It's not like it's wrong wrong, just something that's probably bad most of the time.
Indeed, this patch goes so far as to allow this sort of comparison in general when JDBC types match in a "reasonable" way. Where "reasonable" involves some handwavey heuristics. Not perfect but I guess this probably a case where worse is better.
UPDATE: I was also obligated to "fix" the non-bug HHH-16859 by side-effect of cleaning up another issue with type-checking of enum function args. Once again, I'm really not sold that
max(enum)is a great thing to write, but given that we used to allow it, it's pretty hard for me to justify inserting new code to specifically disallow it.