fix(FiveSql2): 9 latent bugs from static analysis sweep

Systematic bug-hunt driven by an automated analysis of all FiveSql2
source files. Each fix is targeted — no speculative refactoring.

--- #1 CLASSDATA hSubCache leaked across queries (CRITICAL) ---

  CLASSDATA hSubCache INIT { => } SHARED

shared one hash across ALL TSqlExecutor instances. A non-correlated
subquery cached in query A was silently returned for an unrelated
query B if the subquery text happened to produce the same cache key.
Converted to instance DATA initialized in New().

--- #5+#21 IS NULL / COALESCE treated empty string as NULL (HIGH) ---

  RETURN xL == NIL .OR. ( ValType(xL) == "C" .AND. Empty(AllTrim(xL)) )

SQL standard: '' is a valid non-NULL value. Removed the empty-string
check from both IS NULL evaluation and COALESCE skip logic.

--- #4 Multiple ? parameters all returned first value (HIGH) ---

ND_PAR nodes had no index — EvalExpr always returned ::aParams[1].
Parser now stamps each ? with a sequential 1-based index in xNode[2].
EvalExpr uses it to return the correct ::aParams[n].

--- #10+#11 SqlEvalRowExpr missing / and || operators, single-arg
    function eval (MEDIUM) ---

Division and string concatenation fell through to RETURN NIL in the
row-expression evaluator used by recursive CTEs and aggregate
ComputeAgg. Also, multi-argument functions like SUBSTR(x,2,3) only
received the first argument. Both fixed.

--- #9 SUM/AVG/MIN/MAX of all NULLs returned 0 instead of NULL
    (MEDIUM) ---

SQL standard requires NULL. Changed the aggregate return path to
return NIL when nCount == 0 (SUM/AVG) or when xMin/xMax == NIL.

--- #8 MIN/MAX used SqlCoerceNum for comparison (MEDIUM) ---

Strings and dates were coerced to numbers (Val()) before comparing,
making MIN('banana') == MIN('apple') == 0. Switched to SqlCmpLt
which handles type-appropriate comparison.

--- #7 SqlExprHasAgg only checked top-level node (MEDIUM) ---

Expressions like `salary + COUNT(*)` were not detected as containing
an aggregate because the top node was ND_BIN, not ND_FN. Made the
function recursive — walks ND_BIN, ND_UNI, ND_FN args, ND_CASE
branches.

--- #13 SELECT * only expanded first table in JOINs (MEDIUM) ---

`SELECT * FROM orders o JOIN customers c ON ...` only included
fields from orders. Changed the expansion loop to iterate ALL
entries in ::aTables.

--- #2 s_aOuterStack not unwound on subquery error (HIGH) ---

SubqueryCached's PushOuter/PopOuter pair was not protected by
BEGIN SEQUENCE. A runtime error inside the subquery left a stale
entry on the module-level outer stack, corrupting all subsequent
queries' correlated column resolution. Wrapped in SEQUENCE/RECOVER.

Validation:
  - FiveSql2 43/43
  - Harbour compat 51/51
  - go test ./... ALL PASS

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

_FiveSql2/src/TSqlAgg.prg

@@ -424,10 +424,11 @@ METHOD ComputeAgg( xE, aGR, aFN ) CLASS TSqlAgg
       IF xVal != NIL
          nCount++
          nSum += SqlCoerceNum( xVal )
-         IF xMin == NIL .OR. SqlCoerceNum( xVal ) < SqlCoerceNum( xMin )
+         /* Use SqlCmpLt for type-safe comparison (handles strings, dates) */
+         IF xMin == NIL .OR. SqlCmpLt( xVal, xMin )
             xMin := xVal
          ENDIF
-         IF xMax == NIL .OR. SqlCoerceNum( xVal ) > SqlCoerceNum( xMax )
+         IF xMax == NIL .OR. SqlCmpLt( xMax, xVal )
             xMax := xVal
          ENDIF
       ENDIF
