github-actions[bot] commented on code in PR #68873:
URL: https://github.com/apache/doris/pull/68873#discussion_r4237217340


##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/source/JdbcScanNode.java:
##########
@@ -213,13 +219,183 @@ private String getTvfQuery() {
         if (!projectsTimestamps) {
             return query;
         }
-        String source = query.trim();
-        if (source.endsWith(";")) {
-            source = source.substring(0, source.length() - 1);
+        boolean mysql = jdbcType == TOdbcTableType.MYSQL || jdbcType == 
TOdbcTableType.OCEANBASE;
+        if (mysql && mysqlSqlMode == null) {
+            // Resolve only for wrapped MySQL queries and reuse the result for 
EXPLAIN and scan serialization.
+            org.apache.doris.datasource.jdbc.JdbcExternalCatalog catalog =
+                    (org.apache.doris.datasource.jdbc.JdbcExternalCatalog) 
org.apache.doris.catalog.Env
+                            
.getCurrentEnv().getCatalogMgr().getCatalog(catalogId);
+            mysqlSqlMode = catalog.getJdbcClient().getSessionSqlMode();

Review Comment:
   [P2] Bind the session mode to the TVF's catalog snapshot before scan 
serialization. `getTvfQuery()` now looks up the catalog and its JDBC client 
after `getScanNode` has configured the scan table. A concurrent `ALTER CATALOG` 
can close/null that client before this call, causing an NPE or closed-pool 
failure; if the catalog is reinitialized first, the mode can instead come from 
a new JDBC URL while the scan table still uses the old one. A concurrent DROP 
can also remove the catalog ID. Capture the mode with the same configuration 
and safe client lifetime used to configure the scan table, then reuse it during 
`toThrift`; cover a reset between TVF setup and serialization.



##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/source/JdbcScanNode.java:
##########
@@ -213,13 +219,183 @@ private String getTvfQuery() {
         if (!projectsTimestamps) {
             return query;
         }
-        String source = query.trim();
-        if (source.endsWith(";")) {
-            source = source.substring(0, source.length() - 1);
+        boolean mysql = jdbcType == TOdbcTableType.MYSQL || jdbcType == 
TOdbcTableType.OCEANBASE;
+        if (mysql && mysqlSqlMode == null) {
+            // Resolve only for wrapped MySQL queries and reuse the result for 
EXPLAIN and scan serialization.
+            org.apache.doris.datasource.jdbc.JdbcExternalCatalog catalog =
+                    (org.apache.doris.datasource.jdbc.JdbcExternalCatalog) 
org.apache.doris.catalog.Env
+                            
.getCurrentEnv().getCatalogMgr().getCatalog(catalogId);
+            mysqlSqlMode = catalog.getJdbcClient().getSessionSqlMode();
+        }
+        String source = stripTerminalDelimiter(query.trim(),
+                mysql && mysqlSqlMode.contains("NO_BACKSLASH_ESCAPES"), mysql 
&& mysqlSqlMode.contains("ANSI_QUOTES"));
+        // WITH SESSION must stay outside the derived table; terminate 
trailing line comments with a newline.
+        int start = jdbcType == TOdbcTableType.TRINO ? 
trinoSessionQueryStart(source) : 0;
+        return source.substring(0, start) + "SELECT " + Joiner.on(", 
").join(columns)
+                + " FROM (" + source.substring(start) + "\n) 
doris_jdbc_source";
+    }
+
+    private String stripTerminalDelimiter(String sql, boolean 
noBackslashEscapes, boolean ansiQuotes) {
+        boolean mysql = jdbcType == TOdbcTableType.MYSQL || jdbcType == 
TOdbcTableType.OCEANBASE;
+        List<Integer> delimiters = new java.util.ArrayList<>();
+        for (int i = 0; i < sql.length();) {
+            char c = sql.charAt(i);
+            if (Character.isWhitespace(c)) {
+                i++;
+            } else if (jdbcType == TOdbcTableType.CLICKHOUSE
+                    && (sql.startsWith("//", i) || sql.startsWith("#!", i) || 
sql.startsWith("# ", i))) {
+                // ClickHouse comments may contain unmatched quotes; their 
contents end only at LF.
+                int newline = sql.indexOf('\n', i + 2);
+                i = newline < 0 ? sql.length() : newline + 1;
+            // MySQL's second dash needs a following whitespace/control 
character; --1 is arithmetic.
+            } else if ((sql.startsWith("--", i) && (!mysql || (i + 2 < 
sql.length()
+                    && (Character.isWhitespace(sql.charAt(i + 2)) || 
Character.isISOControl(sql.charAt(i + 2))))))
+                    || (c == '#'
+                    && (jdbcType == TOdbcTableType.MYSQL || jdbcType == 
TOdbcTableType.OCEANBASE))) {
+                while (i < sql.length() && sql.charAt(i) != '\n' && 
sql.charAt(i) != '\r') {
+                    i++;
+                }
+            } else if (sql.startsWith("/*", i)) {
+                i = skipBlockComment(sql, i, jdbcType == 
TOdbcTableType.CLICKHOUSE);
+            } else if (jdbcType == TOdbcTableType.CLICKHOUSE && c == '$') {
+                i = skipClickHouseHeredoc(sql, i);
+                delimiters.clear();
+            } else if (c == '\'' || c == '"' || c == '`') {

Review Comment:
   [P2] Scan MariaDB bracket identifiers when stripping TVF delimiters. MariaDB 
connections map to MYSQL here, and with `sql_mode=MSSQL` the query `SELECT ts 
AS [x'y] FROM t;` has a valid bracket quoted alias. For a mapped TIMESTAMP TVF, 
this scanner treats the apostrophe inside `[...]` as an opening string quote, 
misses the terminal `;`, and emits `FROM (SELECT ts AS [x'y] FROM t;\n)`, which 
is invalid. The previous wrapper removed the final semicolon. Handle bracket 
quoted identifiers (including escaped `]]`) when MSSQL mode is active and cover 
this TVF case. MariaDB documents [identifier 
quoting](https://mariadb.com/docs/server/reference/sql-structure/sql-language-structure/identifier-names)
 and [MSSQL 
mode](https://mariadb.com/docs/server/server-management/variables-and-modes/sql_mode).



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