github-actions[bot] commented on code in PR #68873:
URL: https://github.com/apache/doris/pull/68873#discussion_r4237117442
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcTable.java:
##########
@@ -152,7 +152,18 @@ public String getInsertSql(List<String> insertCols) {
if (i != 0) {
sb.append(", ");
}
- sb.append("?");
+ Column column = getColumn(insertCols.get(i));
+ TOdbcTableType remoteType = getJdbcTableType();
+ // Legacy drivers bind zoned timestamps as VARCHAR; parse their
explicit offsets on the server.
+ if (column != null && column.getType().isTimeStampTz()
+ && (remoteType == TOdbcTableType.ORACLE || remoteType ==
TOdbcTableType.OCEANBASE_ORACLE)) {
+ sb.append("TO_TIMESTAMP_TZ(?, 'YYYY-MM-DD HH24:MI:SS.FF6
TZH:TZM')");
+ } else if (column != null && column.getType().isTimeStampTz() &&
remoteType == TOdbcTableType.PRESTO) {
Review Comment:
[P1] Preserve Oracle zoned-timestamp inserts during rolling FE/BE upgrades.
This new `TO_TIMESTAMP_TZ(?, 'YYYY-MM-DD HH24:MI:SS.FF6 TZH:TZM')` expects the
new BE's text bind, but an old BE still binds an `OffsetDateTime` as JDBC
`TIMESTAMP_WITH_TIMEZONE`. Oracle must convert that typed value to text under
the session `NLS_TIMESTAMP_TZ_FORMAT` before applying this explicit mask, so a
different session mask breaks new-FE/old-BE writes. The reverse old-FE/new-BE
pairing sends the new text bind to a raw `?` and again relies on session
conversion. Keep the FE SQL and BE binding compatible across both version
pairings, and exercise a nonmatching NLS mask. See [Oracle
TO_TIMESTAMP_TZ](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/TO_TIMESTAMP_TZ.html)
and
[NLS_TIMESTAMP_TZ_FORMAT](https://docs.oracle.com/en/database/oracle/oracle-database/19/refrn/NLS_TIMESTAMP_TZ_FORMAT.html).
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/source/JdbcScanNode.java:
##########
@@ -213,13 +219,151 @@ 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++;
+ // 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 (c == '\'' || c == '"' || c == '`') {
+ char quote = c;
+ for (i++; i < sql.length(); i++) {
+ // ANSI_QUOTES makes double quotes identifiers, where
backslashes never escape the closing quote.
+ // Both quoting modes belong to the remote session.
+ if (sql.charAt(i) == '\\' && !(mysql && noBackslashEscapes)
+ && !(mysql && ansiQuotes && quote == '"') && quote
!= '`'
Review Comment:
[P2] Honor ClickHouse's escaped backticks when scanning identifiers.
ClickHouse permits a backslash-escaped backtick inside a backtick-quoted
identifier, but this condition excludes backtick quotes from escape skipping.
In a timestamp TVF query with an identifier containing a backslash-escaped
internal backtick followed by a terminal `;`, this closes the identifier early,
treats its real closing backtick as a new opener, and leaves the final
semicolon inside the generated `FROM (...)` subquery. The prior wrapper
stripped that terminal semicolon. Apply ClickHouse identifier escaping in this
branch and cover the escaped-backtick case. See the [ClickHouse syntax
reference](https://clickhouse.com/docs/reference/syntax).
##########
fe/be-java-extensions/jdbc-scanner/pom.xml:
##########
@@ -35,6 +35,12 @@ under the License.
</properties>
<dependencies>
+ <dependency>
+ <groupId>com.oracle.database.jdbc</groupId>
+ <artifactId>ojdbc11</artifactId>
+ <version>23.26.2.0.0</version>
Review Comment:
[P2] Keep the configured Oracle integration driver from being shadowed.
Adding `ojdbc11` to the test classpath makes
`JdbcZonedTimestampIntegrationTest.withDriver` resolve
`oracle.jdbc.OracleDriver` from its parent classloader before the JAR named by
`oracle.integration.driverJar`; its `URLClassLoader` uses
`getClass().getClassLoader()` as parent and calls `loadClass`. Consequently the
legacy-driver matrix silently exercises 23.26.2 instead of the requested
driver, hiding the compatibility behavior this PR needs to test. Isolate the
selected driver or verify that its class actually came from the configured JAR.
See [URLClassLoader parent-first
delegation](https://docs.oracle.com/javase/8/docs/api/java/net/URLClassLoader.html).
##########
fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/TrinoJdbcExecutor.java:
##########
@@ -42,13 +42,22 @@
public class TrinoJdbcExecutor extends BaseJdbcExecutor {
private static final DateTimeFormatter TIMESTAMP_TZ_WRITE_FORMATTER =
DateTimeFormatter.ofPattern("uuuu-MM-dd HH:mm:ss.SSSSSS");
+ private static final DateTimeFormatter ARRAY_TIMESTAMP_FORMATTER = new
java.time.format.DateTimeFormatterBuilder()
+ .appendPattern("uuuu-MM-dd HH:mm:ss")
+ .appendFraction(java.time.temporal.ChronoField.NANO_OF_SECOND, 0,
9, true)
+ .appendLiteral(' ').appendZoneOrOffsetId().toFormatter();
public TrinoJdbcExecutor(byte[] thriftParams) throws Exception {
super(thriftParams);
}
@Override
protected void setTimestampTz(int parameterIndex, LocalDateTime value)
throws SQLException {
+ if (config != null && config.getTableType() ==
org.apache.doris.thrift.TOdbcTableType.PRESTO) {
+ // PrestoDB requires VARCHAR plus a server-side cast; its driver
rejects JDBC type 2014.
+ preparedStatement.setString(parameterIndex,
value.format(TIMESTAMP_TZ_WRITE_FORMATTER) + " UTC");
+ return;
Review Comment:
[P1] Preserve PRESTO zoned-timestamp writes with an old FE. For
`io.prestosql.jdbc.PrestoDriver` 350, the previous JDBC 2014 `setObject` bind
emits a typed zoned timestamp, while this new `setString` emits VARCHAR. An old
FE still supplies raw `VALUES (?)`, without this head's matching `CAST(? AS
TIMESTAMP(6) WITH TIME ZONE)`, and Presto cannot implicitly coerce that VARCHAR
to the zoned timestamp column. Thus a BE-first rolling upgrade breaks
previously working non-null writes. Gate the text bind on a compatible FE SQL
contract or retain a version-compatible typed bind. See the [PrestoSQL 350 JDBC
binder](https://github.com/trinodb/trino/blob/350/presto-jdbc/src/main/java/io/prestosql/jdbc/PrestoPreparedStatement.java)
and [type coercion
rules](https://github.com/trinodb/trino/blob/350/presto-main/src/main/java/io/prestosql/type/TypeCoercion.java).
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/source/JdbcScanNode.java:
##########
@@ -213,13 +219,151 @@ 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++;
+ // 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 (c == '\'' || c == '"' || c == '`') {
+ char quote = c;
Review Comment:
[P2] Skip ClickHouse heredoc literals as a whole. ClickHouse treats
`$tag$...$tag$` content as raw text. For the valid timestamp TVF source `SELECT
ts FROM t WHERE length($h$foo'bar$h$) > 0;`, this scanner interprets the
apostrophe inside the heredoc as an opening quote and leaves the terminal
semicolon inside the generated derived table; the old wrapper stripped it.
Match the closing heredoc tag before interpreting contained quotes or
semicolons, and cover this literal form. See the [ClickHouse heredoc
syntax](https://clickhouse.com/docs/reference/syntax).
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/source/JdbcScanNode.java:
##########
@@ -213,13 +219,151 @@ 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++;
+ // 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 (c == '\'' || c == '"' || c == '`') {
+ char quote = c;
+ for (i++; i < sql.length(); i++) {
+ // ANSI_QUOTES makes double quotes identifiers, where
backslashes never escape the closing quote.
+ // Both quoting modes belong to the remote session.
+ if (sql.charAt(i) == '\\' && !(mysql && noBackslashEscapes)
+ && !(mysql && ansiQuotes && quote == '"') && quote
!= '`'
+ && jdbcType != TOdbcTableType.TRINO && jdbcType !=
TOdbcTableType.PRESTO) {
+ i++;
+ } else if (sql.charAt(i) == quote) {
+ if (i + 1 < sql.length() && sql.charAt(i + 1) ==
quote) {
+ i++;
+ } else {
+ i++;
+ break;
+ }
+ }
+ }
+ delimiters.clear();
+ } else {
+ if (c == ';') {
+ delimiters.add(i);
+ } else {
+ delimiters.clear();
+ }
+ i++;
+ }
+ }
+ // A statement delimiter may precede trailing comments, which must
stay outside SQL literals.
+ // Remove only the terminal delimiter run; interior delimiters still
fail as multi-statements.
+ if (delimiters.isEmpty()) {
+ return sql;
+ }
+ StringBuilder result = new StringBuilder(sql);
+ for (int i = delimiters.size() - 1; i >= 0; i--) {
+ result.deleteCharAt(delimiters.get(i));
}
- return "SELECT " + Joiner.on(", ").join(columns) + " FROM (" + source
+ ") doris_jdbc_source";
+ return result.toString();
}
+ private static int skipBlockComment(String sql, int start, boolean nested)
{
+ // Trino, Presto and MySQL end at the first closing marker; ClickHouse
permits nested comments.
+ int depth = 1;
+ int i = start + 2;
+ while (i < sql.length() && depth > 0) {
+ if (nested && sql.startsWith("/*", i)) {
+ depth++;
+ i += 2;
+ } else if (sql.startsWith("*/", i)) {
+ depth--;
+ i += 2;
+ } else {
+ i++;
+ }
+ }
+ return i;
+ }
+
+ private static int trinoSessionQueryStart(String sql) {
+ int depth = 0;
+ int prefixWords = 0;
+ for (int i = 0; i < sql.length();) {
+ char c = sql.charAt(i);
+ if (c == '\'' || c == '"') {
+ char quote = c;
+ for (i++; i < sql.length(); i++) {
+ if (sql.charAt(i) == quote) {
+ if (i + 1 < sql.length() && sql.charAt(i + 1) ==
quote) {
+ i++;
+ } else {
+ i++;
+ break;
+ }
+ }
+ }
+ } else if (sql.startsWith("--", i)) {
+ int newline = sql.indexOf('\n', i + 2);
Review Comment:
[P2] End Trino `--` comments on CR in the new `WITH SESSION` scanner. Trino
permits a carriage return to terminate a line comment, but this branch searches
only for `\n`. For the valid root query `WITH SESSION time_zone='UTC' --
note\rSELECT ts FROM t`, it skips the `SELECT`, returns 0, and embeds `WITH
SESSION` inside `FROM (...)`, where Trino does not allow it. Use either CR or
LF as the comment boundary and cover a CR-only TVF source. See the [Trino 481
SQL
grammar](https://raw.githubusercontent.com/trinodb/trino/481/core/trino-grammar/src/main/antlr4/io/trino/grammar/sql/SqlBase.g4).
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/source/JdbcScanNode.java:
##########
@@ -213,13 +219,151 @@ 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++;
+ // 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 (c == '\'' || c == '"' || c == '`') {
+ char quote = c;
+ for (i++; i < sql.length(); i++) {
+ // ANSI_QUOTES makes double quotes identifiers, where
backslashes never escape the closing quote.
+ // Both quoting modes belong to the remote session.
+ if (sql.charAt(i) == '\\' && !(mysql && noBackslashEscapes)
+ && !(mysql && ansiQuotes && quote == '"') && quote
!= '`'
+ && jdbcType != TOdbcTableType.TRINO && jdbcType !=
TOdbcTableType.PRESTO) {
+ i++;
+ } else if (sql.charAt(i) == quote) {
+ if (i + 1 < sql.length() && sql.charAt(i + 1) ==
quote) {
+ i++;
+ } else {
+ i++;
+ break;
+ }
+ }
+ }
+ delimiters.clear();
+ } else {
+ if (c == ';') {
+ delimiters.add(i);
+ } else {
+ delimiters.clear();
+ }
+ i++;
+ }
+ }
+ // A statement delimiter may precede trailing comments, which must
stay outside SQL literals.
+ // Remove only the terminal delimiter run; interior delimiters still
fail as multi-statements.
+ if (delimiters.isEmpty()) {
+ return sql;
+ }
+ StringBuilder result = new StringBuilder(sql);
+ for (int i = delimiters.size() - 1; i >= 0; i--) {
+ result.deleteCharAt(delimiters.get(i));
}
- return "SELECT " + Joiner.on(", ").join(columns) + " FROM (" + source
+ ") doris_jdbc_source";
+ return result.toString();
}
+ private static int skipBlockComment(String sql, int start, boolean nested)
{
+ // Trino, Presto and MySQL end at the first closing marker; ClickHouse
permits nested comments.
+ int depth = 1;
+ int i = start + 2;
+ while (i < sql.length() && depth > 0) {
+ if (nested && sql.startsWith("/*", i)) {
+ depth++;
+ i += 2;
+ } else if (sql.startsWith("*/", i)) {
+ depth--;
+ i += 2;
+ } else {
+ i++;
+ }
+ }
+ return i;
+ }
+
+ private static int trinoSessionQueryStart(String sql) {
+ int depth = 0;
+ int prefixWords = 0;
+ for (int i = 0; i < sql.length();) {
+ char c = sql.charAt(i);
+ if (c == '\'' || c == '"') {
+ char quote = c;
+ for (i++; i < sql.length(); i++) {
+ if (sql.charAt(i) == quote) {
+ if (i + 1 < sql.length() && sql.charAt(i + 1) ==
quote) {
+ i++;
+ } else {
+ i++;
+ break;
+ }
+ }
+ }
+ } else if (sql.startsWith("--", i)) {
+ int newline = sql.indexOf('\n', i + 2);
+ i = newline < 0 ? sql.length() : newline + 1;
+ } else if (sql.startsWith("/*", i)) {
+ i = skipBlockComment(sql, i, false);
+ } else if (c == '(') {
+ depth++;
Review Comment:
[P2] Recognize a parenthesized query after Trino `WITH SESSION`. Trino's
grammar permits `WITH SESSION time_zone='UTC' (SELECT ts FROM t)`, but this `(`
raises `depth`, so the scanner ignores the inner `SELECT` and returns 0. The
timestamp TVF wrapper then puts `WITH SESSION` inside `FROM (...)`, where Trino
only permits a derived `query`. This form already failed in the old wrapper,
but the new `WITH SESSION` handling explicitly added here leaves this valid
query-primary form unsupported. Detect the parenthesized root query boundary
and add a TVF case. See the [Trino 481 SQL
grammar](https://raw.githubusercontent.com/trinodb/trino/481/core/trino-grammar/src/main/antlr4/io/trino/grammar/sql/SqlBase.g4).
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/source/JdbcScanNode.java:
##########
@@ -213,13 +219,151 @@ 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++;
+ // 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))))))
Review Comment:
[P2] Recognize ClickHouse `//` line comments before scanning quotes. A valid
timestamp TVF source ending with `SELECT ts FROM t // 'note` followed by a
newline and `;` has a quote only in the comment. This branch does not recognize
`//`, so the quote branch consumes the final semicolon and the wrapper leaves
`;` inside `FROM (...)`; the previous wrapper removed it. Handle ClickHouse
line-comment forms (`//`, `#!`, and `# `) and cover a comment containing an
unmatched quote before a terminal delimiter. See the [ClickHouse comment
syntax](https://clickhouse.com/docs/reference/syntax).
--
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]