@@ -437,13 +438,14 @@ METHOD ComputeAgg( xE, aGR, aFN ) CLASS TSqlAgg
    CASE cFunc == "COUNT"
       RETURN nCount
    CASE cFunc == "SUM"
-      RETURN nSum
+      /* SQL standard: SUM of all NULLs = NULL, not 0 */
+      RETURN iif( nCount > 0, nSum, NIL )
    CASE cFunc == "AVG"
-      RETURN iif( nCount > 0, nSum / nCount, 0 )
+      RETURN iif( nCount > 0, nSum / nCount, NIL )
    CASE cFunc == "MIN"
-      RETURN iif( xMin != NIL, xMin, 0 )
+      RETURN xMin
    CASE cFunc == "MAX"
-      RETURN iif( xMax != NIL, xMax, 0 )
+      RETURN xMax
    CASE cFunc == "GROUP_CONCAT" .OR. cFunc == "STRING_AGG"
       cResult := ""
       cSep := ", "

_FiveSql2/src/TSqlExecutor.prg

@@ -40,7 +40,7 @@ CLASS TSqlExecutor
    DATA nSubCacheSeq    INIT 0          /* monotonic ID for subqueries */
    DATA aSemiJoinSlots  INIT {}         /* list of {xSubNode, semiJoinData | "NO"} */
 
-   CLASSDATA hSubCache INIT { => } SHARED
+   DATA hSubCache
 
    METHOD New( hQuery, aParams ) CONSTRUCTOR
    METHOD Run()
@@ -99,6 +99,7 @@ METHOD New( hQuery, aParams ) CLASS TSqlExecutor
     * can end up sharing the same instance across New() calls depending
     * on the compile path, which would let one query's subquery cache
     * leak into the next query's results. */
+   ::hSubCache      := { => }
    ::hSubCorrCache  := { => }
    ::aSubCacheSlots := {}
    ::aSemiJoinSlots := {}
@@ -412,10 +413,10 @@ METHOD EvalExpr( xNode ) CLASS TSqlExecutor
       RETURN ::Resolve( xNode[ 2 ] )
 
    CASE xNode[ 1 ] == ND_PAR
-      IF Len( ::aParams ) > 0
+      /* xNode[2] = 1-based parameter index from parser */
-         /* Use static counter per expression evaluation chain */
+      nPI := iif( xNode[ 2 ] != NIL, xNode[ 2 ], 1 )
-         xVal := ::aParams[ 1 ]
+      IF nPI >= 1 .AND. nPI <= Len( ::aParams )
-         RETURN xVal
+         RETURN ::aParams[ nPI ]
       ENDIF
       RETURN NIL
 
@@ -494,10 +495,11 @@ METHOD EvalExpr( xNode ) CLASS TSqlExecutor
       /* IS NULL / IS NOT NULL */
       IF cOp == "IS NULL" .OR. cOp == "IS NOT NULL"
          xL := ::EvalExpr( xNode[ 3 ] )
+         /* SQL standard: only NIL is NULL, empty string '' is NOT NULL */
          IF cOp == "IS NULL"
-            RETURN xL == NIL .OR. ( ValType( xL ) == "C" .AND. Empty( AllTrim( xL ) ) )
+            RETURN xL == NIL
          ELSE
-            RETURN !( xL == NIL .OR. ( ValType( xL ) == "C" .AND. Empty( AllTrim( xL ) ) ) )
+            RETURN xL != NIL
          ENDIF
       ENDIF
 
@@ -1244,14 +1246,14 @@ METHOD RunSelect() CLASS TSqlExecutor
          cColAlias := SqlExprName( xExpr )
       ENDIF
       AAdd( aResultExprs, { xExpr, cColAlias } )
-      /* Expand SELECT * */
+      /* Expand SELECT * — iterate ALL tables (primary + joined) */
       IF xExpr[ 1 ] == ND_COL .AND. xExpr[ 2 ] == "*"
          aResultExprs := {}
          aFieldNames := {}
