From 1d846243da932dc1113cecef56d33e49d4823bae Mon Sep 17 00:00:00 2001 From: shobhit-seg Date: Fri, 24 Jul 2026 22:30:27 +0530 Subject: [PATCH] Improve query and UI performance --- app.py | 4 + components/query_interface.py | 120 ++++++++------------------- components/table_browser_ui.py | 114 ++++++++++++++++++++++++-- data_explorer.py | 19 +---- database.py | 79 +++++++++++++++--- tests/test_database.py | 80 +++++++++++++++++- tests/test_explorer_performance.py | 29 +++++++ tests/test_query_interface.py | 106 ++++++++++++------------ tests/test_table_browser_ui.py | 125 +++++++++++++++++++++++++++++ utils.py | 15 ++++ 10 files changed, 520 insertions(+), 171 deletions(-) create mode 100644 tests/test_explorer_performance.py create mode 100644 tests/test_table_browser_ui.py diff --git a/app.py b/app.py index 9e168ed..bf5ab5d 100644 --- a/app.py +++ b/app.py @@ -123,6 +123,7 @@ def _handle_connect_success(self): self.credentials_ui.save_credentials_callback(None, None) # Automatically list tables after successful connection + self.table_browser_ui.invalidate_table_cache() current_search = UIHelpers.safe_get_value("table_search", "") self.table_browser_ui.filter_tables_callback(None, current_search) @@ -154,10 +155,13 @@ def run(self): # threads can safely update the UI without thread-safety issues. while is_dearpygui_running(): self.async_worker.process_pending() + self.table_browser_ui.process_pending_filter() self.ui_layout.splitter.update() self.tabbed_query_interface.update_resize() self.tabbed_query_interface.poll_closed_tabs() self.tabbed_explorer.poll_closed_tabs() render_dearpygui_frame() + self.connection_pool.close_all() + self.db_manager.disconnect() destroy_context() diff --git a/components/query_interface.py b/components/query_interface.py index f1abbef..bb868a9 100644 --- a/components/query_interface.py +++ b/components/query_interface.py @@ -14,6 +14,7 @@ from config import DEFAULT_TAB_LABEL, MAX_QUERY_TABS, QUERY_INPUT_HEIGHT from database import ConnectionPool, DatabaseManager from icon_manager import icon_manager +from utils import get_result_column_types MIN_QUERY_INPUT_HEIGHT = 80 MAX_QUERY_INPUT_HEIGHT = 600 @@ -33,7 +34,6 @@ class QueryTabState: tab_tag: str input_tag: str run_btn_tag: str - format_btn_tag: str save_btn_tag: str resizer_tag: str results_window_tag: str @@ -64,7 +64,7 @@ def __init__( async_worker=None, ): self.connection_pool = connection_pool - self.db_manager = db_manager # used only for get_table_columns() (metadata) + self.db_manager = db_manager self.theme_manager = theme_manager self.async_worker = async_worker self.status_callback: Callable[[str, bool], None] | None = None @@ -75,6 +75,7 @@ def __init__( self._resize_start_y: float = 0 self._resize_start_height: int = 0 self._resize_handlers_ready = False + self._themed_resizer_tab_id: int | None = None self.table_theme = None self._setup_table_theme() @@ -120,25 +121,34 @@ def update_resize(self): if not self._resize_handlers_ready: self._setup_resize_handlers() - # Highlight resizer while actively dragging - if self.theme_manager: - for state in self._tabs.values(): - if not does_item_exist(state.resizer_tag): - continue - is_active = self._resizing_tab_id == state.tab_id - bind_item_theme( - state.resizer_tag, - self.theme_manager.get_theme( - "resizer_active" if is_active else "resizer" - ), - ) + if ( + self._resizing_tab_id is not None + and self._resizing_tab_id not in self._tabs + ): + self._resizing_tab_id = None + + # Theme bindings are DearPyGUI commands. Only send them when drag state + # changes instead of rebinding every tab on every rendered frame. + if self.theme_manager and self._themed_resizer_tab_id != self._resizing_tab_id: + for tab_id in { + self._themed_resizer_tab_id, + self._resizing_tab_id, + } - {None}: + state = self._tabs.get(tab_id) + if state and does_item_exist(state.resizer_tag): + bind_item_theme( + state.resizer_tag, + self.theme_manager.get_theme( + "resizer_active" + if tab_id == self._resizing_tab_id + else "resizer" + ), + ) + self._themed_resizer_tab_id = self._resizing_tab_id if self._resizing_tab_id is None: return state = self._tabs.get(self._resizing_tab_id) - if state is None: - self._resizing_tab_id = None - return _, my = get_mouse_pos(local=False) delta = my - self._resize_start_y new_height = int(self._resize_start_height + delta) @@ -173,7 +183,6 @@ def create_tab(self) -> int: tab_tag=f"query_tab_{tab_id}", input_tag=f"query_input_{tab_id}", run_btn_tag=f"run_query_btn_{tab_id}", - format_btn_tag=f"format_btn_{tab_id}", save_btn_tag=f"save_json_btn_{tab_id}", resizer_tag=f"query_resizer_{tab_id}", results_window_tag=f"results_window_{tab_id}", @@ -228,17 +237,6 @@ def _build_tab_ui(self, state: QueryTabState): self.theme_manager.get_theme("button_primary"), ) - add_button( - label="{ } Format", - tag=state.format_btn_tag, - callback=self._make_format_callback(state.tab_id), - ) - if self.theme_manager: - bind_item_theme( - state.format_btn_tag, - self.theme_manager.get_theme("button_secondary"), - ) - add_button( label=f"{icon_manager.get('export')} Save as JSON", tag=state.save_btn_tag, @@ -381,38 +379,12 @@ def callback(sender, data): return callback - def _make_format_callback(self, tab_id: int): - def callback(sender, data): - self._format_query_for_tab(tab_id) - - return callback - def _make_save_callback(self, tab_id: int): def callback(sender, data): self._save_as_json_for_tab(tab_id) return callback - def _format_query_for_tab(self, tab_id: int): - state = self._tabs.get(tab_id) - if state is None: - return - query = get_value(state.input_tag).strip() - if not query: - if self.status_callback: - self.status_callback("Query is empty", True) - return - formatted = sqlparse.format( - query, - reindent=True, - keyword_case="upper", - indent_width=2, - wrap_after=80, - ) - set_value(state.input_tag, formatted) - if self.status_callback: - self.status_callback("Query formatted", False) - def poll_closed_tabs(self): """Called each frame to detect tabs closed via the DPG X button.""" closed_ids = [ @@ -489,10 +461,9 @@ def _run_query_for_tab(self, tab_id: int): self._on_query_error(tab_id, e) def _execute_query_task(self, tab_id: int, query: str): - """Background thread: execute query and fetch column types.""" + """Background thread: execute a query and reuse its returned metadata.""" result = self.connection_pool.execute_query(tab_id, query) - column_types = self._get_column_types_from_query(query, result.column_names) - return result, column_types + return result, get_result_column_types(result) def _on_query_done(self, tab_id: int, payload): """Main-thread callback when query completes successfully.""" @@ -605,6 +576,7 @@ def _setup_results_table(self, state: QueryTabState, columns, column_types=None) scrollX=True, scrollY=True, freeze_rows=1, + clipper=True, height=-1, width=-1, resizable=True, @@ -794,28 +766,6 @@ def _convert_results_to_json(self, state: QueryTabState): # Helpers (shared with QueryInterface logic) # ------------------------------------------------------------------ - def _get_column_types_from_query(self, query, columns): - """Fetch column types from DB metadata. Call from background thread only.""" - column_types = {} - try: - query_lower = query.lower().strip() - from_match = re.search(r"\bfrom\s+([a-zA-Z_][a-zA-Z0-9_]*)", query_lower) - if from_match: - table_name = from_match.group(1) - try: - table_columns = self.db_manager.get_table_columns(table_name) - table_column_types = { - col_name: col_type for col_name, col_type in table_columns - } - for col in columns: - if col in table_column_types: - column_types[col] = table_column_types[col] - except Exception: - pass - except Exception: - pass - return column_types - @staticmethod def _format_cell_value(val) -> str: if val is None: @@ -826,19 +776,17 @@ def _format_cell_value(val) -> str: except Exception: return str(val) elif isinstance(val, str): - try: - return val.encode("utf-8", errors="replace").decode("utf-8") - except Exception: - return str(val) + return val return str(val) @staticmethod def _add_default_limit(query: str) -> str: try: query_clean = query.strip() - query_lower = query_clean.lower() - if not query_lower.startswith("select"): + statements = sqlparse.parse(query_clean) + if len(statements) != 1 or statements[0].get_type() != "SELECT": return query + query_lower = query_clean.lower() limit_patterns = [ r"\blimit\s+\d+\b", r"\blimit\s+\d+\s+offset\s+\d+\b", diff --git a/components/table_browser_ui.py b/components/table_browser_ui.py index 43b884b..4bd23a5 100644 --- a/components/table_browser_ui.py +++ b/components/table_browser_ui.py @@ -7,6 +7,9 @@ from utils import UIHelpers +TABLE_CACHE_TTL_SECONDS = 5.0 +TABLE_SEARCH_DEBOUNCE_SECONDS = 0.2 + class TableBrowserUI: """Manages table browsing and filtering UI functionality.""" @@ -37,6 +40,11 @@ def __init__( # Sequence number to discard stale async table-list results self._tables_seq = 0 + self._table_cache: tuple[str, ...] | None = None + self._table_cache_key: tuple | None = None + self._table_cache_created_at = 0.0 + self._pending_table_search: str | None = None + self._table_search_due_at = 0.0 # Explicitly tracked active connection name (set when connecting to a saved connection) self.active_connection_name: str = "" @@ -45,8 +53,50 @@ def set_double_click_callback(self, callback: Callable[[str], None]): """Set callback for table single-click events to open explorer.""" self.double_click_callback = callback + def invalidate_table_cache(self): + """Force the next expanded table view to refresh from ClickHouse.""" + self._table_cache = None + self._table_cache_key = None + self._table_cache_created_at = 0.0 + + def _get_table_cache_key(self) -> tuple: + """Identify the active database without retaining credentials.""" + connection_info = self.db_manager.connection_info + return ( + connection_info.get("host", ""), + str(connection_info.get("port", "")), + connection_info.get("username", ""), + connection_info.get("database", ""), + ) + def filter_tables_callback(self, sender, app_data): - """Filter tables in the left panel based on the search query (non-blocking).""" + """Debounce user typing while keeping programmatic refreshes immediate.""" + search_query = app_data or "" + if sender is not None: + self._pending_table_search = search_query + self._table_search_due_at = time.monotonic() + TABLE_SEARCH_DEBOUNCE_SECONDS + return + + self._pending_table_search = None + self._apply_table_filter(None, search_query) + + def process_pending_filter(self, now: float | None = None) -> bool: + """Apply the latest search once its debounce window has elapsed.""" + if self._pending_table_search is None: + return False + + if now is None: + now = time.monotonic() + if now < self._table_search_due_at: + return False + + search_query = self._pending_table_search + self._pending_table_search = None + self._apply_table_filter("table_search", search_query) + return True + + def _apply_table_filter(self, sender, app_data): + """Filter tables in the left panel using cached metadata when possible.""" preserve_scroll = sender is not None scroll_y = 0 if preserve_scroll: @@ -69,19 +119,38 @@ def filter_tables_callback(self, sender, app_data): self.show_saved_connections() return - # Show a loading placeholder while fetching tables in background - add_text(" Loading tables...", parent="tables_list", color=(100, 150, 255)) - - # Bump sequence so a stale async result from a previous call is ignored + # Every new filter supersedes any older async result. self._tables_seq += 1 seq = self._tables_seq + cache_key = self._get_table_cache_key() + + cache_is_fresh = ( + self._table_cache_key == cache_key + and self._table_cache is not None + and time.monotonic() - self._table_cache_created_at + < TABLE_CACHE_TTL_SECONDS + ) + if cache_is_fresh: + self._finish_filter_tables( + self._table_cache, + seq, + search_query, + connection_name, + preserve_scroll, + scroll_y, + ) + return + + # Only the initial load (or an explicit invalidation) reaches ClickHouse. + add_text(" Loading tables...", parent="tables_list", color=(100, 150, 255)) if self.async_worker: self.async_worker.run_async( task=lambda: self.db_manager.get_tables(), - on_done=lambda tables: self._finish_filter_tables( + on_done=lambda tables: self._on_tables_loaded( tables, seq, + cache_key, search_query, connection_name, preserve_scroll, @@ -92,15 +161,46 @@ def filter_tables_callback(self, sender, app_data): else: # Synchronous fallback all_tables = self.db_manager.get_tables() - self._finish_filter_tables( + self._on_tables_loaded( all_tables, seq, + cache_key, search_query, connection_name, preserve_scroll, scroll_y, ) + def _on_tables_loaded( + self, + all_tables, + seq, + cache_key, + search_query, + connection_name, + preserve_scroll, + scroll_y, + ): + """Cache a current table response, then render its latest filter.""" + if ( + seq != self._tables_seq + or not self.db_manager.is_connected + or cache_key != self._get_table_cache_key() + ): + return + + self._table_cache = tuple(all_tables) + self._table_cache_key = cache_key + self._table_cache_created_at = time.monotonic() + self._finish_filter_tables( + self._table_cache, + seq, + search_query, + connection_name, + preserve_scroll, + scroll_y, + ) + def _finish_filter_tables( self, all_tables, seq, search_query, connection_name, preserve_scroll, scroll_y ): diff --git a/data_explorer.py b/data_explorer.py index 20f764b..181e6cc 100644 --- a/data_explorer.py +++ b/data_explorer.py @@ -8,6 +8,7 @@ from config import DEFAULT_LIMIT, MAX_CELL_LENGTH, MAX_EXPLORER_TABS from database import DatabaseManager +from utils import get_result_column_types class DataExplorer: @@ -254,14 +255,9 @@ def refresh_data(self, status_callback=None): # ------------------------------------------------------------------ def _fetch_data_task(self, query: str, table_name: str): - """Run in background thread: execute query and fetch column types.""" + """Run in background thread and reuse metadata returned with the query.""" result = self.db_manager.execute_query(query) - try: - table_columns = self.db_manager.get_table_columns(table_name) - column_types = {col_name: col_type for col_name, col_type in table_columns} - except Exception: - column_types = {} - return result, column_types + return result, get_result_column_types(result) # ------------------------------------------------------------------ # Main-thread callbacks (called via AsyncWorker queue) @@ -332,13 +328,6 @@ def _on_data_ready(self, payload, seq: int, status_callback=None): width_fixed=False, no_resize=False, ) - try: - configure_item(column_tag, width=200) - except Exception: - try: - set_item_width(column_tag, 200) - except Exception: - pass # Force widths after all columns created for column_tag in column_tags: @@ -433,7 +422,7 @@ def _format_cell_value(self, val) -> str: elif isinstance(val, bytes): cell_value = val.decode("utf-8", errors="replace") elif isinstance(val, str): - cell_value = val.encode("utf-8", errors="replace").decode("utf-8") + cell_value = val else: cell_value = str(val) diff --git a/database.py b/database.py index 63e3f69..a1bc27a 100644 --- a/database.py +++ b/database.py @@ -56,6 +56,8 @@ def connect( tuple: (success: bool, message: str) """ with self._lock: + new_client = None + previous_client = None try: # Validate inputs if not all([host, str(port), username, database]): @@ -68,7 +70,8 @@ def connect( return False, f"Port must be a number, got: {port}" # Create client - self.client = clickhouse_connect.get_client( + previous_client = self.client + new_client = clickhouse_connect.get_client( host=host, port=port, username=username, @@ -81,9 +84,10 @@ def connect( ) # Test connection - self.client.query("SELECT 1") + new_client.query("SELECT 1") # Store connection info + self.client = new_client self.connection_info = { "host": host, "port": port, @@ -91,12 +95,18 @@ def connect( "database": database, } self.is_connected = True + if previous_client is not None and previous_client is not new_client: + self._close_client(previous_client) return True, f"Connected successfully to {host}:{port}" except Exception as e: + self._close_client(new_client) + if previous_client is not new_client: + self._close_client(previous_client) self.client = None self.is_connected = False + self.connection_info = {} error_msg = "Connection failed:\n" error_msg += f"Error type: {type(e).__name__}\n" error_msg += f"Error message: {str(e)}\n" @@ -131,6 +141,7 @@ def test_credentials( tuple: (success: bool, message: str) """ with self._lock: + test_client = None try: # Validate inputs if not all([host, str(port), username, database]): @@ -158,9 +169,6 @@ def test_credentials( # Test connection test_client.query("SELECT 1") - # Close test client (don't store it) - test_client = None - return True, f"Credentials are valid for {host}:{port}" except Exception as e: @@ -169,6 +177,8 @@ def test_credentials( error_msg += f"Error message: {str(e)}\n" error_msg += f"Details:\n{traceback.format_exc()}" return False, error_msg + finally: + self._close_client(test_client) def disconnect(self) -> str: """ @@ -177,13 +187,26 @@ def disconnect(self) -> str: Returns: str: Status message """ - if self.client: + with self._lock: + client = self.client + if not client: + return "Not connected to database" self.client = None self.is_connected = False self.connection_info = {} - return "Disconnected from database" - else: - return "Not connected to database" + + self._close_client(client) + return "Disconnected from database" + + @staticmethod + def _close_client(client) -> None: + """Close a ClickHouse client without masking the caller's result.""" + if client is None: + return + try: + client.close() + except Exception: + pass def execute_query(self, query: str): """ @@ -245,10 +268,21 @@ def __init__(self): self._lock = threading.Lock() def configure(self, credentials: dict | None) -> None: - """Store (or clear) the credentials used to create new clients.""" + """Store credentials and close clients tied to a different connection.""" + credentials = dict(credentials) if credentials is not None else None with self._lock: + if credentials == self._credentials: + return + clients = list(self._clients.values()) + self._clients.clear() self._credentials = credentials + for client in clients: + try: + client.close() + except Exception: + pass + @property def is_configured(self) -> bool: return self._credentials is not None @@ -277,9 +311,18 @@ def get_or_create_client(self, tab_id: int): ) with self._lock: - if tab_id not in self._clients: + existing_client = self._clients.get(tab_id) + if existing_client is None: self._clients[tab_id] = client - return self._clients[tab_id] + return client + + # Another worker won the creation race for this tab. Keep the cached + # connection and release the redundant HTTP client immediately. + try: + client.close() + except Exception: + pass + return existing_client def execute_query(self, tab_id: int, query: str): """Execute a query, refreshing once if its connection reports unknown table.""" @@ -309,6 +352,18 @@ def release(self, tab_id: int) -> None: except Exception: pass + def close_all(self) -> None: + """Close every per-tab client.""" + with self._lock: + clients = list(self._clients.values()) + self._clients.clear() + + for client in clients: + try: + client.close() + except Exception: + pass + @staticmethod def _is_unknown_table_error(error: Exception) -> bool: """Return whether ClickHouse reported UNKNOWN_TABLE (error code 60).""" diff --git a/tests/test_database.py b/tests/test_database.py index 47a73da..7f742c4 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -256,11 +256,25 @@ def test_connect_test_query_exception(self, mock_clickhouse_connect): self.assertIn("Query failed", message) self.assertFalse(self.db_manager.is_connected) self.assertIsNone(self.db_manager.client) + mock_client.close.assert_called_once_with() + + @patch.object(database, "clickhouse_connect") + def test_credentials_closes_temporary_client(self, mock_clickhouse_connect): + mock_client = Mock() + mock_clickhouse_connect.get_client.return_value = mock_client + + success, _ = self.db_manager.test_credentials( + "localhost", 8123, "default", "password", "default" + ) + + self.assertTrue(success) + mock_client.close.assert_called_once_with() def test_disconnect_when_connected(self): """Test disconnecting when there is an active connection.""" # Arrange - self.db_manager.client = Mock() + mock_client = Mock() + self.db_manager.client = mock_client self.db_manager.is_connected = True self.db_manager.connection_info = {"host": "localhost"} @@ -272,6 +286,7 @@ def test_disconnect_when_connected(self): self.assertIsNone(self.db_manager.client) self.assertFalse(self.db_manager.is_connected) self.assertEqual(self.db_manager.connection_info, {}) + mock_client.close.assert_called_once_with() def test_disconnect_when_not_connected(self): """Test disconnecting when not connected.""" @@ -492,6 +507,69 @@ def test_release_closes_and_removes_client(self, mock_clickhouse_connect): client.close.assert_called_once_with() self.assertNotIn(5, self.pool._clients) + @patch.object(database, "clickhouse_connect") + def test_reconfigure_closes_clients_from_previous_connection( + self, mock_clickhouse_connect + ): + client = Mock() + mock_clickhouse_connect.get_client.return_value = client + self.pool.get_or_create_client(5) + + self.pool.configure( + { + "host": "other-host", + "port": 8123, + "username": "default", + "password": "password", + "database": "default", + } + ) + + client.close.assert_called_once_with() + self.assertEqual(self.pool._clients, {}) + + @patch.object(database, "clickhouse_connect") + def test_same_configuration_keeps_existing_clients(self, mock_clickhouse_connect): + client = Mock() + mock_clickhouse_connect.get_client.return_value = client + self.pool.get_or_create_client(5) + + self.pool.configure(dict(self.pool._credentials)) + + client.close.assert_not_called() + self.assertIs(self.pool._clients[5], client) + + @patch.object(database, "clickhouse_connect") + def test_client_creation_race_closes_redundant_client( + self, mock_clickhouse_connect + ): + cached_client = Mock() + redundant_client = Mock() + + def create_client(**_kwargs): + self.pool._clients[5] = cached_client + return redundant_client + + mock_clickhouse_connect.get_client.side_effect = create_client + + result = self.pool.get_or_create_client(5) + + self.assertIs(result, cached_client) + redundant_client.close.assert_called_once_with() + + @patch.object(database, "clickhouse_connect") + def test_close_all_closes_every_client(self, mock_clickhouse_connect): + clients = [Mock(), Mock()] + mock_clickhouse_connect.get_client.side_effect = clients + self.pool.get_or_create_client(1) + self.pool.get_or_create_client(2) + + self.pool.close_all() + + for client in clients: + client.close.assert_called_once_with() + self.assertEqual(self.pool._clients, {}) + class TestPasswordFunctions(unittest.TestCase): """Test cases for password encryption/decryption functions.""" diff --git a/tests/test_explorer_performance.py b/tests/test_explorer_performance.py new file mode 100644 index 0000000..dfa9d79 --- /dev/null +++ b/tests/test_explorer_performance.py @@ -0,0 +1,29 @@ +"""Performance-focused tests for the data explorer.""" + +from types import SimpleNamespace +from unittest.mock import MagicMock + +from data_explorer import DataExplorer + + +def test_fetch_data_reuses_query_result_metadata(): + db_manager = MagicMock() + result = SimpleNamespace( + column_names=("id", "payload"), + column_types=( + SimpleNamespace(name="UInt64"), + SimpleNamespace(name="Nullable(String)"), + ), + ) + db_manager.execute_query.return_value = result + explorer = DataExplorer.__new__(DataExplorer) + explorer.db_manager = db_manager + + payload = explorer._fetch_data_task("SELECT * FROM events", "events") + + assert payload == ( + result, + {"id": "UInt64", "payload": "Nullable(String)"}, + ) + db_manager.execute_query.assert_called_once_with("SELECT * FROM events") + db_manager.get_table_columns.assert_not_called() diff --git a/tests/test_query_interface.py b/tests/test_query_interface.py index 2e78ef2..3b87445 100644 --- a/tests/test_query_interface.py +++ b/tests/test_query_interface.py @@ -36,6 +36,7 @@ qi_mod.DEFAULT_TAB_LABEL = "Query" qi_mod.MAX_QUERY_TABS = 10 qi_mod.QUERY_INPUT_HEIGHT = 150 +qi_mod.mvTable_SizingFixedFit = 0 # Alias for patch with create=True (star-import names don't exist on the module) _P = "components.query_interface" @@ -71,7 +72,6 @@ def _make_state(tab_id=0, label="Query 1"): tab_tag=f"query_tab_{tab_id}", input_tag=f"query_input_{tab_id}", run_btn_tag=f"run_query_btn_{tab_id}", - format_btn_tag=f"format_btn_{tab_id}", save_btn_tag=f"save_json_btn_{tab_id}", resizer_tag=f"query_resizer_{tab_id}", results_window_tag=f"results_window_{tab_id}", @@ -390,54 +390,6 @@ def test_cancel_callback_delegates(self): mock.assert_called_once_with(3) -class TestFormatQuery(unittest.TestCase): - """Test _format_query_for_tab formats SQL via sqlparse.""" - - @_patch("set_value") - @_patch("get_value", return_value="select id, name from users where id > 1") - def test_formats_sql(self, _get, mock_set): - iface = _make_interface() - status = MagicMock() - iface.status_callback = status - state = _make_state(tab_id=0) - iface._tabs[0] = state - - iface._format_query_for_tab(0) - - # set_value called with formatted SQL (keywords uppercased, reindented) - mock_set.assert_called_once() - formatted = mock_set.call_args[0][1] - self.assertIn("SELECT", formatted) - self.assertIn("FROM", formatted) - self.assertIn("WHERE", formatted) - status.assert_called_once_with("Query formatted", False) - - @_patch("set_value") - @_patch("get_value", return_value=" ") - def test_empty_query_shows_error(self, _get, mock_set): - iface = _make_interface() - status = MagicMock() - iface.status_callback = status - state = _make_state(tab_id=0) - iface._tabs[0] = state - - iface._format_query_for_tab(0) - - mock_set.assert_not_called() - status.assert_called_once_with("Query is empty", True) - - def test_noop_when_tab_missing(self): - iface = _make_interface() - iface._format_query_for_tab(999) - - def test_make_format_callback_delegates(self): - iface = _make_interface() - with patch.object(iface, "_format_query_for_tab") as mock: - cb = iface._make_format_callback(7) - cb("s", "d") - mock.assert_called_once_with(7) - - class TestResizeMouseDown(unittest.TestCase): """Test _on_resize_mouse_down sets drag state when hovering resizer.""" @@ -559,7 +511,9 @@ def test_noop_when_not_dragging(self, _exists, mock_bind): iface._resize_handlers_ready = True iface.update_resize() - # bind_item_theme called for highlight logic, but no configure_item + iface.update_resize() + + mock_bind.assert_not_called() def test_clears_state_when_tab_gone(self): iface = _make_interface(theme_manager=None) @@ -571,5 +525,57 @@ def test_clears_state_when_tab_gone(self): self.assertIsNone(iface._resizing_tab_id) +class TestQueryPerformanceHelpers(unittest.TestCase): + """Regression coverage for query request and result-size optimizations.""" + + def test_execute_query_reuses_result_column_types(self): + iface = _make_interface() + uint_type = MagicMock() + uint_type.name = "UInt64" + string_type = MagicMock() + string_type.name = "String" + result = MagicMock( + column_names=("id", "name"), + column_types=(uint_type, string_type), + ) + iface.connection_pool.execute_query.return_value = result + + payload = iface._execute_query_task(3, "SELECT id, name FROM events") + + self.assertIs(payload[0], result) + self.assertEqual(payload[1], {"id": "UInt64", "name": "String"}) + iface.connection_pool.execute_query.assert_called_once_with( + 3, "SELECT id, name FROM events" + ) + iface.db_manager.get_table_columns.assert_not_called() + + def test_default_limit_applies_to_cte_select(self): + query = "WITH recent AS (SELECT * FROM events) SELECT * FROM recent" + + limited = TabbedQueryInterface._add_default_limit(query) + + self.assertEqual(limited, f"{query} LIMIT 100") + + def test_default_limit_does_not_modify_non_select(self): + query = "INSERT INTO events VALUES (1)" + + self.assertEqual(TabbedQueryInterface._add_default_limit(query), query) + + @_patch("set_item_width") + @_patch("configure_item") + @_patch("add_table_column") + @_patch("bind_item_theme") + @_patch("add_table") + def test_results_table_enables_row_clipping( + self, add_table, _bind_theme, _add_column, _configure, _set_width + ): + iface = _make_interface() + state = _make_state() + + iface._setup_results_table(state, ["id"]) + + self.assertTrue(add_table.call_args.kwargs["clipper"]) + + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/tests/test_table_browser_ui.py b/tests/test_table_browser_ui.py new file mode 100644 index 0000000..d89dd5b --- /dev/null +++ b/tests/test_table_browser_ui.py @@ -0,0 +1,125 @@ +"""Performance-focused tests for table-list filtering.""" + +from unittest.mock import MagicMock, patch + +from components.table_browser_ui import ( + TABLE_CACHE_TTL_SECONDS, + TABLE_SEARCH_DEBOUNCE_SECONDS, + TableBrowserUI, +) + + +def _make_browser(): + db_manager = MagicMock() + db_manager.is_connected = True + db_manager.connection_info = { + "host": "clickhouse.example", + "port": 8123, + "username": "default", + "database": "analytics", + } + db_manager.get_tables.return_value = ["events", "event_daily", "users"] + browser = TableBrowserUI(db_manager, MagicMock()) + browser.active_connection_name = "Analytics" + browser.connections_expanded.add("current") + return browser, db_manager + + +@patch("components.table_browser_ui.add_text") +@patch("components.table_browser_ui.delete_item") +@patch("components.table_browser_ui.get_y_scroll", return_value=0) +def test_programmatic_filters_use_cached_tables_without_new_queries( + _get_scroll, _delete_item, _add_text +): + browser, db_manager = _make_browser() + + with patch.object(browser, "_finish_filter_tables") as finish: + browser.filter_tables_callback(None, "e") + browser.filter_tables_callback(None, "ev") + browser.filter_tables_callback(None, "eve") + + assert db_manager.get_tables.call_count == 1 + assert finish.call_count == 3 + assert finish.call_args.args[0] == ("events", "event_daily", "users") + assert finish.call_args.args[2] == "eve" + + +@patch("components.table_browser_ui.add_text") +@patch("components.table_browser_ui.delete_item") +@patch("components.table_browser_ui.get_y_scroll", return_value=0) +def test_search_debounces_rapid_keystrokes(_get_scroll, _delete_item, _add_text): + browser, db_manager = _make_browser() + + with ( + patch.object(browser, "_finish_filter_tables") as finish, + patch("components.table_browser_ui.time.monotonic", return_value=100.0), + ): + browser.filter_tables_callback("table_search", "e") + browser.filter_tables_callback("table_search", "ev") + browser.filter_tables_callback("table_search", "eve") + + assert not browser.process_pending_filter( + now=100.0 + TABLE_SEARCH_DEBOUNCE_SECONDS - 0.001 + ) + assert browser.process_pending_filter(now=100.0 + TABLE_SEARCH_DEBOUNCE_SECONDS) + + db_manager.get_tables.assert_called_once_with() + finish.assert_called_once() + assert finish.call_args.args[2] == "eve" + + +@patch("components.table_browser_ui.add_text") +@patch("components.table_browser_ui.delete_item") +@patch("components.table_browser_ui.get_y_scroll", return_value=0) +def test_programmatic_refresh_cancels_pending_search( + _get_scroll, _delete_item, _add_text +): + browser, db_manager = _make_browser() + + with ( + patch.object(browser, "_finish_filter_tables") as finish, + patch("components.table_browser_ui.time.monotonic", return_value=100.0), + ): + browser.filter_tables_callback("table_search", "stale") + browser.filter_tables_callback(None, "current") + + assert not browser.process_pending_filter(now=200.0) + + db_manager.get_tables.assert_called_once_with() + finish.assert_called_once() + assert finish.call_args.args[2] == "current" + + +@patch("components.table_browser_ui.add_text") +@patch("components.table_browser_ui.delete_item") +@patch("components.table_browser_ui.get_y_scroll", return_value=0) +def test_invalidating_cache_refreshes_tables(_get_scroll, _delete_item, _add_text): + browser, db_manager = _make_browser() + + with patch.object(browser, "_finish_filter_tables"): + browser.filter_tables_callback(None, "") + browser.invalidate_table_cache() + browser.filter_tables_callback(None, "") + + assert db_manager.get_tables.call_count == 2 + + +@patch("components.table_browser_ui.add_text") +@patch("components.table_browser_ui.delete_item") +@patch("components.table_browser_ui.get_y_scroll", return_value=0) +def test_expired_cache_refreshes_tables(_get_scroll, _delete_item, _add_text): + browser, db_manager = _make_browser() + + with ( + patch.object(browser, "_finish_filter_tables"), + patch("components.table_browser_ui.time.monotonic") as monotonic, + ): + monotonic.side_effect = [ + 10.0, + 10.0 + TABLE_CACHE_TTL_SECONDS + 1, + 16.0, + ] + browser.filter_tables_callback(None, "") + browser.filter_tables_callback(None, "events") + + assert db_manager.get_tables.call_count == 2 diff --git a/utils.py b/utils.py index 1445367..b3f2fc1 100644 --- a/utils.py +++ b/utils.py @@ -171,3 +171,18 @@ def validate_connection_params( return False, f"Port must be a number, got: {port}" return True, "" + + +def get_result_column_types(result) -> dict[str, str]: + """Return ClickHouse column type names already included in a query result.""" + column_names = getattr(result, "column_names", ()) or () + column_types = getattr(result, "column_types", ()) or () + resolved_types = {} + + for column_name, column_type in zip(column_names, column_types, strict=False): + type_name = getattr(column_type, "name", None) + if not isinstance(type_name, str): + type_name = str(column_type) + resolved_types[str(column_name)] = type_name + + return resolved_types