Skip to content

[fix] Use cursor.description to detect result-returning queries#75

Open
jonasbrami wants to merge 1 commit intoapache:masterfrom
jonasbrami:fix-cte-cursor-description
Open

[fix] Use cursor.description to detect result-returning queries#75
jonasbrami wants to merge 1 commit intoapache:masterfrom
jonasbrami:fix-cte-cursor-description

Conversation

@jonasbrami
Copy link
Copy Markdown

Replace brittle sql_upper.startswith() keyword check with cursor.description, which reliably detects any statement that returns a result set (SELECT, SHOW, DESCRIBE, EXPLAIN, CTEs, etc.) without maintaining a hardcoded list of SQL keywords.

Replace brittle sql_upper.startswith() keyword check with
cursor.description, which reliably detects any statement that
returns a result set (SELECT, SHOW, DESCRIBE, EXPLAIN, CTEs, etc.)
without maintaining a hardcoded list of SQL keywords.
@catpineapple
Copy link
Copy Markdown

Blocking: missing regression tests.

This classification branch has already caused Issue #62 Bug 5 once, and the cases motivating this PR (leading comments, parenthesized SELECT) aren't covered by tests either. Without tests, the same bug class will regress again.

Please add regression tests covering at least:

  • /* c */ SELECT 1 — leading SQL comment
  • WITH t AS (SELECT 1) SELECT * FROM t — CTE, guards Issue [some bugs] #62 Bug 5
  • SHOW TABLES / DESC <tbl> / EXPLAIN SELECT 1
  • INSERT / UPDATE / CREATE TABLE — locks in the else branch behavior

Copy link
Copy Markdown

@catpineapple catpineapple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution, but some additional test cases are needed to ensure the robustness of doris MCP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants