Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions src/ops/query.c
Original file line number Diff line number Diff line change
Expand Up @@ -1128,8 +1128,22 @@ ray_op_t* compile_expr_dag(ray_graph_t* g, ray_t* expr) {
* shortcuts may make the scan dead (count/max/projection paths), but
* execution must still reject the original column reference. */
ray_op_t* missing = ray_scan(g, ray_str_ptr(s));
/* `_dist` is exempt, but only where it can legitimately appear: a
* rerank produces it during execution, so it is absent from the
* operand table at compile time. Exempting it unconditionally let
* every symptom of an unknown column back in for that one name —
* a projection under `_dist` shifted the labels left, and an
* aggregate over it answered its neutral element — in queries with
* no nearest: clause at all. */
int64_t dist_sym = ray_sym_find("_dist", 5);
if (missing && expr->i64 != dist_sym) {
bool exempt = false;
if (missing && expr->i64 == dist_sym) {
for (uint32_t i = 0; i < g->node_count && !exempt; i++)
if (g->nodes[i].opcode == OP_ANN_RERANK ||
g->nodes[i].opcode == OP_KNN_RERANK)
exempt = true;
}
if (missing && !exempt) {
missing->flags |= OP_FLAG_INVALID_SCAN;
ray_op_ext_t* ext = find_ext(g, missing->id);
if (ext) ext->base.flags |= OP_FLAG_INVALID_SCAN;
Expand Down Expand Up @@ -10154,8 +10168,16 @@ ray_t* ray_select(ray_t** args, int64_t n) {
int j = 0;
for (int64_t i = 0; i + 1 < dict_n; i += 2) {
int64_t kid = dict_elems[i]->i64;
/* Every clause keyword has to be skipped here, not just
* most of them: this walk is positional, so a keyword it
* fails to skip consumes an output slot and every name
* after it lands on the wrong column. nearest: was
* missing, and it is counted out where the outputs are
* counted, so a query naming it before its outputs
* labelled them one position early. */
if (kid == from_id || kid == where_id || kid == by_id ||
kid == take_id || kid == asc_id || kid == desc_id) continue;
kid == take_id || kid == asc_id || kid == desc_id ||
kid == nearest_id) continue;
if (n_key_cols + j < ncols)
ray_table_set_col_name(result, n_key_cols + j, kid);
j++;
Expand Down
43 changes: 43 additions & 0 deletions test/rfl/regress/issue_428.rfl
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
;; A name a select cannot resolve must raise, and every clause keyword must
;; be skipped when the outputs are named.
;;
;; The general case was fixed with #414: an unknown column reference gets an
;; invalid-scan marker that ray_execute rejects. Two ways around it
;; survived, and both produced the #428 report — a projection under the
;; wrong label, and an aggregate answering its neutral element.
;;
;; 1. `_dist` was exempt from the marker unconditionally. The exemption is
;; needed, because a rerank produces that column during execution and it
;; is genuinely absent when the query compiles — but it applied just as
;; well to a query with no nearest: clause, where nothing will ever
;; produce it.
;;
;; 2. `nearest:` was missing from the skip list in the loop that names the
;; output columns. That walk is positional, so a keyword it fails to
;; skip eats an output slot and every name after it lands one column
;; early. It is counted out where the outputs are counted, which is why
;; only the naming disagreed.

(set _T (table [a b] (list [1 2 3] [4 5 6])))

;; ── 1. `_dist` with no rerank to produce it: an ordinary unknown name ────
(select {x: a d: _dist z: b from: _T}) !- schema
(select {d: _dist from: _T}) !- schema
(select {s: (sum _dist) from: _T}) !- schema
(select {c: (count _dist) from: _T}) !- schema
(select {m: (max _dist) from: _T}) !- schema
(select {f: (first _dist) from: _T}) !- schema
(select {s: (sum _dist) by: a from: _T}) !- schema
;; and the plain unknown name it used to be exempt from behaving like
(select {x: a y: nosuch z: b from: _T}) !- schema

;; ── 2. `_dist` where a rerank does produce it — still resolves, still
;; correct. The nearest row is id 1 at distance 0. ───────────────────
(set _TN (table [id Vec] (list [1 2 3 4] (list [1.0 0.0] [0.0 1.0] [1.0 1.0] [0.5 0.5]))))
(at (select {i: id d: _dist from: _TN nearest: (knn Vec [1.0 0.0]) take: 2}) 'i) -- [1 3]
(at (select {i: id d: _dist from: _TN nearest: (knn Vec [1.0 0.0]) take: 2}) 'd) -- [0.0 0.29]

;; ── 3. the same query with nearest: written BEFORE the outputs. Same
;; columns, same names — the keyword must not take one of the slots. ──
(at (select {from: _TN nearest: (knn Vec [1.0 0.0]) i: id d: _dist take: 2}) 'i) -- [1 3]
(at (select {from: _TN nearest: (knn Vec [1.0 0.0]) i: id d: _dist take: 2}) 'd) -- [0.0 0.29]
Loading