wangzhigang1999 opened a new issue, #7412: URL: https://github.com/apache/kyuubi/issues/7412
### Code of Conduct - [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct) ### Search before asking - [x] I have searched in the [issues](https://github.com/apache/kyuubi/issues?q=is%3Aissue) and found no similar issues. ### Describe the feature The `run_select_query` tool introduced in #7400 uses `SqlReadOnlyChecker`, which whitelists only the first significant token of a SQL statement as the read-only guard. This is bypassed by two legitimate Spark/Hive patterns, as pointed out by @wForget in #7400: ```sql -- Hive multi-insert (first token = FROM, whitelisted for the -- `FROM t SELECT ...` read variant) FROM t1 INSERT INTO t2 SELECT c1, c2; -- Spark/Hive CTE attached to INSERT (first token = WITH, whitelisted -- for SELECT CTEs) WITH tmp AS (SELECT * FROM t1) INSERT INTO t2 SELECT * FROM tmp; ``` Both execute writes despite passing the first-token check. This request tracks replacing the first-token heuristic with a real SQL read-only check so the tool's name matches its behavior for the configured dialects. ### Motivation `SqlReadOnlyChecker` is a guardrail against LLM misrouting, not a security boundary — writes are routed to the separate `run_mutation_query` tool at the prompt layer. In practice the two bypass patterns above are thin in LLM training data and rarely produced unsolicited; the realistic exposure is user-supplied SQL pasted into the conversation. That said, the contract is off: a tool named "read-only" should actually be read-only for the configured dialects, and reviewers reasonably expect that guarantee. ### Describe the solution Parse with Apache Calcite Babel (or others) + the matching `SqlDialect` (Spark, Hive, Trino, MySQL, SQLite) and reject if: - the root statement is not a query / DQL node - any CTE body contains a DML/DDL node - the statement is a multi-statement block containing any DML/DDL node ### Additional context - Parent umbrella: #7379 (Data Agent Engine). - Follow-up from #7400 (PR 2a of the data-agent series). - Calcite is already a transitive dependency in the wider Kyuubi tree via Spark/Hive, so the delta for the data-agent module is modest. - `Connection.setReadOnly(true)` was considered and rejected during #7400 review — Spark and Hive Thrift JDBC drivers throw or silently ignore it. ### Are you willing to submit PR? - [x] Yes. I would be willing to submit a PR with guidance from the Kyuubi community to improve. - [ ] No. I cannot submit a PR at this time. -- 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]
