julianhyde commented on code in PR #4460:
URL: https://github.com/apache/calcite/pull/4460#discussion_r2190363937


##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -6069,6 +6069,21 @@ private void checkLiteral2(String expression, String 
expected) {
     sql(query).withLibrary(SqlLibrary.POSTGRESQL).ok(expected);
   }
 
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-7088";>[CALCITE-7088]
+   * X LIKE '%%' should simply to X=X</a>. */

Review Comment:
   RelToSqllComverterTest tests dialect support. This feature is about 
simplification. So this test should not be here. 
   
   Nowhere do you test what the summary says - that ‘x like “%%”’ simplifies to 
‘x = x’. Is that even true?



##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -511,6 +512,15 @@ private RexNode simplifyLike(RexCall e, RexUnknownAs 
unknownAs) {
     return simplifyGenericNode(e);
   }
 
+  private boolean withSameChar(String str, Character c) {
+    for (Character character : str.toCharArray()) {

Review Comment:
   This is awful. The method name is meaningless to me.  There is no javadoc. 
You box char to Character for no good reason. 
   
   And there is a glaring bug. The method succeeds when the string is empty. 
   
   Get rid of the method and use a built-in mechanism like regex. 



##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3952,9 +3954,13 @@ private void checkSarg(String message, Sarg sarg,
         "OR(IS NOT NULL($0), LIKE($0, '% %'))", "true");
     checkSimplify(or(isNull(ref), like(ref, literal("%"))),
         "true");
+    checkSimplify(or(isNull(ref), like(ref, literal("%%"))),
+        "true");
     checkSimplify(or(isNull(ref), like(ref, literal("%"), literal("#"))),
         "true");
     checkSimplifyUnchanged(like(ref, literal("%A")));
+    checkSimplifyUnchanged(like(ref, literal("%%A")));

Review Comment:
   Maybe ‘ref LIKE “%%A”’ should be simplified. 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to