feat: Enhance caching mechanism with session-aware queries and improved logging

Krishnachaitanyakc · Krishnachaitanyakc · commit eece936a3269 · 2025-07-26T23:26:02.000-07:00
diff --git a/src/main/cache/CacheManager.ts b/src/main/cache/CacheManager.ts
@@ -27,14 +27,32 @@ export class CacheManager {
   async getCachedResult(
     query: string,
     connectionId: string,
-    database?: string
+    database?: string,
+    sessionId?: string
   ): Promise<QueryResult | null> {
     if (!this.enabled || !QueryNormalizer.shouldCache(query)) {
       return null
     }
 
-    const cacheKey = QueryNormalizer.generateCacheKey(query, connectionId, database)
-    return await this.queryCache.get(cacheKey)
+    const cacheKey = QueryNormalizer.generateCacheKey(query, connectionId, database, sessionId)
+    const isSessionIndependent = QueryNormalizer.isSessionIndependentQuery(query)
+
+    const result = await this.queryCache.get(cacheKey)
+
+    // Enhanced logging for cache strategy monitoring
+    if (result) {
+      console.log(
+        `[CACHE HIT] ${isSessionIndependent ? 'Session-Independent' : 'Session-Dependent'} query cache hit`,
+        {
+          queryType: QueryNormalizer.normalize(query).queryType,
+          isSessionIndependent,
+          sessionId: isSessionIndependent ? 'N/A' : sessionId,
+          cacheKey: cacheKey.substring(0, 16) + '...'
+        }
+      )
+    }
+
+    return result
   }
 
   /**
@@ -44,7 +62,8 @@ export class CacheManager {
     query: string,
     result: QueryResult,
     connectionId: string,
-    database?: string
+    database?: string,
+    sessionId?: string
   ): Promise<void> {
     if (!this.enabled || !QueryNormalizer.shouldCache(query)) {
       return
@@ -56,7 +75,8 @@ export class CacheManager {
     }
 
     const normalizedQuery = QueryNormalizer.normalize(query)
-    const cacheKey = QueryNormalizer.generateCacheKey(query, connectionId, database)
+    const cacheKey = QueryNormalizer.generateCacheKey(query, connectionId, database, sessionId)
+    const isSessionIndependent = QueryNormalizer.isSessionIndependentQuery(query)
 
     const metadata: CacheMetadata = {
       connectionId,
@@ -65,10 +85,24 @@ export class CacheManager {
       hitCount: 0,
       lastAccessed: Date.now(),
       tables: normalizedQuery.tables,
-      queryType: normalizedQuery.queryType
+      queryType: normalizedQuery.queryType,
+      sessionId: isSessionIndependent ? undefined : sessionId,
+      isSessionIndependent
     }
 
     await this.queryCache.set(cacheKey, result, metadata)
+
+    // Enhanced logging for cache strategy monitoring
+    console.log(
+      `[CACHE STORE] ${isSessionIndependent ? 'Session-Independent' : 'Session-Dependent'} query cached`,
+      {
+        queryType: normalizedQuery.queryType,
+        isSessionIndependent,
+        sessionId: isSessionIndependent ? 'N/A' : sessionId,
+        tables: normalizedQuery.tables,
+        cacheKey: cacheKey.substring(0, 16) + '...'
+      }
+    )
   }
 
   /**
@@ -163,14 +197,21 @@ export class CacheManager {
   /**
    * Get cache entry details for debugging
    */
-  getCacheEntryInfo(query: string, connectionId: string, database?: string): any {
-    const cacheKey = QueryNormalizer.generateCacheKey(query, connectionId, database)
+  getCacheEntryInfo(
+    query: string,
+    connectionId: string,
+    database?: string,
+    sessionId?: string
+  ): any {
+    const cacheKey = QueryNormalizer.generateCacheKey(query, connectionId, database, sessionId)
     const normalizedQuery = QueryNormalizer.normalize(query)
+    const isSessionIndependent = QueryNormalizer.isSessionIndependentQuery(query)
 
     return {
       cacheKey,
       normalizedQuery,
       shouldCache: QueryNormalizer.shouldCache(query),
+      isSessionIndependent,
       isEnabled: this.enabled
     }
   }
diff --git a/src/main/cache/QueryNormalizer.ts b/src/main/cache/QueryNormalizer.ts
@@ -34,12 +34,96 @@ export class QueryNormalizer {
     }
   }
 
+  /**
+   * Check if a query can be cached session-independently
+   */
+  static isSessionIndependentQuery(query: string): boolean {
+    const normalized = this.normalize(query)
+
+    // Only SELECT queries can be session-independent
+    if (normalized.queryType !== 'SELECT') {
+      return false
+    }
+
+    const upperQuery = normalized.normalized
+
+    // Session-dependent patterns to exclude
+    const sessionDependentPatterns = [
+      // User or session-specific functions
+      'USER()',
+      'CURRENT_USER',
+      'SESSION_USER',
+      'CONNECTION_ID()',
+      'LAST_INSERT_ID()',
+
+      // Temporary tables
+      '#',
+      'TEMPORARY',
+
+      // Variables
+      '@',
+      'SET @',
+      'SELECT @',
+
+      // Time-sensitive functions (already covered in shouldCache)
+      'NOW()',
+      'CURRENT_TIMESTAMP',
+      'CURRENT_DATE',
+      'CURRENT_TIME',
+      'RAND()',
+      'RANDOM()',
+      'UUID()',
+      'NEWID()'
+    ]
+
+    // Check for session-dependent patterns
+    const hasSessionDependentContent = sessionDependentPatterns.some((pattern) =>
+      upperQuery.includes(pattern)
+    )
+
+    if (hasSessionDependentContent) {
+      return false
+    }
+
+    // Check for table browsing patterns (simple SELECT * with basic conditions)
+    const isTableBrowsing =
+      // Simple SELECT * queries
+      (upperQuery.match(/^SELECT \* FROM/) ||
+        // SELECT with specific columns from single table
+        upperQuery.match(/^SELECT [^()]+FROM [^\s,;()]+$/)) &&
+      // With basic WHERE conditions (no subqueries or complex joins)
+      (!upperQuery.includes('(') || upperQuery.match(/WHERE [^()]+$/)) &&
+      // No complex operations
+      !upperQuery.includes('JOIN') &&
+      !upperQuery.includes('UNION') &&
+      !upperQuery.includes('CASE') &&
+      !upperQuery.includes('EXISTS')
+
+    return isTableBrowsing
+  }
+
   /**
    * Generate a cache key from query and connection parameters
    */
-  static generateCacheKey(query: string, connectionId: string, database?: string): string {
+  static generateCacheKey(
+    query: string,
+    connectionId: string,
+    database?: string,
+    sessionId?: string
+  ): string {
     const normalized = this.normalize(query)
-    const contextData = `${connectionId}:${database || 'default'}`
+
+    // Check if this query can be cached session-independently
+    const isSessionIndependent = this.isSessionIndependentQuery(query)
+
+    let contextData: string
+    if (isSessionIndependent) {
+      // For session-independent queries, exclude sessionId from cache key
+      contextData = `${connectionId}:${database || 'default'}`
+    } else {
+      // For session-dependent queries, include sessionId
+      contextData = `${connectionId}:${database || 'default'}:${sessionId || 'default'}`
+    }
 
     return crypto
       .createHash('sha256')
diff --git a/src/main/cache/types.ts b/src/main/cache/types.ts
@@ -19,6 +19,8 @@ export interface CacheMetadata {
   lastAccessed: number
   tables: string[]
   queryType: 'SELECT' | 'DDL' | 'DML' | 'OTHER'
+  sessionId?: string
+  isSessionIndependent?: boolean
 }
 
 export interface CacheConfig {
diff --git a/src/main/database/base.ts b/src/main/database/base.ts
@@ -113,7 +113,12 @@ export abstract class BaseDatabaseManager implements DatabaseManagerInterface {
       const database = connectionInfo?.database
 
       // Try to get cached result first
-      const cachedResult = await this.cacheManager.getCachedResult(sql, connectionId, database)
+      const cachedResult = await this.cacheManager.getCachedResult(
+        sql,
+        connectionId,
+        database,
+        sessionId
+      )
       if (cachedResult) {
         console.log('Cache hit for query:', sql.substring(0, 100) + '...')
         return cachedResult
@@ -126,7 +131,7 @@ export abstract class BaseDatabaseManager implements DatabaseManagerInterface {
 
       // Cache the result if successful
       if (result.success) {
-        await this.cacheManager.cacheResult(sql, result, connectionId, database)
+        await this.cacheManager.cacheResult(sql, result, connectionId, database, sessionId)
       }
 
       // Handle cache invalidation for DDL/DML queries
diff --git a/src/main/database/clickhouse.ts b/src/main/database/clickhouse.ts
@@ -10,6 +10,7 @@ import {
   TableQueryOptions,
   TableFilter
 } from './interface'
+import { logger } from '../utils/logger'
 
 interface ClickHouseConfig {
   host: string
@@ -166,6 +167,9 @@ class ClickHouseManager extends BaseDatabaseManager {
   }
 
   async executeQuery(connectionId: string, sql: string, sessionId?: string): Promise<QueryResult> {
+    const startTime = Date.now()
+    const timestamp = new Date().toISOString()
+
     try {
       const connection = this.connections.get(connectionId)
       if (!connection || !connection.isConnected) {
@@ -175,6 +179,12 @@ class ClickHouseManager extends BaseDatabaseManager {
       // Validate read-only queries
       const validation = this.validateReadOnlyQuery(connectionId, sql)
       if (!validation.valid) {
+        logger.warn(`[${timestamp}] ClickHouse read-only query validation failed`, {
+          connectionId,
+          sql: sql.slice(0, 100) + (sql.length > 100 ? '...' : ''),
+          error: validation.error,
+          timestamp
+        })
         return this.createQueryResult(false, validation.error || 'Query not allowed')
       }
 
@@ -186,13 +196,41 @@ class ClickHouseManager extends BaseDatabaseManager {
       const isDDL = queryType === QueryType.DDL
       const isDML = [QueryType.INSERT, QueryType.UPDATE, QueryType.DELETE].includes(queryType)
 
+      logger.debug(`[${timestamp}] ClickHouse query details`, {
+        connectionId,
+        queryType,
+        isDDL,
+        isDML,
+        sessionId,
+        host: connection.config.host,
+        database: connection.config.database,
+        timestamp
+      })
+
       if (isDDL || isDML) {
+        logger.info(`[${timestamp}] Executing ClickHouse command`, {
+          connectionId,
+          queryType,
+          sessionId,
+          sql: sql.slice(0, 200) + (sql.length > 200 ? '...' : ''),
+          timestamp
+        })
+
         // Use command() for DDL/DML queries that don't return data
         await connection.client.command({
           query: sql,
           query_id: sessionId || undefined
         })
 
+        const duration = Date.now() - startTime
+        const completionTimestamp = new Date().toISOString()
+        logger.info(`[${completionTimestamp}] ClickHouse command completed in ${duration}ms`, {
+          queryType,
+          sessionId,
+          duration,
+          timestamp: completionTimestamp
+        })
+
         return this.createQueryResult(
           true,
           'Command executed successfully',
@@ -202,6 +240,14 @@ class ClickHouseManager extends BaseDatabaseManager {
           isDML ? 1 : 0 // For DML, we don't get affected rows from ClickHouse easily
         )
       } else {
+        logger.info(`[${timestamp}] Executing ClickHouse query`, {
+          connectionId,
+          queryType,
+          sessionId,
+          sql: sql.slice(0, 200) + (sql.length > 200 ? '...' : ''),
+          timestamp
+        })
+
         // Use query() for SELECT and data-returning queries
         const abortController = new AbortController()
 
@@ -234,6 +280,16 @@ class ClickHouseManager extends BaseDatabaseManager {
           this.activeQueries.delete(sessionId)
         }
 
+        const duration = Date.now() - startTime
+        const completionTimestamp = new Date().toISOString()
+        logger.info(`[${completionTimestamp}] ClickHouse query completed in ${duration}ms`, {
+          queryType,
+          rowCount: data.length,
+          sessionId,
+          duration,
+          timestamp: completionTimestamp
+        })
+
         return this.createQueryResult(
           true,
           `Query executed successfully. Returned ${data.length} rows.`,
@@ -248,10 +304,27 @@ class ClickHouseManager extends BaseDatabaseManager {
         this.activeQueries.delete(sessionId)
       }
 
-      console.error('ClickHouse query error:', error)
+      const duration = Date.now() - startTime
+      const errorTimestamp = new Date().toISOString()
+
+      logger.error(`[${errorTimestamp}] ClickHouse query error after ${duration}ms`, {
+        error: error instanceof Error ? error.message : 'Unknown error',
+        connectionId,
+        sessionId,
+        duration,
+        sql: sql.slice(0, 200) + (sql.length > 200 ? '...' : ''),
+        timestamp: errorTimestamp
+      })
 
       // Check if it was cancelled
       if (error instanceof Error && error.name === 'AbortError') {
+        logger.info(`[${errorTimestamp}] ClickHouse query was cancelled`, {
+          connectionId,
+          sessionId,
+          duration,
+          timestamp: errorTimestamp
+        })
+
         return this.createQueryResult(
           false,
           'Query was cancelled',
diff --git a/src/main/database/manager.ts b/src/main/database/manager.ts

Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,8 @@ export interface CacheMetadata {`
`19`	`19`	`lastAccessed: number`
`20`	`20`	`tables: string[]`
`21`	`21`	`queryType: 'SELECT' \| 'DDL' \| 'DML' \| 'OTHER'`
	`22`	`+ sessionId?: string`
	`23`	`+ isSessionIndependent?: boolean`
`22`	`24`	`}`
`23`	`25`
`24`	`26`	`export interface CacheConfig {`