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

feat(pp,rtl): pre-release accuracy round (Wave 3)

Browse Source 
Four audit findings around correctness/consistency in std.ch and the
SORT/UPDATE/TOTAL handlers:

  * #13: TOTAL/UPDATE key idiom inconsistency documented as inherent.
    TOTAL evaluates `<key>` only in the source workarea so verbatim
    `<{key}>` (alias-qualified or `_FIELD->`-prefixed by the user)
    works. UPDATE evaluates the same block in BOTH master and detail
    context, so it must wrap as `_FIELD-><key>` to dispatch to
    whichever WA is selected at eval time. The two rules look alike
    but their evaluation contexts differ — also documented in
    std.ch alongside both rules so the asymmetry isn't a surprise.
    Plus: TOTAL TO and ON are now mandatory (matching the COUNT/
    UPDATE pattern from Wave 1) — bare TOTAL would have produced
    broken syntax via the unconditional `<(f)>`/`<{key}>` template
    references.

  * #15/#16: SDF / DELIMITED variants of COPY and TO PRINTER /
    TO FILE variants of LIST / DISPLAY are now matched by stub
    rules (placed *before* the regular rules so they win) that
    expand to a new `__dbNotImpl(reason)` RTL primitive raising a
    clear `&hbrt.HbError`. BEGIN SEQUENCE / RECOVER catches the
    panic, so callers get a real error instead of the previous
    silent dispatch-to-regular-DBF-copy.

  * #19: SORT /C (case-insensitive) now actually folds case before
    the string compare, instead of being silently treated as
    ascending. Suffix parser also rebuilt as a multi-letter scanner
    so `name/CD`, `name/DC`, `name/C/D`, `name/D/C` all parse the
    same way — combine /C and /D freely. Unknown suffix letters
    (e.g., `name/X`) leave the suffix attached to the field name
    so a stray slash in user input doesn't get silently mangled
    into a broken field reference.

  * #27 SET DELETED: verified with a regression test that
    `SET DELETED ON` causes COUNT/COPY (and by extension
    SORT/TOTAL/JOIN/UPDATE — all of which iterate via Area.Skip)
    to skip rows marked deleted. The filtering is implemented at
    the workarea level (skipFilter in dbf.go honors hbrdd.IsSetDeleted)
    so no RTL changes were needed; this commit just adds the
    coverage so the behavior doesn't silently regress.

Gates green:
  go test ./...      : PASS
  FiveSql2 SQL:1999  : 43/43
  Harbour compat     : 56/56

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
CharlesKWON
2026-05-01 08:01:42 +09:00
parent f30704a854
commit 5b1d3fb32f
3 changed files with 103 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
compiler/pp/std.ch
View File

