From 4df6c400343afefdc3b3cd6a5879cf6add939db2 Mon Sep 17 00:00:00 2001 From: filipriec Date: Thu, 24 Jul 2025 21:56:07 +0200 Subject: [PATCH] removal of hardcoded fix, now working in general --- .../handlers/dependency_analyzer.rs | 83 +++++++++---------- .../handlers/post_table_script.rs | 2 +- 2 files changed, 42 insertions(+), 43 deletions(-) diff --git a/server/src/table_script/handlers/dependency_analyzer.rs b/server/src/table_script/handlers/dependency_analyzer.rs index 9b128b7..3a2dd50 100644 --- a/server/src/table_script/handlers/dependency_analyzer.rs +++ b/server/src/table_script/handlers/dependency_analyzer.rs @@ -106,7 +106,11 @@ impl DependencyAnalyzer { /// Analyzes a Steel script to extract all table dependencies /// Uses regex patterns to find function calls that create dependencies - pub fn analyze_script_dependencies(&self, script: &str) -> Result, DependencyError> { + /// + /// # Arguments + /// * `script` - The Steel script to analyze + /// * `current_table_name` - Name of the table this script belongs to (for self-references) + pub fn analyze_script_dependencies(&self, script: &str, current_table_name: &str) -> Result, DependencyError> { let mut dependencies = Vec::new(); // Extract function calls and SQL dependencies using regex @@ -114,10 +118,10 @@ impl DependencyAnalyzer { dependencies.extend(self.extract_sql_dependencies(script)?); // Extract get-var calls (for transformed scripts with variables) - dependencies.extend(self.extract_get_var_calls(script)?); + dependencies.extend(self.extract_get_var_calls(script, current_table_name)?); // Extract direct variable references like @price, @quantity - dependencies.extend(self.extract_variable_references(script)?); + dependencies.extend(self.extract_variable_references(script, current_table_name)?); Ok(dependencies) } @@ -160,7 +164,8 @@ impl DependencyAnalyzer { } /// Extract get-var calls as dependencies (for transformed scripts with variables) - fn extract_get_var_calls(&self, script: &str) -> Result, DependencyError> { + /// These are self-references to the current table + fn extract_get_var_calls(&self, script: &str, current_table_name: &str) -> Result, DependencyError> { let mut dependencies = Vec::new(); // Look for get-var patterns in transformed scripts: (get-var "variable") @@ -170,7 +175,7 @@ impl DependencyAnalyzer { for caps in get_var_pattern.captures_iter(script) { let variable_name = caps[1].to_string(); dependencies.push(Dependency { - target_table: "SAME_TABLE".to_string(), // Special marker for same-table references + target_table: current_table_name.to_string(), // Use actual table name dependency_type: DependencyType::ColumnAccess { column: variable_name }, context: None, }); @@ -180,7 +185,8 @@ impl DependencyAnalyzer { } /// Extract direct variable references like @price, @quantity - fn extract_variable_references(&self, script: &str) -> Result, DependencyError> { + /// These are self-references to the current table + fn extract_variable_references(&self, script: &str, current_table_name: &str) -> Result, DependencyError> { let mut dependencies = Vec::new(); // Look for @variable patterns: @price, @quantity, etc. @@ -190,7 +196,7 @@ impl DependencyAnalyzer { for caps in variable_pattern.captures_iter(script) { let variable_name = caps[1].to_string(); dependencies.push(Dependency { - target_table: "SAME_TABLE".to_string(), // Same table reference + target_table: current_table_name.to_string(), // Use actual table name dependency_type: DependencyType::ColumnAccess { column: variable_name }, context: None, }); @@ -286,23 +292,19 @@ impl DependencyAnalyzer { // Add new dependencies to test - EXCLUDE self-references for dep in new_dependencies { - // Look up target table ID - let target_id = if dep.target_table == "SAME_TABLE" { - table_id // Same table reference - } else { - sqlx::query_scalar!( - "SELECT id FROM table_definitions WHERE schema_id = $1 AND table_name = $2", - self.schema_id, - dep.target_table - ) - .fetch_optional(&mut **tx) - .await - .map_err(|e| DependencyError::DatabaseError { error: e.to_string() })? - .ok_or_else(|| DependencyError::InvalidTableReference { - table_name: dep.target_table.clone(), - script_context: format!("table_id_{}", table_id), - })? - }; + // Look up target table ID using the actual table name + let target_id = sqlx::query_scalar!( + "SELECT id FROM table_definitions WHERE schema_id = $1 AND table_name = $2", + self.schema_id, + dep.target_table + ) + .fetch_optional(&mut **tx) + .await + .map_err(|e| DependencyError::DatabaseError { error: e.to_string() })? + .ok_or_else(|| DependencyError::InvalidTableReference { + table_name: dep.target_table.clone(), + script_context: format!("table_id_{}", table_id), + })?; // Only add to cycle detection graph if it's NOT a self-reference if table_id != target_id { @@ -444,8 +446,8 @@ impl DependencyAnalyzer { match &dep.dependency_type { // Structured access must respect link constraints (but self-references are always allowed) DependencyType::ColumnAccess { column } | DependencyType::IndexedAccess { column, .. } => { - // Skip validation for SAME_TABLE marker (these are always allowed) - if dep.target_table == "SAME_TABLE" { + // Self-references are always allowed (compare table names directly) + if dep.target_table == current_table_name { continue; } @@ -522,22 +524,19 @@ impl DependencyAnalyzer { // Insert new dependencies for dep in dependencies { - let target_id = if dep.target_table == "SAME_TABLE" { - table_id // Use the same table as the script - } else { - sqlx::query_scalar!( - "SELECT id FROM table_definitions WHERE schema_id = $1 AND table_name = $2", - self.schema_id, - dep.target_table - ) - .fetch_optional(&mut **tx) - .await - .map_err(|e| DependencyError::DatabaseError { error: e.to_string() })? - .ok_or_else(|| DependencyError::InvalidTableReference { - table_name: dep.target_table.clone(), - script_context: format!("script_id_{}", script_id), - })? - }; + // Look up target table ID using actual table name (no magic strings!) + let target_id = sqlx::query_scalar!( + "SELECT id FROM table_definitions WHERE schema_id = $1 AND table_name = $2", + self.schema_id, + dep.target_table + ) + .fetch_optional(&mut **tx) + .await + .map_err(|e| DependencyError::DatabaseError { error: e.to_string() })? + .ok_or_else(|| DependencyError::InvalidTableReference { + table_name: dep.target_table.clone(), + script_context: format!("script_id_{}", script_id), + })?; sqlx::query!( r#"INSERT INTO script_dependencies diff --git a/server/src/table_script/handlers/post_table_script.rs b/server/src/table_script/handlers/post_table_script.rs index de1caa7..2b7d214 100644 --- a/server/src/table_script/handlers/post_table_script.rs +++ b/server/src/table_script/handlers/post_table_script.rs @@ -631,7 +631,7 @@ pub async fn post_table_script( // Analyze script dependencies let dependencies = analyzer - .analyze_script_dependencies(&request.script) + .analyze_script_dependencies(&request.script, &table_def.table_name) .map_err(|e| Status::from(e))?; // Check for circular dependencies BEFORE making any changes