Skip to content

fix(readonly): Respect server readonly settings #35

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

Merged
merged 1 commit into from
Apr 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions mcp_clickhouse/mcp_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,7 @@ def _validate_required_vars(self) -> None:
missing_vars.append(var)

if missing_vars:
raise ValueError(
f"Missing required environment variables: {', '.join(missing_vars)}"
)
raise ValueError(f"Missing required environment variables: {', '.join(missing_vars)}")


# Global instance placeholder for the singleton pattern
Expand Down
52 changes: 45 additions & 7 deletions mcp_clickhouse/mcp_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ def list_tables(database: str, like: str = None):
result = client.command(query)

# Get all table comments in one query
table_comments_query = f"SELECT name, comment FROM system.tables WHERE database = {format_query_value(database)}"
table_comments_query = (
f"SELECT name, comment FROM system.tables WHERE database = {format_query_value(database)}"
)
table_comments_result = client.query(table_comments_query)
table_comments = {row[0]: row[1] for row in table_comments_result.result_rows}

Expand All @@ -82,14 +84,16 @@ def get_table_info(table):
for i, col_name in enumerate(column_names):
column_dict[col_name] = row[i]
# Add comment from our pre-fetched comments
if table in column_comments and column_dict['name'] in column_comments[table]:
column_dict['comment'] = column_comments[table][column_dict['name']]
if table in column_comments and column_dict["name"] in column_comments[table]:
column_dict["comment"] = column_comments[table][column_dict["name"]]
else:
column_dict['comment'] = None
column_dict["comment"] = None
columns.append(column_dict)

# Get row count and column count from the table
row_count_query = f"SELECT count() FROM {quote_identifier(database)}.{quote_identifier(table)}"
row_count_query = (
f"SELECT count() FROM {quote_identifier(database)}.{quote_identifier(table)}"
)
row_count_result = client.query(row_count_query)
row_count = row_count_result.result_rows[0][0] if row_count_result.result_rows else 0
column_count = len(columns)
Expand Down Expand Up @@ -125,7 +129,8 @@ def get_table_info(table):
def execute_query(query: str):
client = create_clickhouse_client()
try:
res = client.query(query, settings={"readonly": 1})
read_only = get_readonly_setting(client)
res = client.query(query, settings={"readonly": read_only})
column_names = res.column_names
rows = []
for row in res.result_rows:
Expand Down Expand Up @@ -161,7 +166,10 @@ def run_select_query(query: str):
logger.warning(f"Query timed out after {SELECT_QUERY_TIMEOUT_SECS} seconds: {query}")
future.cancel()
# Return a properly structured response for timeout errors
return {"status": "error", "message": f"Query timed out after {SELECT_QUERY_TIMEOUT_SECS} seconds"}
return {
"status": "error",
"message": f"Query timed out after {SELECT_QUERY_TIMEOUT_SECS} seconds",
}
except Exception as e:
logger.error(f"Unexpected error in run_select_query: {str(e)}")
# Catch all other exceptions and return them in a structured format
Expand All @@ -188,3 +196,33 @@ def create_clickhouse_client():
except Exception as e:
logger.error(f"Failed to connect to ClickHouse: {str(e)}")
raise


def get_readonly_setting(client) -> str:
"""Get the appropriate readonly setting value to use for queries.

This function handles potential conflicts between server and client readonly settings:
- readonly=0: No read-only restrictions
- readonly=1: Only read queries allowed, settings cannot be changed
- readonly=2: Only read queries allowed, settings can be changed (except readonly itself)

If server has readonly=2 and client tries to set readonly=1, it would cause:
"Setting readonly is unknown or readonly" error

This function preserves the server's readonly setting unless it's 0, in which case
we enforce readonly=1 to ensure queries are read-only.

Args:
client: ClickHouse client connection

Returns:
String value of readonly setting to use
"""
read_only = client.server_settings.get("readonly")
if read_only:
if read_only == "0":
return "1" # Force read-only mode if server has it disabled
else:
return read_only.value # Respect server's readonly setting (likely 2)
else:
return "1" # Default to basic read-only mode if setting isn't present
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "mcp-clickhouse"
version = "0.1.6"
version = "0.1.7"
description = "An MCP server for ClickHouse."
readme = "README.md"
license = "Apache-2.0"
Expand Down
2 changes: 1 addition & 1 deletion uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.