Repository navigation
fix(cmp): reject not over STR, SYM and GUID vectors - #1114
belowzeroff wants to merge 2 commits into
Conversation
not on a STR, SYM or GUID vector fell through to the atom rule and answered one boolean for the whole vector, while the compiled not rejects STR and GUID columns with a type error. Raise the same type error from the eager not, so the two paths agree. not has no defined meaning for the cells of these types, and a test already pins the compiled rejection of a symbol column. The compiled not over a SYM column took the numeric lane path and answered booleans unless the column held an empty symbol. Bail it to the typed fallback, as ordering comparisons on SYM already do, so it raises whatever the column holds. Atoms and the list recursion are unchanged.
There was a problem hiding this comment.
Source approval for this bounded NOT fix: complete feature diff and the direct/list-recursion/compiler-admission contracts read; immutable snapshots independently checked. Concrete STR/SYM/GUID vectors reject before scalar truthiness, including empty/slices; SYM compiler decline reaches the typed error rather than intern-ID arithmetic. Numeric/scalar paths and comparison admission are unchanged. No local candidate runtime/full-suite/current-base integration claim; author runs retain their scope.
Keeping Refs #976 is important even after #1074 unless broadcasting is independently resolved. My existing DEBUG ELF (same cmp.c blob as accepted ae8e5c0d, not a fresh accepted/candidate build) still gives scalar b8 values for (or 3 [0 2]) / (and 0 [0 2]): [true 'b8 false 'b8]. The issue expects broadcast vectors. This does not block this narrower NOT correction, but please do not treat #1103 + #1074 + this change as demonstrated closure of every #976 case. No new logical-operator policy chosen here.
Nov7 ordinary accepted-base import: checked head e0b8dc5, ordered parents [9f95b31,2d3b66b0145dd3bc677c688251eb753cd070e9f8], tree 8b226ab62663259820b4b85aff60656a840e861f. All four feature blobs unchanged; 27 disjoint accepted imports exact. Merge-tree and independent strict feature-patch application agree. Existing source approval carried, not a new candidate runtime/full-suite/CI or landing claim. #976 remains OPEN: broadcasting remains outside this fix, even after #1074; no automatic closing references. Evidence: review-nov7-0940-intake/1114/accepted-2d3-import/composition-proof.json.
Nov7 local small-shape validation on unchanged feature blobs at e0b8dc5: compatible DEBUG ASan/UBSan compositions replacing cmp/expr TUs, not fresh whole-parent/new-head builds. Two original observer children check STR/SYM/GUID slices at N0/1/2 plus eight unchanged atom/list/numeric controls. Two additional children use flat, unshared SYM columns at N0/1/2/3 and a null-containing case; these actually reproduce the old compiled BOOL lane at N1/2/3 and its new TYPE result, not merely an already-falling-back slice. Both studies independently sealed; no sanitizer diagnostics, LSan off. No sort queries were used; the selected late-head counter stayed zero. The shared private sort TU was common to both compositions. (Scope wording clarified: that counter is not instrumentation of every function in sort.c.)
Empty WHERE skips evaluation and returns an empty table on both binaries for all three types; this is not a new defect or a promise to validate unevaluated rows. No full-suite/floor/performance/fault certification. Evidence: review-nov7-0940-intake/1114/runtime-smallshapes/parent-independent-proof.json and runtime-flat-sym/parent-independent-proof.json. CI/current-base/final landing guards remain separate; #976 broadcasting remains open.
How it looks from the user's side
Before (dev 28e4c29):
One boolean for a whole vector in the direct call, while the same
notinsidea query is a type error for STR and GUID, and for a SYM column only when it
holds an empty symbol; otherwise it answers booleans.
After:
The query form now raises for every STR, SYM and GUID column, whatever it
holds, and agrees with the direct call. Atoms and
lists of atoms are unchanged:
(not "")istrue,(not 'a)isfalse,(not (list "a" ""))is[false true].Cause
ray_not_fnis element-wise for BOOL, numeric and temporal vectors only. Anyother vector fell through to the atom rule and was tested as one value. The
compiled
notraised a type error for STR and GUID columns, but a SYM columnwas compiled as a numeric lane of intern ids and answered booleans unless it
held an empty symbol (which sent it to the typed fallback).
Fix
ray_not_fnraises the compiled path'stypeerror for a STR, SYM or GUIDvector (an empty one too, as the compiled path does). The query compiler no longer
accepts
notover a SYM column (the same bail it already has for orderingcomparisons on SYM), so the typed fallback raises for any SYM column;
equality and the other comparisons are unchanged. The issue allows
either element-wise or a type error;
nothas no defined meaning for thesecells, and
test/rfl/query/not_bool_column.rflalready pins the compiledrejection of a symbol column, so the eager path now matches it.
Tests
test/rfl/cmp/not.rfl: each type raisestypein the direct call, includinga slice and an empty vector; atoms are unchanged; the query form raises too.
Also a SYM column without empties, with empties, splayed with a sym file,
in
where:, and 80000 rows. Comparisons on a SYM column still work. The newcases fail without the fix.
Scope
This covers STR, SYM and GUID. The TIMESPAN part of the issue is in #1074.
The and/or part was fixed by #1103. With #1074 this finishes #976, so
Closes #976is right once #1074 has landed; on its own this should sayRefs #976.