Fix AddTablePrefixToColumns to handle parenthesized AND conditions correctly

Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

pkg/common/sql_helpers.go

@@ -265,6 +265,41 @@ func stripOuterParentheses(s string) string {
 	}
 }
 
+// stripOneOuterParentheses removes only one level of matching outer parentheses from a string
+// Unlike stripOuterParentheses, this only strips once, preserving nested parentheses
+func stripOneOuterParentheses(s string) string {
+	s = strings.TrimSpace(s)
+
+	if len(s) < 2 || s[0] != '(' || s[len(s)-1] != ')' {
+		return s
+	}
+
+	// Check if these parentheses match (i.e., they're the outermost pair)
+	depth := 0
+	matched := false
+	for i := 0; i < len(s); i++ {
+		switch s[i] {
+		case '(':
+			depth++
+		case ')':
+			depth--
+			if depth == 0 && i == len(s)-1 {
+				matched = true
+			} else if depth == 0 {
+				// Found a closing paren before the end, so outer parens don't match
+				return s
+			}
+		}
+	}
+
+	if !matched {
+		return s
+	}
+
+	// Strip only the outer parentheses
+	return strings.TrimSpace(s[1 : len(s)-1])
+}
+
 // splitByAND splits a WHERE clause by AND operators (case-insensitive)
 // This is parenthesis-aware and won't split on AND operators inside subqueries
 func splitByAND(where string) []string {
@@ -683,8 +718,8 @@ func AddTablePrefixToColumns(where string, tableName string) string {
 //   - No valid column reference is found
 //   - The column doesn't exist in the table (when validColumns is provided)
 func addPrefixToSingleCondition(cond string, tableName string, validColumns map[string]bool) string {
-	// Strip outer grouping parentheses to get to the actual condition
-	strippedCond := stripOuterParentheses(cond)
+	// Strip one level of outer grouping parentheses to get to the actual condition
+	strippedCond := stripOneOuterParentheses(cond)
 
 	// Skip SQL literals and trivial conditions (true, false, null, 1=1, etc.)
 	if IsSQLExpression(strippedCond) || IsTrivialCondition(strippedCond) {
@@ -692,6 +727,34 @@ func addPrefixToSingleCondition(cond string, tableName string, validColumns map[
 		return cond
 	}
 
+	// After stripping outer parentheses, check if there are multiple AND-separated conditions
+	// at the top level. If so, split and process each separately to avoid incorrectly
+	// treating "true AND status" as a single column name.
+	subConditions := splitByAND(strippedCond)
+	if len(subConditions) > 1 {
+		// Multiple conditions found - process each separately
+		logger.Debug("Found %d sub-conditions after stripping parentheses, processing separately", len(subConditions))
+		processedConditions := make([]string, 0, len(subConditions))
+		for _, subCond := range subConditions {
+			// Recursively process each sub-condition
+			processed := addPrefixToSingleCondition(subCond, tableName, validColumns)
+			processedConditions = append(processedConditions, processed)
+		}
+		result := strings.Join(processedConditions, " AND ")
+		// Preserve original outer parentheses if they existed
+		if cond != strippedCond {
+			result = "(" + result + ")"
+		}
+		return result
+	}
+
+	// If we stripped parentheses and still have more parentheses, recursively process
+	if cond != strippedCond && strings.HasPrefix(strippedCond, "(") && strings.HasSuffix(strippedCond, ")") {
+		// Recursively handle nested parentheses
+		processed := addPrefixToSingleCondition(strippedCond, tableName, validColumns)
+		return "(" + processed + ")"
+	}
+
 	// Extract the left side of the comparison (before the operator)
 	columnRef := extractLeftSideOfComparison(strippedCond)
 	if columnRef == "" {

pkg/common/sql_helpers_test.go

@@ -658,3 +658,76 @@ func TestSanitizeWhereClauseWithModel(t *testing.T) {
 		})
 	}
 }
+
+func TestAddTablePrefixToColumns_ComplexConditions(t *testing.T) {
+tests := []struct {
+name      string
+where     string
+tableName string
+expected  string
+}{
+{
+name:      "Parentheses with true AND condition - should not prefix true",
+where:     "(true AND status = 'active')",
+tableName: "mastertask",
+expected:  "(true AND mastertask.status = 'active')",
+},
+{
+name:      "Parentheses with multiple conditions including true",
+where:     "(true AND status = 'active' AND id > 5)",
+tableName: "mastertask",
+expected:  "(true AND mastertask.status = 'active' AND mastertask.id > 5)",
+},
+{
+name:      "Nested parentheses with true",
+where:     "((true AND status = 'active'))",
+tableName: "mastertask",
+expected:  "((true AND mastertask.status = 'active'))",
+},
+{
+name:      "Mixed: false AND valid conditions",
+where:     "(false AND name = 'test')",
+tableName: "mastertask",
+expected:  "(false AND mastertask.name = 'test')",
+},
+{
+name:      "Mixed: null AND valid conditions",
+where:     "(null AND status = 'active')",
+tableName: "mastertask",
+expected:  "(null AND mastertask.status = 'active')",
+},
+{
+name:      "Multiple true conditions in parentheses",
+where:     "(true AND true AND status = 'active')",
+tableName: "mastertask",
+expected:  "(true AND true AND mastertask.status = 'active')",
+},
+{
+name:      "Simple true without parens - should not prefix",
+where:     "true",
+tableName: "mastertask",
+expected:  "true",
+},
+{
+name:      "Simple condition without parens - should prefix",
+where:     "status = 'active'",
+tableName: "mastertask",
+expected:  "mastertask.status = 'active'",
+},
+{
+name:      "Unregistered table with true - should not prefix true",
+where:     "(true AND status = 'active')",
+tableName: "unregistered_table",
+expected:  "(true AND unregistered_table.status = 'active')",
+},
+}
+
+for _, tt := range tests {
+t.Run(tt.name, func(t *testing.T) {
+result := AddTablePrefixToColumns(tt.where, tt.tableName)
+if result != tt.expected {
+t.Errorf("AddTablePrefixToColumns(%q, %q) = %q; want %q", tt.where, tt.tableName, result, tt.expected)
+}
+})
+}
+}

pkg/common/test_addprefix_test.go

@@ -1,63 +0,0 @@
-package common
-
-import (
-"fmt"
-"testing"
-)
-
-func TestAddTablePrefixToColumns_BugRepro(t *testing.T) {
-tests := []struct {
-name      string
-where     string
-tableName string
-expected  string
-}{
-{
-name:      "SQL literal true - should not add prefix",
-where:     "true",
-tableName: "mastertask",
-expected:  "true",
-},
-{
-name:      "SQL literal TRUE uppercase - should not add prefix",
-where:     "TRUE",
-tableName: "mastertask",
-expected:  "TRUE",
-},
-{
-name:      "SQL literal false - should not add prefix",
-where:     "false",
-tableName: "mastertask",
-expected:  "false",
-},
-{
-name:      "SQL literal null - should not add prefix",
-where:     "null",
-tableName: "mastertask",
-expected:  "null",
-},
-{
-name:      "SQL literal NULL uppercase - should not add prefix",
-where:     "NULL",
-tableName: "mastertask",
-expected:  "NULL",
-},
-{
-name:      "Multiple true conditions",
-where:     "true AND true",
-tableName: "mastertask",
-expected:  "true AND true",
-},
-}
-
-for _, tt := range tests {
-t.Run(tt.name, func(t *testing.T) {
-result := AddTablePrefixToColumns(tt.where, tt.tableName)
-if result != tt.expected {
-t.Errorf("AddTablePrefixToColumns(%q, %q) = %q; want %q", tt.where, tt.tableName, result, tt.expected)
-} else {
-fmt.Printf("✓ Test passed: %s\n", tt.name)
-}
-})
-}
-}

pkg/common/test_unregistered_test.go

@@ -1,51 +0,0 @@
-package common
-
-import (
-"fmt"
-"testing"
-)
-
-func TestAddTablePrefixToColumns_UnregisteredModel(t *testing.T) {
-tests := []struct {
-name      string
-where     string
-tableName string
-expected  string
-}{
-{
-name:      "SQL literal true with unregistered model - should not add prefix",
-where:     "true",
-tableName: "unregistered_table",
-expected:  "true",
-},
-{
-name:      "SQL literal false with unregistered model - should not add prefix",
-where:     "false",
-tableName: "unregistered_table",
-expected:  "false",
-},
-{
-name:      "SQL literal null with unregistered model - should not add prefix",
-where:     "null",
-tableName: "unregistered_table",
-expected:  "null",
-},
-{
-name:      "Valid column with unregistered model - should not add prefix (no validation)",
-where:     "status = 'active'",
-tableName: "unregistered_table",
-expected:  "unregistered_table.status = 'active'",
-},
-}
-
-for _, tt := range tests {
-t.Run(tt.name, func(t *testing.T) {
-result := AddTablePrefixToColumns(tt.where, tt.tableName)
-if result != tt.expected {
-t.Errorf("AddTablePrefixToColumns(%q, %q) = %q; want %q", tt.where, tt.tableName, result, tt.expected)
-} else {
-fmt.Printf("✓ Test passed: %s\n", tt.name)
-}
-})
-}
-}