-         IF Len( ::aTables ) > 0
+         FOR k := 1 TO Len( ::aTables )
-            cAlias := ::aTables[ 1 ][ 2 ]
+            cAlias := ::aTables[ k ][ 2 ]
             IF Empty( cAlias )
-               cAlias := ::aTables[ 1 ][ 1 ]
+               cAlias := ::aTables[ k ][ 1 ]
             ENDIF
             nWA := Select( cAlias )
             IF nWA > 0
@@ -1261,7 +1263,7 @@ METHOD RunSelect() CLASS TSqlExecutor
                   AAdd( aResultExprs, { SqlNode( ND_COL, cFN, NIL, NIL, NIL ), cFN } )
                NEXT
             ENDIF
-         ENDIF
+         NEXT
          EXIT
       ENDIF
    NEXT
@@ -1995,16 +1997,24 @@ METHOD SubqueryCached( xSubNode ) CLASS TSqlExecutor
    ENDIF
 
    /* Cache miss — execute the subquery. PushOuter so nested ::Resolve
-    * calls can see the current outer row's values. */
+    * calls can see the current outer row's values. Use BEGIN SEQUENCE
+    * to guarantee PopOuter runs even on subquery runtime errors —
+    * a stale s_aOuterStack entry would corrupt all subsequent queries. */
    nSavedWA := Select()
    ::PushOuter()
-   oSub := TSqlExecutor():New( hQ, ::aParams )
+   BEGIN SEQUENCE
-   oSub:nDepth := ::nDepth
+      oSub := TSqlExecutor():New( hQ, ::aParams )
-   aResult := oSub:Run()
+      oSub:nDepth := ::nDepth
+      aResult := oSub:Run()
+   RECOVER
+      aResult := NIL
+   END SEQUENCE
    ::PopOuter()
    dbSelectArea( nSavedWA )
 
-   ::hSubCorrCache[ cCacheKey ] := aResult
+   IF aResult != NIL
+      ::hSubCorrCache[ cCacheKey ] := aResult
+   ENDIF
 
 RETURN aResult
 

_FiveSql2/src/TSqlExpr.prg

@@ -42,15 +42,43 @@ FUNCTION SqlExprName( xE )
 
 RETURN "expr"
 
-/* Check whether an expression node is an aggregate function call */
+/* Check whether an expression tree contains an aggregate function call.
+ * Recurses into ND_BIN, ND_UNI, ND_FN args, ND_CASE to find nested
+ * aggregates like `salary + COUNT(*)` or `CASE WHEN ... THEN SUM(x)`. */
 FUNCTION SqlExprHasAgg( xE )
 
+   LOCAL i
+
    IF xE == NIL
       RETURN .F.
    ENDIF
    IF xE[ 1 ] == ND_FN .AND. SqlIsAggName( xE[ 2 ] )
       RETURN .T.
    ENDIF
+   /* Recurse into sub-expressions */
+   IF xE[ 1 ] == ND_BIN
+      RETURN SqlExprHasAgg( xE[ 3 ] ) .OR. SqlExprHasAgg( xE[ 4 ] )
+   ENDIF
+   IF xE[ 1 ] == ND_UNI
+      RETURN SqlExprHasAgg( xE[ 3 ] )
+   ENDIF
+   IF xE[ 1 ] == ND_FN .AND. ValType( xE[ 3 ] ) == "A"
+      FOR i := 1 TO Len( xE[ 3 ] )
+         IF SqlExprHasAgg( xE[ 3 ][ i ] )
+            RETURN .T.
+         ENDIF
+      NEXT
+   ENDIF
+   IF xE[ 1 ] == ND_CASE .AND. ValType( xE[ 2 ] ) == "A"
+      FOR i := 1 TO Len( xE[ 2 ] )
+         IF SqlExprHasAgg( xE[ 2 ][ i ][ 1 ] ) .OR. SqlExprHasAgg( xE[ 2 ][ i ][ 2 ] )
+            RETURN .T.
+         ENDIF
+      NEXT
+      IF xE[ 3 ] != NIL .AND. SqlExprHasAgg( xE[ 3 ] )
+         RETURN .T.
+      ENDIF
+   ENDIF
 
 RETURN .F.
 
