Skip to content

Commit b4f56fe

Browse files
authored
#223: Made profile_exasol_query side-effect free. (#224)
1 parent fd6b4bd commit b4f56fe

3 files changed

Lines changed: 66 additions & 7 deletions

File tree

doc/changes/unreleased.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Unreleased
22

3+
## Bug Fixes
4+
5+
* #223: Fixed `profile_exasol_query` to be side-effect free: the tool now checks whether
6+
session profiling is already enabled via `SYS.EXA_PARAMETERS` and skips the
7+
`ALTER SESSION SET PROFILE` statements if it was already `ON`.
8+
39
## Features
410

511
* #217: Added `summarize_exasol_table` tool that returns per-column statistics

exasol/ai/mcp/server/tools/mcp_server.py

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,15 @@ def _build_list_preprocessors_query() -> str:
350350
)
351351

352352

353+
def _build_profile_status_query() -> str:
354+
return (
355+
exp.select(exp.column("SESSION_VALUE"))
356+
.from_(exp.Table(this=exp.Identifier(this="EXA_PARAMETERS"), db=_SYS))
357+
.where(exp.column("PARAMETER_NAME").eq("PROFILE"))
358+
.sql(dialect="exasol", identify=True)
359+
)
360+
361+
353362
def _build_current_preprocessor_query() -> str:
354363
return (
355364
exp.select(exp.column("SESSION_VALUE"))
@@ -639,13 +648,17 @@ def profile_query(self, query: QueryArg) -> list[dict[str, Any]]:
639648
raise RuntimeError("Query profiling is disabled.")
640649
if not verify_query(query):
641650
raise ValueError("The query is invalid or not a SELECT statement.")
642-
statements = [
643-
"ALTER SESSION SET PROFILE = 'ON'",
644-
query,
645-
"FLUSH STATISTICS",
646-
"ALTER SESSION SET PROFILE = 'OFF'",
647-
_build_profile_select(query),
648-
]
651+
profile_already_on = (
652+
self.connection.execute_query(_build_profile_status_query()).fetchval()
653+
== "ON"
654+
)
655+
statements: list[str] = []
656+
if not profile_already_on:
657+
statements.append("ALTER SESSION SET PROFILE = 'ON'")
658+
statements.extend([query, "FLUSH STATISTICS"])
659+
if not profile_already_on:
660+
statements.append("ALTER SESSION SET PROFILE = 'OFF'")
661+
statements.append(_build_profile_select(query))
649662
return self.connection.execute_query(statements, snapshot=False).fetchall()
650663

651664
async def execute_write_query(self, query: QueryArg, ctx: Context) -> str | None:

test/unit/tools/test_mcp_server.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
_build_list_preprocessors_query,
1313
_build_preview_query,
1414
_build_profile_select,
15+
_build_profile_status_query,
1516
_build_set_preprocessor_query,
1617
_build_stats_query,
1718
_build_top_values_query,
@@ -387,6 +388,45 @@ def test_build_profile_select():
387388
assert sql == expected
388389

389390

391+
def test_build_profile_status_query():
392+
sql = collapse_spaces(_build_profile_status_query())
393+
expected = collapse_spaces("""
394+
SELECT "SESSION_VALUE"
395+
FROM "SYS"."EXA_PARAMETERS"
396+
WHERE "PARAMETER_NAME" = 'PROFILE'
397+
""")
398+
assert sql == expected
399+
400+
401+
def _make_profile_server(profile_already_on: bool):
402+
connection = MagicMock()
403+
connection.execute_query.return_value.fetchval.return_value = (
404+
"ON" if profile_already_on else "OFF"
405+
)
406+
connection.execute_query.return_value.fetchall.return_value = []
407+
config = MagicMock()
408+
config.enable_query_profiling = True
409+
server = ExasolMCPServer(connection=connection, config=config)
410+
return server, connection
411+
412+
413+
def test_profile_query_enables_and_disables_profiling_when_off():
414+
server, connection = _make_profile_server(profile_already_on=False)
415+
server.profile_query("SELECT 1")
416+
calls = [str(c) for c in connection.execute_query.call_args_list]
417+
statements = connection.execute_query.call_args_list[-1][0][0]
418+
assert statements[0] == "ALTER SESSION SET PROFILE = 'ON'"
419+
assert statements[-2] == "ALTER SESSION SET PROFILE = 'OFF'"
420+
421+
422+
def test_profile_query_skips_profile_toggle_when_already_on():
423+
server, connection = _make_profile_server(profile_already_on=True)
424+
server.profile_query("SELECT 1")
425+
statements = connection.execute_query.call_args_list[-1][0][0]
426+
assert "ALTER SESSION SET PROFILE = 'ON'" not in statements
427+
assert "ALTER SESSION SET PROFILE = 'OFF'" not in statements
428+
429+
390430
def test_build_list_preprocessors_query():
391431
sql = collapse_spaces(_build_list_preprocessors_query())
392432
expected = collapse_spaces("""

0 commit comments

Comments
 (0)