fivedev/five
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chore: audit cleanup — remove orphan parser + dead TSqlIndex methods

Browse Source 
Opus 4.7 audit of the codebase surfaced several items that Opus 4.6
sessions left behind. This pass removes what's definitively dead and
fixes one trivial defensive bug; the real logic bugs (transaction
ordering, missing RunUpdate/RunDelete validation) come in a separate
commit.

Deletions:

- `_FiveSql2/src/TSqlParser_orig.prg` (1173 lines) — superseded by
  `TSqlParser2.prg` (Pratt). Production never instantiates the old
  parser; the only callers were the comparison/benchmark test files
  also being removed.
- `_FiveSql2/test/test_parser_cmp.prg` — compared orig vs Pratt AST,
  useless now that orig is gone.
- `_FiveSql2/test/bench_parser.prg` — benched both, same reason.
- `_FiveSql2/Makefile` `test_cmp:` and `bench:` targets referenced
  the removed files.
- `TSqlIndex.prg` methods `ApplyScope`, `ClearScope`, `ApplySeek`,
  `IndexInfo`, `CreateTempIndex`, `DropTempIndex` — each declared in
  the class header and implemented (~165 lines total) but zero
  callers anywhere in `_FiveSql2/` or `hbrtl/`. Class declarations
  removed alongside the bodies.

Small fixes:

- `TSqlDDL.prg:179-180` stale comment claiming Five doesn't support
  `@byref` — false since commit e95afad (2026-04-13) wired @byref
  via RefCell. The same method uses @nPos correctly elsewhere.
- `hbrt/class.go:tryBinaryOp` defensive nil-check on AsArray().
  IsObject() checks the type tag; a corrupted Value with tag=Object
  but ptr=nil would crash on `.Class`. Correct construction paths
  never hit this, but the guard is cheap.

Compat tests: FiveSql2 43/43, Harbour compat 56/56, Go test ALL PASS.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
CharlesKWON
2026-04-18 22:46:17 +09:00
parent e5843bdde4
commit e368402682
7 changed files with 9 additions and 1545 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
_FiveSql2/Makefile
View File

@@ -68,12 +68,6 @@ test_challenge: $(OUTDIR)
test_extreme: $(OUTDIR)
$(HB) $(TESTDIR)/test_sql_extreme.prg $(SOURCES) -o$(OUTDIR)/test_extreme $(HBFLAGS)
test_cmp: $(OUTDIR)
$(HB) $(TESTDIR)/test_parser_cmp.prg $(SRCDIR)/TSqlParser2.prg $(SRCDIR)/TSqlLexer.prg $(SRCDIR)/TSqlExpr.prg $(SRCDIR)/TSqlFunc.prg $(SRCDIR)/FiveSqlDef.ch -o$(OUTDIR)/test_cmp $(HBFLAGS)
bench: $(OUTDIR)
$(HB) $(TESTDIR)/bench_parser.prg $(SRCDIR)/TSqlParser2.prg $(SRCDIR)/TSqlLexer.prg $(SRCDIR)/TSqlExpr.prg $(SRCDIR)/TSqlFunc.prg $(SRCDIR)/FiveSqlDef.ch -o$(OUTDIR)/bench_parser $(HBFLAGS)
# ============================================================================
# Run tests
# ============================================================================

3
_FiveSql2/src/TSqlDDL.prg
View File

@@ -176,8 +176,7 @@ METHOD CreateTable( aTokens, nPos ) CLASS TSqlDDL
LOOP
ENDIF
/* CHECK constraint (table-level) — inline parens extraction
* because Five does not support pass-by-reference (@nPos) */
/* CHECK constraint (table-level) — inline parens extraction. */
IF ::DDL_IsKW( aTokens, nPos, "CHECK" )
nPos++
cCheckExpr := ""

183
_FiveSql2/src/TSqlIndex.prg
View File

