jerry-024 commented on code in PR #309:
URL: https://github.com/apache/paimon-rust/pull/309#discussion_r3206099195
##########
crates/integrations/datafusion/src/sql_context.rs:
##########
@@ -190,8 +291,16 @@ impl SQLContext {
} else {
(sql.to_string(), vec![])
};
- let dialect = GenericDialect {};
- let statements = Parser::parse_sql(&dialect, &rewritten_sql)
+ let sql_lower = rewritten_sql.to_lowercase();
+ let has_time_travel =
+ sql_lower.contains("version as of") ||
sql_lower.contains("timestamp as of");
Review Comment:
**P0 — Must fix (false positive on string literals)**
This `contains()` check is the entry gate that short-circuits all subsequent
SQL parsing. Any SQL containing the substring `version as of` or `timestamp as
of` — even inside a string literal or comment — will skip `Parser::parse_sql`
entirely and enter the regex-based time-travel path.
Examples that would be misrouted:
```sql
SELECT * FROM t WHERE note = 'version as of 1'
-- comment: version as of something
INSERT INTO t VALUES ('timestamp as of now')
```
This needs at minimum a quoted-string-aware scan before deciding to treat
the query as time-travel. One approach: skip over single-quoted strings
(`'...'`) and SQL comments (`--` / `/* */`) before checking for the keyword.
--
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]
