[bugfix] fix(sql): strip trailing -- comments before OPTIONS regex match

pinot-common/src/main/java/org/apache/pinot/sql/parsers/ParserUtils.java

@@ -47,17 +47,90 @@ public static void validateFunction(String canonicalName, List<Expression> opera
    */
   public static String sanitizeSql(String sql) {
 
-    // 1. Remove trailing whitespaces
+    // 1. Strip single-line SQL comments (-- ... to end of line).
+    // The legacy OPTIONS regex anchors at end-of-string, so without this step a query
+    // like "SELECT col1 FROM foo -- option(skipUpsert=true)" would be mistakenly matched
+    // as if skipUpsert were a real query option.
+    sql = stripSingleLineComments(sql);
 
+    // 2. Remove trailing whitespace
     int endIndex = sql.length() - 1;
     while (endIndex >= 0 && Character.isWhitespace(sql.charAt(endIndex))) {
       endIndex--;
     }
-    sql = sql.substring(0, endIndex + 1);
-
-    // Likewise extend for other improvements
+    return sql.substring(0, endIndex + 1);
+  }
 
-    return sql;
+  /**
+   * Returns the sql string with all single-line SQL comments (-- ... to end of line) removed,
+   * respecting single-quoted string literals, double-quoted identifiers, and block comments.
+   * A "--" found inside a block comment or a quoted context is not treated as a comment marker.
+   */
+  static String stripSingleLineComments(String sql) {
+    StringBuilder result = new StringBuilder(sql.length());
+    int len = sql.length();
+    boolean inSingleQuote = false;
+    boolean inDoubleQuote = false;
+    boolean inBlockComment = false;
+    int i = 0;
+    while (i < len) {
+      char c = sql.charAt(i);
+      if (inBlockComment) {
+        result.append(c);
+        if (c == '*' && i + 1 < len && sql.charAt(i + 1) == '/') {
+          result.append('/');
+          inBlockComment = false;
+          i += 2;
+        } else {
+          i++;
+        }
+      } else if (inSingleQuote) {
+        result.append(c);
+        if (c == '\'' && i + 1 < len && sql.charAt(i + 1) == '\'') {
+          result.append('\'');
+          i += 2; // '' escape inside a single-quoted literal
+        } else {
+          if (c == '\'') {
+            inSingleQuote = false;
+          }
+          i++;
+        }
+      } else if (inDoubleQuote) {
+        result.append(c);
+        if (c == '"' && i + 1 < len && sql.charAt(i + 1) == '"') {
+          result.append('"');
+          i += 2; // "" escape inside a double-quoted identifier
+        } else {
+          if (c == '"') {
+            inDoubleQuote = false;
+          }
+          i++;
+        }
+      } else {
+        if (c == '\'') {
+          inSingleQuote = true;
+          result.append(c);
+          i++;
+        } else if (c == '"') {
+          inDoubleQuote = true;
+          result.append(c);
+          i++;
+        } else if (c == '/' && i + 1 < len && sql.charAt(i + 1) == '*') {
+          inBlockComment = true;
+          result.append(c);
+          i++;
+        } else if (c == '-' && i + 1 < len && sql.charAt(i + 1) == '-') {
+          // Skip from here to end of line; the newline itself is kept.
+          while (i < len && sql.charAt(i) != '\n') {
+            i++;
+          }
+        } else {
+          result.append(c);
+          i++;
+        }
+      }
+    }
+    return result.toString();
   }
 
   private static void validateJsonExtractScalarFunction(List<Expression> operands) {

pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java

@@ -879,6 +879,27 @@ public void testQueryOptions() {
     } catch (SqlCompilationException e) {
       Assert.assertTrue(e.getCause() instanceof ParseException);
     }
+
+    // Options inside SQL comments must NOT be honored.
+    // A trailing -- comment should not be mistaken for a real query option.
+    pinotQuery = compileToPinotQuery(
+        "SELECT col1, count(*) FROM foo GROUP BY col1 -- option(skipUpsert=true)");
+    Assert.assertTrue(pinotQuery.getQueryOptions() == null || pinotQuery.getQueryOptions().isEmpty(),
+        "option(...) inside a -- comment must not be parsed as a query option");
+
+    // Same check when the -- comment appears inline after a WHERE predicate (multi-line query).
+    // The regex finds OPTION() starting from the 'O', ignoring the preceding '--', so without
+    // the fix this would incorrectly honour skipUpsert.
+    pinotQuery = compileToPinotQuery(
+        "select *\nfrom foo\nwhere uuid = 1 --OPTION(skipUpsert = true)");
+    Assert.assertTrue(pinotQuery.getQueryOptions() == null || pinotQuery.getQueryOptions().isEmpty(),
+        "OPTION(...) after -- in a WHERE clause must not be parsed as a query option");
+
+    // A /* */ block comment should also not trigger the OPTIONS parser.
+    pinotQuery = compileToPinotQuery(
+        "SELECT col1, count(*) FROM foo GROUP BY col1 /* option(skipUpsert=true) */");
+    Assert.assertTrue(pinotQuery.getQueryOptions() == null || pinotQuery.getQueryOptions().isEmpty(),
+        "option(...) inside a /* */ comment must not be parsed as a query option");
   }
 
   @Test