@@ -30,17 +30,11 @@ CLASS TSqlIndex
METHOD ListTags( nWA )
METHOD FindBestTag( nWA, cField )
METHOD FindCompoundTag( nWA, aFields )
METHOD ApplyScope( nWA, cField, xLow, xHigh )
METHOD ClearScope( nWA )
METHOD ApplySeek( nWA, cField, xValue )
METHOD BuildKey( nWA, xValue )
METHOD MatchOrderByTag( nWA, aOrderBy, aFieldNames )
METHOD TryIndexScan( nWA, xWhere, xFullWhere, aTables, aParams, aRE, aRows )
METHOD TryIndexJoinScan( nWA, xWhere, aTables, aParams, aRE, aRows, aJoins )
METHOD IndexInfo( nWA )
METHOD BuildKeyExpr( nWA, cField )
METHOD CreateTempIndex( nWA, cField )
METHOD DropTempIndex( nWA, cTempFile )
METHOD ExtractStrWidth( cExpr )
METHOD TryCompoundSeek( nWA, xWhere, xFullWhere, aTables, aParams, aRE, aRows )
METHOD ExtractEqPairs( xW, aTables, aParams, aOut )
@@ -344,95 +338,6 @@ METHOD FindCompoundTag( nWA, aFields ) CLASS TSqlIndex
RETURN nBestTag
/*
* Apply scope boundaries for range queries.
*
* CDX advantage: Can switch to the best tag for this field from among
* multiple available tags, then set scope on that specific tag.
* After the query, the original tag order is restored.
*
* NTX limitation: ordScope() applies to the currently active index.
* FindBestTag must select the right .ntx file first. Only one
* scope can be active at a time across all attached NTX files.
*/
METHOD ApplyScope( nWA, cField, xLow, xHigh ) CLASS TSqlIndex
LOCAL nSaved, nTag, cRDD
LOCAL xScopeLow, xScopeHigh, nPrevTag
nSaved := Select()
dbSelectArea( nWA )
cRDD := Upper( rddName() )
/* Save current tag so we can restore it after CDX tag switch */
nPrevTag := ordSetFocus()
nTag := ::FindBestTag( nWA, cField )
IF nTag == 0
/* No matching index — restore previous tag and fall back to full scan */
IF nPrevTag > 0
ordSetFocus( nPrevTag )
ENDIF
dbSelectArea( nSaved )
RETURN .F.
ENDIF
xScopeLow := ::BuildKey( nWA, xLow )
xScopeHigh := ::BuildKey( nWA, xHigh )
IF cRDD == "DBFCDX"
/* CDX: tag is already set by FindBestTag. ordScope applies to active tag.
* Multiple tags can have independent scopes in CDX. */
ordScope( 0, xScopeLow )
ordScope( 1, xScopeHigh )
ELSE
/* NTX: only one active index at a time. Scope applies globally. */
ordScope( 0, xScopeLow )
ordScope( 1, xScopeHigh )
ENDIF
dbSelectArea( nSaved )
RETURN .T.
METHOD ClearScope( nWA ) CLASS TSqlIndex
LOCAL nSaved
nSaved := Select()
dbSelectArea( nWA )
ordScope( 0, NIL )
ordScope( 1, NIL )
dbSelectArea( nSaved )
RETURN NIL
METHOD ApplySeek( nWA, cField, xValue ) CLASS TSqlIndex
LOCAL nSaved, nTag, xSeekKey, lFound
nSaved := Select()
dbSelectArea( nWA )
nTag := ::FindBestTag( nWA, cField )
IF nTag == 0
dbSelectArea( nSaved )
RETURN .F.
ENDIF
xSeekKey := ::BuildKey( nWA, xValue )
lFound := dbSeek( xSeekKey, .T. )
dbSelectArea( nSaved )
RETURN lFound
METHOD BuildKey( nWA, xValue ) CLASS TSqlIndex
LOCAL cExpr, nSaved, nWidth
@@ -714,34 +619,6 @@ METHOD TryIndexJoinScan( nWA, xWhere, aTables, aParams, aRE, aRows, aJoins ) CLA
RETURN .F.
METHOD IndexInfo( nWA ) CLASS TSqlIndex
LOCAL aTags, i, cInfo
aTags := ::ListTags( nWA )
cInfo := "RDD: " + ::DetectRDD( nWA ) + ;
" Orders: " + hb_ntos( Len( aTags ) ) + Chr(10)
FOR i := 1 TO Len( aTags )
cInfo += " [" + hb_ntos( i ) + "] " + ;
PadR( aTags[ i ][ 1 ], 12 ) + ;
" Key: " + aTags[ i ][ 2 ]
IF ! Empty( aTags[ i ][ 3 ] )
cInfo += " FOR: " + aTags[ i ][ 3 ]
ENDIF
IF aTags[ i ][ 4 ]
cInfo += " UNIQUE"
ENDIF
IF aTags[ i ][ 5 ]
cInfo += " DESC"
ENDIF
cInfo += Chr(10)
NEXT
RETURN cInfo
METHOD BuildKeyExpr( nWA, cField ) CLASS TSqlIndex
LOCAL nSaved, nFPos, cType, nLen
@@ -774,66 +651,6 @@ METHOD BuildKeyExpr( nWA, cField ) CLASS TSqlIndex
RETURN cField
METHOD CreateTempIndex( nWA, cField ) CLASS TSqlIndex
LOCAL nSaved, cTable, cTempFile, cExpr, cRDD
LOCAL lOk := .F.
nSaved := Select()
dbSelectArea( nWA )
cRDD := rddName()
cTable := Lower( AllTrim( Alias() ) )
cField := Upper( AllTrim( cField ) )
cExpr := ::BuildKeyExpr( nWA, cField )
IF Upper( cRDD ) == "DBFCDX"
cTempFile := "__tmp_" + cTable + ".cdx"
BEGIN SEQUENCE
ordCreate( cTempFile, "__TMP_" + cField, cExpr )
lOk := .T.
RECOVER
END SEQUENCE
ELSE
cTempFile := "__tmp_" + cTable + "_" + Lower( cField ) + ".ntx"
BEGIN SEQUENCE
dbCreateIndex( cTempFile, cExpr )
lOk := .T.
RECOVER
END SEQUENCE
ENDIF
dbSelectArea( nSaved )
IF lOk
RETURN cTempFile
ENDIF
RETURN ""
METHOD DropTempIndex( nWA, cTempFile ) CLASS TSqlIndex
LOCAL nSaved
IF Empty( cTempFile )
RETURN NIL
ENDIF
nSaved := Select()
dbSelectArea( nWA )
ordSetFocus( 0 )
dbClearIndex()
dbSelectArea( nSaved )
FErase( cTempFile )
RETURN NIL
METHOD ExtractStrWidth( cExpr ) CLASS TSqlIndex
LOCAL nComma, nParen, cWidth

