[CALCITE-5865] ClassCastException with FLOOR and CEIL on conformances that are not builtin#3327
[CALCITE-5865] ClassCastException with FLOOR and CEIL on conformances that are not builtin#3327gianm merged 1 commit intoapache:mainfrom
Conversation
… that are not builtin CALCITE-5747 introduced some BigQuery-specific logic for FLOOR and CEIL that is keyed off the conformance being SqlConformanceEnum.BIG_QUERY. However, it was implemented in such a way that implementations of SqlConformance that are not SqlConformanceEnum would throw ClassCastException.
tanclary
left a comment
tanclary
left a comment
There was a problem hiding this comment.
Thanks again for opening this fix
| switch (conformance) { | ||
| case BIG_QUERY: | ||
| public static SqlOperator floorCeil(boolean floor, SqlConformance conformance) { | ||
| if (SqlConformanceEnum.BIG_QUERY.equals(conformance)) { |
There was a problem hiding this comment.
Is there a test you can add for this?
There was a problem hiding this comment.
I thought a little about this, but I wasn't sure what kind of test to add, or where.
I could certainly add a test for this specific method verifying that a user-defined conformance works OK. But it doesn't seem likely to be a high-value test. It would be better to somehow write a test that verifies everything works with user-defined conformances. But I didn't see a way to do that after a brief look.
I'm open to thoughts here. If you think it's valuable to write a test for this specific method, LMK and I will add one. Even better, if you have an idea about how to write a more comprehensive test, LMK.
|
Kudos, SonarCloud Quality Gate passed! |
|
LGTM. |
|
@rubenada Thanks for your input, I agree with that. LGTM |
|
Thanks all. I have committed the patch to |
3 participants
CALCITE-5747 introduced some BigQuery-specific logic for FLOOR and CEIL that is keyed off the conformance being
SqlConformanceEnum.BIG_QUERY. However, it was implemented in such a way that implementations ofSqlConformancethat are notSqlConformanceEnumwould throwClassCastException.