From 1e6a9ae602c8350f2a05495a9d53168bcf1e0c8c Mon Sep 17 00:00:00 2001
From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Date: Tue, 22 Jul 2025 23:30:49 +0000
Subject: [PATCH] refactor: address code review feedback

- Simplify test assertion in test_wildcard_allowed_on_cte for better clarity
- Clean up validate_wildcard_on_tables logic to remove redundant checks
- Improve code readability and maintainability

Co-Authored-By: Dallin Bentley <dallinbentley98@gmail.com>
---
 apps/api/libs/sql_analyzer/src/analysis.rs    | 20 ++-----------------
 .../libs/sql_analyzer/tests/analysis_tests.rs |  9 +--------
 2 files changed, 3 insertions(+), 26 deletions(-)

diff --git a/apps/api/libs/sql_analyzer/src/analysis.rs b/apps/api/libs/sql_analyzer/src/analysis.rs
index 6ed4f4dd2..6526f75e1 100644
--- a/apps/api/libs/sql_analyzer/src/analysis.rs
+++ b/apps/api/libs/sql_analyzer/src/analysis.rs
@@ -1150,25 +1150,9 @@ impl QueryAnalyzer {
     }
 
     fn validate_wildcard_on_tables(&self) -> Result<(), SqlAnalyzerError> {
-        // Only validate tables that are actually in the FROM clause
-        if let Some(from_table) = &self.current_from_relation_identifier {
-            if let Some(table_info) = self.tables.get(from_table) {
-                if table_info.kind == TableKind::Base {
-                    return Err(SqlAnalyzerError::BlockedWildcardUsage(format!(
-                        "table '{}'", table_info.table_identifier
-                    )));
-                }
-            }
-        }
-        
-        // Also check any tables that might be in current scope aliases that are physical tables
+        // Check all tables in current scope aliases for physical tables
+        // This covers tables that are accessible in the current SELECT scope
         for alias in self.current_scope_aliases.keys() {
-            if let Some(from_table) = &self.current_from_relation_identifier {
-                if alias == from_table {
-                    continue;
-                }
-            }
-            
             if let Some(table_info) = self.tables.get(alias) {
                 if table_info.kind == TableKind::Base {
                     return Err(SqlAnalyzerError::BlockedWildcardUsage(format!(
diff --git a/apps/api/libs/sql_analyzer/tests/analysis_tests.rs b/apps/api/libs/sql_analyzer/tests/analysis_tests.rs
index 2c0bcde10..08cbc17bf 100644
--- a/apps/api/libs/sql_analyzer/tests/analysis_tests.rs
+++ b/apps/api/libs/sql_analyzer/tests/analysis_tests.rs
@@ -444,14 +444,7 @@ async fn test_wildcard_allowed_on_cte() {
     let sql = "WITH user_cte AS (SELECT u.id, u.name FROM schema.users u) SELECT * FROM user_cte";
     let result = analyze_query(sql.to_string(), "postgres").await;
     
-    match result {
-        Ok(_) => {
-        }
-        Err(e) => {
-            eprintln!("DEBUG: Unexpected error in test_wildcard_allowed_on_cte: {:?}", e);
-            panic!("Wildcard on CTE should be allowed, but got error: {:?}", e);
-        }
-    }
+    assert!(result.is_ok(), "Wildcard on CTE should be allowed");
 }
 
 #[tokio::test]