1173
_FiveSql2/src/TSqlParser_orig.prg
View File

File diff suppressed because it is too large Load Diff

82
_FiveSql2/test/bench_parser.prg
View File

@@ -1,82 +0,0 @@
/*
* bench_parser.prg — Parser performance benchmark
* Compares TSqlParser (original) vs TSqlParser2 (Pratt)
*
* Copyright (c) 2025-2026 Charles KWON (Charles KWON OhJun)
* All rights reserved.
*/
#include "FiveSqlDef.ch"
#include "hbclass.ch"
#define ITERS 10000
PROCEDURE Main()
LOCAL aSQL, i, j, t0, t1, oLex, aTokens, oP, h
LOCAL nT1, nT2, aAllTokens
aSQL := { ;
"SELECT name, salary FROM employees WHERE salary > 5000", ;
"SELECT e.name, o.product FROM employees e JOIN orders o ON e.id = o.emp_id WHERE o.amount > 100", ;
"SELECT dept, COUNT(*) AS cnt, AVG(salary) AS avg_sal FROM employees GROUP BY dept HAVING AVG(salary) > 5000 ORDER BY avg_sal DESC", ;
"SELECT name FROM employees WHERE id IN (SELECT emp_id FROM orders WHERE amount > 500)", ;
"SELECT name, salary, ROW_NUMBER() OVER (PARTITION BY dept ORDER BY salary DESC) AS rn FROM employees", ;
"SELECT name, CASE WHEN salary > 7000 THEN 'High' WHEN salary > 5000 THEN 'Mid' ELSE 'Low' END AS tier FROM employees WHERE dept = 'Engineering' OR (salary > 6000 AND mgr_id IS NOT NULL)", ;
"INSERT INTO employees (id, name, dept, salary) VALUES (99, 'Test', 'QA', 5000)", ;
"UPDATE employees SET salary = salary * 1.1 + 500 WHERE dept = 'Sales' AND salary BETWEEN 4000 AND 6000" ;
}
? "================================================================"
? " Parser Benchmark: TSqlParser vs TSqlParser2 (Pratt)"
? " Queries: " + hb_ntos( Len( aSQL ) ) + " Iterations: " + hb_ntos( ITERS )
? "================================================================"
?
/* Pre-tokenize all queries */
aAllTokens := {}
FOR i := 1 TO Len( aSQL )
oLex := TSqlLexer():New( aSQL[ i ] )
oLex:Tokenize()
AAdd( aAllTokens, oLex:GetTokens() )
NEXT
/* Benchmark TSqlParser (original) */
t0 := hb_MilliSeconds()
FOR j := 1 TO ITERS
FOR i := 1 TO Len( aSQL )
oP := TSqlParser():New( aAllTokens[ i ], {} )
h := oP:Parse()
NEXT
NEXT
t1 := hb_MilliSeconds()
nT1 := t1 - t0
? " TSqlParser (original) : " + Str( nT1, 8 ) + " ms (" + ;
Str( nT1 * 1000 / ( ITERS * Len( aSQL ) ), 6, 1 ) + " us/parse)"
/* Benchmark TSqlParser2 (Pratt) */
t0 := hb_MilliSeconds()
FOR j := 1 TO ITERS
FOR i := 1 TO Len( aSQL )
oP := TSqlParser2():New( aAllTokens[ i ], {} )
h := oP:Parse()
NEXT
NEXT
t1 := hb_MilliSeconds()
nT2 := t1 - t0
? " TSqlParser2 (Pratt) : " + Str( nT2, 8 ) + " ms (" + ;
Str( nT2 * 1000 / ( ITERS * Len( aSQL ) ), 6, 1 ) + " us/parse)"
?
IF nT2 > 0 .AND. nT1 > 0
IF nT1 > nT2
? " Pratt is " + Str( nT1 / nT2, 5, 2 ) + "x faster"
ELSEIF nT2 > nT1
? " Original is " + Str( nT2 / nT1, 5, 2 ) + "x faster"
ELSE
? " Same speed"
ENDIF
ENDIF
? "================================================================"
RETURN

