Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle result sets for SHOW INDEX and SHOW CREATE TABLE commands #7

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wenjiachengy
Copy link

Problem

When executing SHOW INDEX FROM table_name or SHOW CREATE TABLE table_name through the MCP tool, it returns an error "Error executing query: Unread result found".

For example:

{
  "query": "SHOW INDEX FROM user_action;"
}
{
  "query": "SHOW CREATE TABLE user_action;"
}

@wenjiachengy wenjiachengy force-pushed the fix/handle-result-sets branch from 4893972 to 7a03c6e Compare February 27, 2025 15:29
@designcomputer
Copy link
Owner

Thank you for your contribution! The change to use cursor.description for handling queries that return result sets is a good idea for improving flexibility. However, there are a few potential downsides we should consider:

Ambiguity in Cursor State: If cursor.description is checked without ensuring that the cursor has executed a query that returns a result set, it could lead to unintended behavior. We need to ensure that cursor.description is appropriately set before accessing it.

Error Handling: There is no additional error handling to ensure that cursor.description is set. If a non-select query that doesn't return a result set is executed, cursor.description might not be set, which could lead to errors when trying to access it.

Performance: Depending on the database and the queries involved, checking cursor.description might have a performance impact if not handled properly, especially in cases where many different types of queries are executed frequently.

To mitigate these issues, I suggest adding checks to ensure cursor.description is set and handling cases where it might not be. Additionally, adding some error handling around this check would be beneficial.

Let me know your thoughts or if you need any help implementing these changes!

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