Copilot commented on code in PR #53457:
URL: https://github.com/apache/doris/pull/53457#discussion_r2212767988


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilderAssistant.java:
##########
@@ -35,6 +31,10 @@
  */
 public class LogicalPlanBuilderAssistant {
 
+    // acording to MySQL's dev doc: 
https://dev.mysql.com/doc/refman/8.4/en/string-literals.html

Review Comment:
   Minor typo: change "acording" to "according" in the comment.
   ```suggestion
       // according to MySQL's dev doc: 
https://dev.mysql.com/doc/refman/8.4/en/string-literals.html
   ```



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilderAssistant.java:
##########
@@ -84,21 +84,22 @@ public static String escapeBackSlash(String str) {
     }
 
     /**
-     * Handle Integer literal by BigInteger.
+     * MySQL will trim any inviable char and blank at the beginning of the 
string,
+     * and stop at the first \0 if it exists. we follow the behavior.
+     *
+     * @param value the StringLikeLiteral value
+     * @return the alias generated from the value
      */
-    public static Literal handleIntegerLiteral(String value) {
-        BigInteger bigInt = new BigInteger(value);
-        if (BigInteger.valueOf(bigInt.byteValue()).equals(bigInt)) {
-            return new TinyIntLiteral(bigInt.byteValue());
-        } else if (BigInteger.valueOf(bigInt.shortValue()).equals(bigInt)) {
-            return new SmallIntLiteral(bigInt.shortValue());
-        } else if (BigInteger.valueOf(bigInt.intValue()).equals(bigInt)) {
-            return new IntegerLiteral(bigInt.intValue());
-        } else if (BigInteger.valueOf(bigInt.longValue()).equals(bigInt)) {
-            return new BigIntLiteral(bigInt.longValueExact());
-        } else {
-            return new LargeIntLiteral(bigInt);
+    public static String getStringLiteralAlias(String value) {
+        int i = 0;
+        while (i < value.length() && 
NEED_TRIM_CHARS_FOR_ALIAS.contains(value.charAt(i))) {

Review Comment:
   [nitpick] Using `Set<Character>.contains` in a tight loop causes autoboxing 
overhead; consider using a `boolean` lookup (e.g., a `boolean[]` or 
`String.indexOf`) for better performance.
   ```suggestion
           while (i < value.length() && value.charAt(i) < 
NEED_TRIM_CHARS_FOR_ALIAS.length && NEED_TRIM_CHARS_FOR_ALIAS[value.charAt(i)]) 
{
   ```



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilderAssistant.java:
##########
@@ -84,21 +84,22 @@ public static String escapeBackSlash(String str) {
     }
 
     /**
-     * Handle Integer literal by BigInteger.
+     * MySQL will trim any inviable char and blank at the beginning of the 
string,
+     * and stop at the first \0 if it exists. we follow the behavior.
+     *
+     * @param value the StringLikeLiteral value
+     * @return the alias generated from the value
      */
-    public static Literal handleIntegerLiteral(String value) {
-        BigInteger bigInt = new BigInteger(value);
-        if (BigInteger.valueOf(bigInt.byteValue()).equals(bigInt)) {
-            return new TinyIntLiteral(bigInt.byteValue());
-        } else if (BigInteger.valueOf(bigInt.shortValue()).equals(bigInt)) {
-            return new SmallIntLiteral(bigInt.shortValue());
-        } else if (BigInteger.valueOf(bigInt.intValue()).equals(bigInt)) {
-            return new IntegerLiteral(bigInt.intValue());
-        } else if (BigInteger.valueOf(bigInt.longValue()).equals(bigInt)) {
-            return new BigIntLiteral(bigInt.longValueExact());
-        } else {
-            return new LargeIntLiteral(bigInt);
+    public static String getStringLiteralAlias(String value) {

Review Comment:
   Consider adding a null check for `value` (or `Objects.requireNonNull`) to 
avoid a potential NPE if callers ever pass `null`.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -2586,14 +2587,16 @@ public NamedExpression 
visitNamedExpression(NamedExpressionContext ctx) {
             if (ctx.identifierOrText() == null) {
                 if (expression instanceof NamedExpression) {
                     return (NamedExpression) expression;
-                } else if (expression instanceof Literal) {
-                    return new Alias(expression);
                 } else {
                     int start = ctx.expression().start.getStartIndex();
                     int stop = ctx.expression().stop.getStopIndex();
                     String alias = ctx.start.getInputStream()
                             .getText(new 
org.antlr.v4.runtime.misc.Interval(start, stop));
                     if (expression instanceof Literal) {

Review Comment:
   The alias logic now applies to all `Literal` types, not just 
`StringLikeLiteral`. To preserve original behavior for numeric and other 
literals, consider handling non-string literals with `new Alias(expression)` 
separately, and only apply `getStringLiteralAlias` to `StringLikeLiteral`.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to