[GitHub] [calcite] gianm commented on a diff in pull request #3327: [CALCITE-5865] ClassCastException with FLOOR and CEIL on conformances that are not builtin

["gianm (via GitHub)" <gi...@apache.org>]

gianm commented on code in PR #3327:
URL: https://github.com/apache/calcite/pull/3327#discussion_r1268787648


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java:
##########
@@ -2701,11 +2701,10 @@ public static SqlOperator like(boolean negated, boolean caseSensitive) {
 
   /** Returns the operator for {@code FLOOR} and {@code CEIL} with given floor flag
    * and library. */
-  public static SqlOperator floorCeil(boolean floor, SqlConformanceEnum conformance) {
-    switch (conformance) {
-    case BIG_QUERY:
+  public static SqlOperator floorCeil(boolean floor, SqlConformance conformance) {
+    if (SqlConformanceEnum.BIG_QUERY.equals(conformance)) {

Review 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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org