98
_FiveSql2/test/test_parser_cmp.prg
View File

@@ -1,98 +0,0 @@
/*
* test_parser_cmp.prg — Compare AST output: TSqlParser vs TSqlParser2 (Pratt)
*
* Runs identical SQL through both parsers and verifies the AST hashes
* are structurally identical.
*
* Copyright (c) 2025-2026 Charles KWON (Charles KWON OhJun)
* All rights reserved.
*/
#include "FiveSqlDef.ch"
#include "hbclass.ch"
STATIC s_nPass := 0
STATIC s_nFail := 0
PROCEDURE Main()
? "================================================================"
? " TSqlParser vs TSqlParser2 (Pratt) — AST Comparison"
? "================================================================"
?
Cmp( "Simple SELECT", "SELECT name, salary FROM employees WHERE salary > 5000" )
Cmp( "SELECT *", "SELECT * FROM employees" )
Cmp( "JOIN", "SELECT e.name, o.product FROM employees e JOIN orders o ON e.id = o.emp_id" )
Cmp( "LEFT JOIN", "SELECT e.name, o.product FROM employees e LEFT JOIN orders o ON e.id = o.emp_id" )
Cmp( "GROUP BY", "SELECT dept, COUNT(*) AS cnt FROM employees GROUP BY dept" )
Cmp( "HAVING", "SELECT dept, AVG(salary) AS avg_sal FROM employees GROUP BY dept HAVING AVG(salary) > 6000" )
Cmp( "ORDER BY DESC", "SELECT name, salary FROM employees ORDER BY salary DESC, name ASC" )
Cmp( "DISTINCT + TOP", "SELECT DISTINCT TOP 5 dept FROM employees" )
Cmp( "Subquery IN", "SELECT name FROM employees WHERE id IN (SELECT emp_id FROM orders)" )
Cmp( "Subquery NOT IN", "SELECT name FROM employees WHERE id NOT IN (1, 2, 3)" )
Cmp( "BETWEEN", "SELECT name FROM employees WHERE salary BETWEEN 5000 AND 8000" )
Cmp( "NOT BETWEEN", "SELECT name FROM employees WHERE salary NOT BETWEEN 1000 AND 2000" )
Cmp( "LIKE ESCAPE", "SELECT name FROM products WHERE name LIKE '%10!%%' ESCAPE '!'" )
Cmp( "IS NULL", "SELECT name FROM employees WHERE mgr_id IS NULL" )
Cmp( "IS NOT NULL", "SELECT name FROM employees WHERE mgr_id IS NOT NULL" )
Cmp( "OR + AND", "SELECT * FROM employees WHERE dept = 'Sales' OR (salary > 7000 AND mgr_id = 0)" )
Cmp( "NOT", "SELECT * FROM employees WHERE NOT (salary < 5000)" )
Cmp( "Arithmetic", "SELECT name, salary * 12 + 1000 AS annual FROM employees" )
Cmp( "Unary minus", "SELECT -salary FROM employees" )
Cmp( "String concat", "SELECT name || ' - ' || dept FROM employees" )
Cmp( "CASE WHEN", "SELECT name, CASE WHEN salary > 7000 THEN 'High' WHEN salary > 5000 THEN 'Mid' ELSE 'Low' END AS tier FROM employees" )
Cmp( "Nested function", "SELECT UPPER(SUBSTR(name, 1, 3)) FROM employees" )
Cmp( "COUNT(*)", "SELECT COUNT(*) FROM employees" )
Cmp( "Window ROW_NUMBER", "SELECT name, ROW_NUMBER() OVER (ORDER BY salary DESC) AS rn FROM employees" )
Cmp( "Window PARTITION", "SELECT name, RANK() OVER (PARTITION BY dept ORDER BY salary DESC) AS rnk FROM employees" )
Cmp( "UNION ALL", "SELECT name FROM employees WHERE dept = 'Sales' UNION ALL SELECT name FROM employees WHERE dept = 'HR'" )
Cmp( "INSERT", "INSERT INTO employees (id, name) VALUES (99, 'Test')" )
Cmp( "UPDATE", "UPDATE employees SET salary = 9999 WHERE id = 1" )
Cmp( "DELETE", "DELETE FROM employees WHERE id = 99" )
Cmp( "CTE simple", "WITH top_e AS (SELECT name FROM employees WHERE salary > 7000) SELECT * FROM top_e" )
?
? "================================================================"
? " Pass: " + hb_ntos( s_nPass ) + "/" + hb_ntos( s_nPass + s_nFail )
IF s_nFail == 0
? " ALL AST OUTPUTS IDENTICAL"
ENDIF
? "================================================================"
RETURN
STATIC FUNCTION Cmp( cLabel, cSQL )
LOCAL oLex, aTokens, oP1, oP2, h1, h2
LOCAL cAST1, cAST2
/* Tokenize once */
oLex := TSqlLexer():New( cSQL )
oLex:Tokenize()
aTokens := oLex:GetTokens()
/* Parse with TSqlParser (original) */
oP1 := TSqlParser():New( AClone( aTokens ), {} )
h1 := oP1:Parse()
/* Parse with TSqlParser2 (Pratt) */
oP2 := TSqlParser2():New( AClone( aTokens ), {} )
h2 := oP2:Parse()
/* Serialize both ASTs for comparison */
cAST1 := hb_ValToExp( h1 )
cAST2 := hb_ValToExp( h2 )
IF cAST1 == cAST2
s_nPass++
? " PASS: " + cLabel
ELSE
s_nFail++
? " FAIL: " + cLabel
? " P1: " + Left( cAST1, 120 )
? " P2: " + Left( cAST2, 120 )
ENDIF
RETURN NIL

9
hbrt/class.go
View File

@@ -298,7 +298,14 @@ func (t *Thread) tryBinaryOp(op int) bool {
if !a.IsObject() {
return false
}
cls := GetClass(a.AsArray().Class)
// AsArray can return nil if the ptr field is unset despite an object
// tag — a corrupted Value that would otherwise crash at `.Class`.
// Guard defensively; correct construction paths never hit this.
arr := a.AsArray()
if arr == nil {
return false
}
cls := GetClass(arr.Class)
if cls == nil || cls.Operators[op] == nil {
return false
}