@@ -173,7 +201,7 @@ RETURN xExpr
  */
 FUNCTION SqlEvalRowExpr( xExpr, aFN, aRow )
 
-   LOCAL xL, xR, cOp, cName, i
+   LOCAL xL, xR, cOp, cName, i, aFnArgs
 
    IF xExpr == NIL
       RETURN NIL
@@ -221,6 +249,15 @@ FUNCTION SqlEvalRowExpr( xExpr, aFN, aRow )
       IF cOp == "*"
          RETURN SqlCoerceNum( xL ) * SqlCoerceNum( xR )
       ENDIF
+      IF cOp == "/"
+         IF SqlCoerceNum( xR ) != 0
+            RETURN SqlCoerceNum( xL ) / SqlCoerceNum( xR )
+         ENDIF
+         RETURN 0
+      ENDIF
+      IF cOp == "||"
+         RETURN SqlCoerceStr( xL ) + SqlCoerceStr( xR )
+      ENDIF
       IF cOp == "AND"
          RETURN SqlIsTrue( xL ) .AND. SqlIsTrue( xR )
       ENDIF
@@ -263,7 +300,12 @@ FUNCTION SqlEvalRowExpr( xExpr, aFN, aRow )
 
    CASE xExpr[ 1 ] == ND_FN
       IF Len( xExpr[ 3 ] ) > 0
-         RETURN SqlEvalFunc( xExpr[ 2 ], { SqlEvalRowExpr( xExpr[ 3 ][ 1 ], aFN, aRow ) } )
+         /* Evaluate ALL arguments, not just the first */
+         aFnArgs := {}
+         FOR i := 1 TO Len( xExpr[ 3 ] )
+            AAdd( aFnArgs, SqlEvalRowExpr( xExpr[ 3 ][ i ], aFN, aRow ) )
+         NEXT
+         RETURN SqlEvalFunc( xExpr[ 2 ], aFnArgs )
       ENDIF
       RETURN SqlEvalFunc( xExpr[ 2 ], {} )
 

_FiveSql2/src/TSqlFunc.prg

@@ -159,9 +159,9 @@ FUNCTION SqlEvalFunc( cName, aArgs )
       ENDIF
       RETURN SqlArg(aArgs,3)
    CASE cName == "COALESCE"
+      /* SQL standard: empty string is NOT NULL, only NIL is NULL */
       FOR i := 1 TO Len( aArgs )
-         IF aArgs[ i ] != NIL .AND. ;
+         IF aArgs[ i ] != NIL
-            !( ValType( aArgs[ i ] ) == "C" .AND. Empty( AllTrim( aArgs[ i ] ) ) )
             RETURN aArgs[ i ]
          ENDIF
       NEXT

_FiveSql2/src/TSqlParser2.prg

@@ -129,6 +129,7 @@ CLASS TSqlParser2
    DATA aTokens
    DATA nPos
    DATA aParams
+   DATA nParamSeq    INIT 0    /* positional ? counter (1-based) */
    DATA hInfixTT     /* hash: token_type => { op, lbp, rbp, ndType } */
    DATA hInfixKW     /* hash: keyword => { op, lbp, rbp, ndType } */
 
@@ -1375,10 +1376,12 @@ METHOD ParsePrimary() CLASS TSqlParser2
       RETURN SqlNode( ND_LIT, cVal, NIL, NIL, NIL )
    ENDIF
 
-   /* Positional parameter */
+   /* Positional parameter — each ? gets a sequential 1-based index
+    * so EvalExpr can return the correct ::aParams[n] value. */
    IF ::TType( ::nPos ) == TK_QMARK
       ::nPos++
-      RETURN SqlNode( ND_PAR, NIL, NIL, NIL, NIL )
+      ::nParamSeq++
+      RETURN SqlNode( ND_PAR, ::nParamSeq, NIL, NIL, NIL )
    ENDIF
 
    /* Parenthesized expression or scalar subquery */