@@ -74,8 +74,15 @@
/* --- bulk record export ---
COPY TO copies visible records of the current workarea into a fresh
DBF. FIELDS/FOR/WHILE/NEXT/RECORD/REST work as in Harbour. SDF and
DELIMITED variants stay as silent no-ops in the parser until their
backends land. */
DELIMITED variants are not implemented; the matching rules below
raise a clear runtime error so callers don't quietly get a regular
DBF copy when they asked for an SDF dump. Order matters: the SDF /
DELIMITED rules must come before the regular COPY rule. */
#command COPY [TO <(f)>] [FIELDS <fields,...>] SDF [<*tail*>] => ;
__dbNotImpl("COPY TO ... SDF")
#command COPY [TO <(f)>] [FIELDS <fields,...>] DELIMITED [<*tail*>] => ;
__dbNotImpl("COPY TO ... DELIMITED")
#command COPY [TO <(f)>] [FIELDS <fields,...>] ;
[FOR <for>] [WHILE <while>] [NEXT <next>] ;
[RECORD <rec>] [<rest:REST>] [ALL] => ;
@@ -94,8 +101,19 @@
/* --- console output ---
LIST emits every record matching the filter; DISPLAY without ALL
shows just the current record. Both share __dbList — lAll
distinguishes them. TO PRINTER / TO FILE accepted but unused;
stdout is the only sink for now. */
distinguishes them. TO PRINTER / TO FILE redirection is not yet
implemented; the stub rules below surface a clear error rather
than silently sending output to stdout when a printer/file was
requested. Order matters: more specific rules first. */
#command LIST [<v,...>] TO PRINTER [<*tail*>] => ;
__dbNotImpl("LIST ... TO PRINTER")
#command LIST [<v,...>] TO FILE <(f)> [<*tail*>] => ;
__dbNotImpl("LIST ... TO FILE")
#command DISPLAY [<v,...>] TO PRINTER [<*tail*>] => ;
__dbNotImpl("DISPLAY ... TO PRINTER")
#command DISPLAY [<v,...>] TO FILE <(f)> [<*tail*>] => ;
__dbNotImpl("DISPLAY ... TO FILE")
#command LIST [<v,...>] [<off:OFF>] ;
[FOR <for>] [WHILE <while>] [NEXT <next>] ;
[RECORD <rec>] [<rest:REST>] [ALL] => ;
@@ -114,10 +132,14 @@
must already be sorted/indexed on the key for the grouping to
produce one row per distinct value.
Note for callers: Five doesn't auto-resolve a bare identifier
inside a code block to the current workarea's field. Write the
key alias-qualified — `ON src->dept` rather than `ON dept`. */
#command TOTAL [TO <(f)>] [ON <key>] [FIELDS <fields,...>] ;
Note on key syntax — TOTAL evaluates `<key>` only in the source
workarea, so `<{key}>` (verbatim blockify) is enough; user can
write `ON src->dept` (alias-qualified) or `ON _FIELD->dept`
(current-area). UPDATE FROM evaluates the key block in BOTH
master and detail context and therefore needs `_FIELD->`-wrapped
bare keys instead — the two rules look superficially similar but
their evaluation contexts differ. */
#command TOTAL TO <(f)> ON <key> [FIELDS <fields,...>] ;
[FOR <for>] [WHILE <while>] [NEXT <next>] ;
[RECORD <rec>] [<rest:REST>] [ALL] => ;
__dbTotal( <(f)>, <{key}>, { <(fields)> }, ;

78
hbrtl/database.go
View File

@@ -770,6 +770,24 @@ func rtlDbAverage(t *hbrt.Thread) {
t.RetDouble(sum/float64(n), 10, 2)
}
// rtlDbNotImpl raises a runtime error explaining which xBase clause
// the user invoked that Five doesn't yet implement. std.ch routes
// SDF / DELIMITED / TO PRINTER / TO FILE variants here so they fail
// loudly with a helpful diagnostic instead of being silently dropped.
func rtlDbNotImpl(t *hbrt.Thread) {
nParams := t.ParamCount()
t.Frame(nParams, 0)
defer t.EndProcFast()
what := "<unspecified>"
if nParams >= 1 && t.Local(1).IsString() {
what = t.Local(1).AsString()
}
panic(&hbrt.HbError{
Description: "xBase clause not implemented: " + what,
SubSystem: "BASE",
})
}
// rtlDbCopy implements __dbCopy(cFile, aFields, bFor, bWhile, nNext,
// xRec, lRest) — copy visible records from the current workarea into a
// freshly created DBF. Field projection: an empty/missing aFields
@@ -989,10 +1007,13 @@ func rtlDbSort(t *hbrt.Thread) {
return
}
// Parse sort-key spec: name[/D] entries → (fieldIdx, descending).
// Parse sort-key spec: name[/D|/A|/C[/D]|/D[/C]] entries.
// /D = descending, /A = ascending (default), /C = case-insensitive.
// /C and /D can be combined — `name/CD` or `name/DC` both ok.
type sortKey struct {
idx int
desc bool
caseFold bool
}
var keys []sortKey
if nParams >= 2 && t.Local(2).IsArray() {
@@ -1002,13 +1023,39 @@ func rtlDbSort(t *hbrt.Thread) {
continue
}
desc := false
// Suffix `/D` (descending), `/A` (ascending), `/C`
// (case-insensitive — treated as ascending).
if i := strings.LastIndexByte(s, '/'); i > 0 {
caseFold := false
// Strip suffixes — may be multiple, e.g. `name/C/D` or
// `name/CD`. Walk left from the last `/` repeatedly.
for {
i := strings.LastIndexByte(s, '/')
if i < 0 {
break
}
suffix := strings.ToUpper(strings.TrimSpace(s[i+1:]))
switch suffix {
case "D":
if suffix == "" {
break
}
done := false
for _, ch := range suffix {
switch ch {
case 'D':
desc = true
case 'A':
// ascending — explicit no-op
case 'C':
caseFold = true
default:
// Unknown letter — leave the suffix attached
// to the name and stop parsing so a field
// like `name/foo` doesn't get silently mangled.
done = true
}
if done {
break
}
}
if done {
break
}
s = strings.TrimSpace(s[:i])
}
@@ -1021,7 +1068,7 @@ func rtlDbSort(t *hbrt.Thread) {
}
}
if idx >= 0 {
keys = append(keys, sortKey{idx: idx, desc: desc})
keys = append(keys, sortKey{idx: idx, desc: desc, caseFold: caseFold})
}
}
}
@@ -1083,13 +1130,26 @@ func rtlDbSort(t *hbrt.Thread) {
// Sort if any keys were given. Stable so equal keys keep input
// order. Comparison is type-aware: numeric by AsNumDouble, date by
// AsNumInt julian, logical by truth, otherwise string.
// AsNumInt julian, logical by truth, otherwise string. /C keys
// fold case before string comparison.
if len(keys) > 0 && len(rows) > 1 {
less := func(i, j int) bool {
for _, k := range keys {
a := rows[i][k.idx]
b := rows[j][k.idx]
cmp := compareValues(a, b)
var cmp int
if k.caseFold && a.IsString() && b.IsString() {
sa := strings.ToLower(a.AsString())
sb := strings.ToLower(b.AsString())
switch {
case sa < sb:
cmp = -1
case sa > sb:
cmp = 1
}
} else {
cmp = compareValues(a, b)
}
if cmp == 0 {
continue
}

1
hbrtl/register.go
View File

@@ -205,6 +205,7 @@ func RegisterRTL(vm *hbrt.VM) {
hbrt.Sym("__DBTOTAL", hbrt.FsPublic, rtlDbTotal),
hbrt.Sym("__DBJOIN", hbrt.FsPublic, rtlDbJoin),
hbrt.Sym("__DBUPDATE", hbrt.FsPublic, rtlDbUpdate),
hbrt.Sym("__DBNOTIMPL", hbrt.FsPublic, rtlDbNotImpl),
hbrt.Sym("DBSETFILTER", hbrt.FsPublic, rtlDbSetFilter),
hbrt.Sym("DBCLEARFILTER", hbrt.FsPublic, rtlDbClearFilter),
hbrt.Sym("DBFILTER", hbrt.FsPublic, rtlDbFilter),