mihaibudiu commented on code in PR #4400:
URL: https://github.com/apache/calcite/pull/4400#discussion_r2109707961
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2364,6 +2366,38 @@ private RexNode simplifyCeilFloor(RexCall e) {
ImmutableList.of(operand, e.getOperands().get(1)));
}
+ /** Simplify TRIM function by eliminating nested duplication.
+ *
+ * <p>Examples:
+ * <ul>
+ *
+ * <li>{@code trim(trim(' aa '))} returns {@code trim(' aa ')}
+ *
+ * <li>{@code trim(BOTH ' ' from trim(BOTH ' ' from ' aa '))}
+ * returns {@code trim(BOTH ' ' from ' aa ')}
+ *
+ * <li>{@code trim(LEADING 'a' from trim(BOTH ' ' from ' aa '))} does not
change
+ *
+ * </ul>
+ */
+ private RexNode simplifyTrim(RexCall e) {
+ if (e.getOperands().size() != 3) {
+ return e;
+ }
+
+ if (e.getOperands().get(2) instanceof RexCall) {
+ RexCall childNode = (RexCall) e.getOperands().get(2);
+ // only strings with the same trim method and deduplication will be
eliminated.
+ if (childNode.getKind() == SqlKind.TRIM
+ &&
simplify(e.operands.get(0)).equals(simplify(childNode.operands.get(0)))
Review Comment:
This will work even if the trim operand 0 is not a literal, but an
expression (e.g., a column name).
Perhaps this is fine, but then you should add a test for this case.
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -2804,6 +2804,122 @@ private SqlDialect nonOrdinalDialect() {
.withPresto().ok(exptectedPresto);
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7042";>[CALCITE-7042]
+ * Trim function does not have idempotency simplification</a>. */
+ @Test void testTrimEliminated() {
+ // eliminate trim test cases:
+ final String query = "SELECT TRIM(both 'a' from TRIM(both 'a' from
\"brand_name\"))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto = "SELECT TRIM(\"brand_name\", 'a')\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query)
+ .withPresto().ok(exptectedPresto);
+
+ final String query1 = "SELECT TRIM(TRAILING cast('a' as char(1)) from"
+ + " TRIM(TRAILING 'a' from \"brand_name\"))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto1 = "SELECT RTRIM(\"brand_name\", 'a')\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query1)
+ .withPresto().ok(exptectedPresto1);
+
+ final String query2 = "SELECT TRIM(TRAILING cast('a' as char(1)) from"
+ + " TRIM(TRAILING cast('a' as char(1)) from \"brand_name\"))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto2 = "SELECT RTRIM(\"brand_name\", 'a')\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query2)
+ .withPresto().ok(exptectedPresto2);
+
+ final String query3 = "SELECT TRIM(TRIM(\"brand_name\"))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto3 = "SELECT TRIM(\"brand_name\")\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query3)
+ .withPresto().ok(exptectedPresto3);
+
+ final String query4 = "SELECT TRIM(LEADING 'a' from TRIM(LEADING 'a' from
\"brand_name\"))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto4 = "SELECT LTRIM(\"brand_name\", 'a')\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query4)
+ .withPresto().ok(exptectedPresto4);
+
+ final String query5 = "SELECT TRIM(TRAILING 'a' from TRIM(TRAILING 'a'
from \"brand_name\"))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto5 = "SELECT RTRIM(\"brand_name\", 'a')\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query5)
+ .withPresto().ok(exptectedPresto5);
+
+ // not eliminate trim test cases:
+ final String query6 = "SELECT TRIM(LEADING 'a' from
TRIM(\"brand_name\"))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto6 = "SELECT LTRIM(TRIM(\"brand_name\"), 'a')\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query6)
+ .withPresto().ok(exptectedPresto6);
+
+ final String query7 = "SELECT TRIM(LEADING 'b' from TRIM(LEADING 'a' from
\"brand_name\"))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto7 = "SELECT LTRIM(LTRIM(\"brand_name\", 'a'),
'b')\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query7)
+ .withPresto().ok(exptectedPresto7);
+
+ final String query8 = "SELECT TRIM(LEADING 'a' from TRIM(BOTH 'a' from
\"brand_name\"))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto8 = "SELECT LTRIM(TRIM(\"brand_name\", 'a'),
'a')\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query8)
+ .withPresto().ok(exptectedPresto8);
+
+ final String query9 = "SELECT TRIM(TRIM(BOTH 'a' from \"brand_name\"))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto9 = "SELECT TRIM(TRIM(\"brand_name\", 'a'))\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query9)
+ .withPresto().ok(exptectedPresto9);
+
+ // NULL argument cases:
+ final String query10 = "SELECT TRIM(TRIM(BOTH NULL from \"brand_name\"))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto10 = "SELECT CAST(NULL AS VARCHAR(60))\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query10)
+ .withPresto().ok(exptectedPresto10);
+
+ final String query11 = "SELECT TRIM(TRIM(BOTH ' ' from NULL))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto11 = "SELECT CAST(NULL AS VARCHAR)\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query11)
+ .withPresto().ok(exptectedPresto11);
+
+ final String query12 = "SELECT TRIM(TRAILING NULL from"
+ + " TRIM(TRAILING NULL from \"brand_name\"))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto12 = "SELECT CAST(NULL AS VARCHAR(60))\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query12)
+ .withPresto().ok(exptectedPresto12);
+
+ final String query13 = "SELECT TRIM(TRAILING ' ' from TRIM(TRAILING ' '
from NULL))\n"
+ + "from \"foodmart\".\"product\"";
+ final String exptectedPresto13 = "SELECT CAST(NULL AS VARCHAR)\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query13)
+ .withPresto().ok(exptectedPresto13);
+
+ final String query14 = "SELECT TRIM(TRAILING NULL from TRIM(TRAILING NULL
from NULL))\n"
Review Comment:
I was also hoping for a test where the arguments are not legal, e.g., the
trimmed string has more than one character.
--
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]
