Skip to content

Commit a9e0bed

Browse files
committed
fix: sum over empty [Float] returns Float 0.0, not Int 0
The in-memory knot_relation_sum had no way to know whether it was summing Floats or Ints, so it always returned Int(0) on empty — diverging from the SQL-pushdown path and corrupting show/toJson output. Fix: add knot_relation_sum_typed(db, f, rel, is_float) for fully-applied calls. Inference records the span and result type of each full sum f rel application, exports spans whose result is Float as SumFloatSpans. Codegen special-cases fully-applied sum to call knot_relation_sum_typed with the is_float flag from that span. The old 3-arg knot_relation_sum keeps its Int 0 fallback for first-class/partially-applied sum (no static type at the call site). 5 runtime tests + 1 end-to-end test. Full workspace green.
1 parent 8c02c31 commit a9e0bed

16 files changed

Lines changed: 290 additions & 19 deletions

crates/knot-compiler/src/codegen.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,11 @@ pub struct Codegen {
311311
// here, so the string from inference is the only carrier of the unit into
312312
// the emitted code.
313313
show_unit_strings: crate::infer::ShowUnitStrings,
314+
315+
// Spans of full `sum f rel` calls whose result is a Float. Passed to the
316+
// runtime as `is_float` so an EMPTY relation sums to `Float 0.0` rather
317+
// than `Int 0`. See `infer::SumFloatSpans`.
318+
sum_float_spans: crate::infer::SumFloatSpans,
314319
}
315320

316321
/// A top-level constant declared as a signature with no body
@@ -609,6 +614,7 @@ pub fn compile(
609614
elem_pushdown_ok: &crate::infer::ElemPushdownOk,
610615
trait_call_targets: &crate::infer::TraitCallTargets,
611616
show_unit_strings: &crate::infer::ShowUnitStrings,
617+
sum_float_spans: &crate::infer::SumFloatSpans,
612618
compile_time_overrides: &HashMap<String, String>,
613619
) -> Result<Vec<u8>, Vec<knot::diagnostic::Diagnostic>> {
614620
crate::stack::grow(|| {
@@ -624,6 +630,7 @@ pub fn compile(
624630
elem_pushdown_ok,
625631
trait_call_targets,
626632
show_unit_strings,
633+
sum_float_spans,
627634
compile_time_overrides,
628635
)
629636
})
@@ -642,6 +649,7 @@ fn compile_inner(
642649
elem_pushdown_ok: &crate::infer::ElemPushdownOk,
643650
trait_call_targets: &crate::infer::TraitCallTargets,
644651
show_unit_strings: &crate::infer::ShowUnitStrings,
652+
sum_float_spans: &crate::infer::SumFloatSpans,
645653
compile_time_overrides: &HashMap<String, String>,
646654
) -> Result<Vec<u8>, Vec<knot::diagnostic::Diagnostic>> {
647655
let mut cg = Codegen::new();
@@ -677,6 +685,7 @@ fn compile_inner(
677685
cg.from_json_targets = from_json_targets.clone();
678686
cg.elem_pushdown_ok = elem_pushdown_ok.clone();
679687
cg.show_unit_strings = show_unit_strings.clone();
688+
cg.sum_float_spans = sum_float_spans.clone();
680689
cg.source_refinements = type_env.source_refinements.clone();
681690
for (name, fields) in &type_env.constructors {
682691
let field_strs: Vec<(String, String)> = fields
@@ -948,6 +957,7 @@ impl Codegen {
948957
from_json_targets: HashMap::new(),
949958
elem_pushdown_ok: crate::infer::ElemPushdownOk::default(),
950959
show_unit_strings: HashMap::new(),
960+
sum_float_spans: crate::infer::SumFloatSpans::new(),
951961
}
952962
}
953963

@@ -1160,6 +1170,7 @@ impl Codegen {
11601170
self.declare_rt("knot_relation_diff", &[p, p, p], &[p]);
11611171
self.declare_rt("knot_relation_inter", &[p, p, p], &[p]);
11621172
self.declare_rt("knot_relation_sum", &[p, p, p], &[p]);
1173+
self.declare_rt("knot_relation_sum_typed", &[p, p, p, types::I64], &[p]);
11631174
self.declare_rt("knot_relation_avg", &[p, p, p], &[p]);
11641175
self.declare_rt("knot_relation_min", &[p, p, p], &[p]);
11651176
self.declare_rt("knot_relation_max", &[p, p, p], &[p]);
@@ -6536,6 +6547,30 @@ impl Codegen {
65366547
);
65376548
}
65386549

6550+
// Special case: `sum f rel` that did not push down to SQL above — pass
6551+
// the statically inferred numeric type so an EMPTY relation sums to the
6552+
// right zero. The runtime otherwise takes the type from the summands,
6553+
// of which there are none, and returns `Int 0` even for a `[Float]`
6554+
// (the SQL pushdown paths pass the same `is_float` flag, derived from
6555+
// the column type). Inference records the span only when the result is
6556+
// a Float; a user-defined `sum` skips this and dispatches normally.
6557+
if let ast::ExprKind::Var(name) = &func_expr.node
6558+
&& name == "sum"
6559+
&& args.len() == 2
6560+
&& !user_shadows_special {
6561+
let f_val = self.compile_expr(builder, args[0], env, db);
6562+
let rel_val = self.compile_expr(builder, args[1], env, db);
6563+
let is_float = builder.ins().iconst(
6564+
types::I64,
6565+
self.sum_float_spans.contains(&expr.span) as i64,
6566+
);
6567+
return self.call_rt(
6568+
builder,
6569+
"knot_relation_sum_typed",
6570+
&[db, f_val, rel_val, is_float],
6571+
);
6572+
}
6573+
65396574
let compiled_args: Vec<Value> = args
65406575
.iter()
65416576
.map(|a| self.compile_arg_expr(builder, a, env, db))

crates/knot-compiler/src/infer.rs

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,13 @@ pub struct FromJsonTarget {
144144
/// Maps parseJson call-site spans to their resolved target info.
145145
pub type FromJsonTargets = HashMap<Span, FromJsonTarget>;
146146

147+
/// Spans of full `sum f rel` applications (including the `rel |> sum f` pipe
148+
/// form) whose result type is a Float. Codegen passes this as an `is_float`
149+
/// flag to the runtime, which needs it ONLY for an EMPTY relation: with no
150+
/// summands there is nothing to take the numeric type from, so `sum` over an
151+
/// empty `[Float]` would otherwise return `Int 0` instead of `Float 0.0`.
152+
pub type SumFloatSpans = HashSet<Span>;
153+
147154
/// Spans of `elem needle haystack` haystack arguments whose element type is a
148155
/// SQL-pushable scalar (peeling aliases & refined types). Codegen consults these
149156
/// sets to decide whether to push an `elem` down to SQL.
@@ -595,6 +602,10 @@ struct Infer {
595602
/// `monad_info` entry keyed by the call span so codegen can tell the
596603
/// runtime which applicative's `pure []` an EMPTY input must produce.
597604
traverse_calls: Vec<(Span, TyVar, TyVar)>,
605+
/// Full `sum f rel` applications: (call span, result type var).
606+
/// Post-inference, calls whose result is a Float land in `SumFloatSpans`
607+
/// so codegen can tell the runtime which zero an EMPTY relation sums to.
608+
sum_calls: Vec<(Span, TyVar)>,
598609

599610
/// Tracks `parseJson` application sites for compile-time FromJSON dispatch.
600611
/// Each entry records (app_span, return_type_var).
@@ -783,6 +794,7 @@ impl Infer {
783794
result_markers: Vec::new(),
784795
unify_depth: 0,
785796
traverse_calls: Vec::new(),
797+
sum_calls: Vec::new(),
786798
from_json_calls: Vec::new(),
787799
trait_call_vars: Vec::new(),
788800
show_calls: Vec::new(),
@@ -4974,6 +4986,16 @@ impl Infer {
49744986
self.traverse_calls.push((expr.span, *res_v, cont_v));
49754987
}
49764988

4989+
// Track full `sum f rel` applications: the resolved result type
4990+
// says whether this is a Float sum, which codegen hands to the
4991+
// runtime to pick the zero for an EMPTY relation (no summand
4992+
// there to infer the numeric type from).
4993+
if let ast::ExprKind::App { func: inner_f, .. } = &func.node
4994+
&& matches!(&inner_f.node, ast::ExprKind::Var(n) if n == "sum")
4995+
&& let Ty::Var(res_v) = &result_ty {
4996+
self.sum_calls.push((expr.span, *res_v));
4997+
}
4998+
49774999
// Track `elem needle haystack` haystack types for SQL pushdown.
49785000
// Curried: outer App's func is `App(Var("elem"), needle)`,
49795001
// outer App's arg is the haystack. Record only when the
@@ -5973,6 +5995,13 @@ impl Infer {
59735995
Box::new(result_ty.clone()),
59745996
);
59755997
self.unify(&rhs_ty, &fun_ty, span);
5998+
// `rel |> sum f` reaches codegen as an application carrying
5999+
// this pipe's span, so record it like the `sum f rel` form.
6000+
if let ast::ExprKind::App { func: inner_f, .. } = &rhs.node
6001+
&& matches!(&inner_f.node, ast::ExprKind::Var(n) if n == "sum")
6002+
&& let Ty::Var(res_v) = &result_ty {
6003+
self.sum_calls.push((span, *res_v));
6004+
}
59766005
result_ty
59776006
}
59786007
}
@@ -10378,6 +10407,24 @@ fn value_references_source_inner(
1037810407

1037910408
// ── Public API ────────────────────────────────────────────────────
1038010409

10410+
/// What `check` hands to the later passes: diagnostics, the inferred types
10411+
/// themselves, and the span-keyed facts codegen cannot re-derive on its own
10412+
/// (monad kinds, refine/parseJson targets, `elem` pushdown eligibility, `show`
10413+
/// units, `sum`'s numeric result type).
10414+
pub type CheckOutput = (
10415+
Vec<Diagnostic>,
10416+
MonadInfo,
10417+
TypeInfo,
10418+
LocalTypeInfo,
10419+
RefineTargets,
10420+
RefinedTypeInfoMap,
10421+
FromJsonTargets,
10422+
ElemPushdownOk,
10423+
TraitCallTargets,
10424+
ShowUnitStrings,
10425+
SumFloatSpans,
10426+
);
10427+
1038110428
/// Run type inference on a parsed module. Returns diagnostics,
1038210429
/// resolved monad info for desugared do-blocks, and inferred type info
1038310430
/// mapping declaration names to their display type strings.
@@ -10390,11 +10437,11 @@ fn value_references_source_inner(
1039010437
///
1039110438
/// Runs on a grown stack: a desugared `do` block nests one `__bind` per
1039210439
/// statement, and `infer_expr` recurses through every level.
10393-
pub fn check(module: &mut ast::Module) -> (Vec<Diagnostic>, MonadInfo, TypeInfo, LocalTypeInfo, RefineTargets, RefinedTypeInfoMap, FromJsonTargets, ElemPushdownOk, TraitCallTargets, ShowUnitStrings) {
10440+
pub fn check(module: &mut ast::Module) -> CheckOutput {
1039410441
crate::stack::grow(|| check_inner(module))
1039510442
}
1039610443

10397-
fn check_inner(module: &mut ast::Module) -> (Vec<Diagnostic>, MonadInfo, TypeInfo, LocalTypeInfo, RefineTargets, RefinedTypeInfoMap, FromJsonTargets, ElemPushdownOk, TraitCallTargets, ShowUnitStrings) {
10444+
fn check_inner(module: &mut ast::Module) -> CheckOutput {
1039810445
let mut infer = Infer::new();
1039910446

1040010447
// Phase 1: Collect type aliases, data types, constructors
@@ -10659,6 +10706,21 @@ fn check_inner(module: &mut ast::Module) -> (Vec<Diagnostic>, MonadInfo, TypeInf
1065910706
monad_info.entry(*span).or_insert(kind);
1066010707
}
1066110708

10709+
// Phase 5c: Resolve the numeric type of each full `sum f rel` call, keyed
10710+
// by the call span. Codegen passes it to the runtime, which uses it ONLY
10711+
// for the EMPTY-input result: no summands means no value to take the type
10712+
// from, and the zero must still be the one the program was checked against
10713+
// (`Float 0.0`, not `Int 0`).
10714+
let mut sum_float_spans = SumFloatSpans::new();
10715+
for (span, res_v) in &infer.sum_calls {
10716+
if matches!(
10717+
infer.apply(&Ty::Var(*res_v)).peel_alias(),
10718+
Ty::Float | Ty::FloatUnit(_)
10719+
) {
10720+
sum_float_spans.insert(*span);
10721+
}
10722+
}
10723+
1066210724
// Export refined type predicates for codegen
1066310725
let refined_type_info: RefinedTypeInfoMap = infer
1066410726
.refined_types
@@ -10722,7 +10784,7 @@ fn check_inner(module: &mut ast::Module) -> (Vec<Diagnostic>, MonadInfo, TypeInf
1072210784
let local_type_info = infer.extract_local_type_info();
1072310785
let elem_pushdown_ok = infer.elem_pushdown_ok.clone();
1072410786

10725-
(infer.to_diagnostics(), monad_info, type_info, local_type_info, refine_targets, refined_type_info, from_json_targets, elem_pushdown_ok, trait_call_targets, show_unit_strings)
10787+
(infer.to_diagnostics(), monad_info, type_info, local_type_info, refine_targets, refined_type_info, from_json_targets, elem_pushdown_ok, trait_call_targets, show_unit_strings, sum_float_spans)
1072610788
}
1072710789

1072810790

@@ -11158,14 +11220,14 @@ mod tests {
1115811220
fn check_src(src: &str) -> Vec<Diagnostic> {
1115911221
let mut module = parse(src);
1116011222
crate::desugar::desugar(&mut module);
11161-
let (diags, _monad_info, _type_info, _local_types, _refine_targets, _refined_types, _from_json, _elem_pushdown, _trait_calls, _show_units) = check(&mut module);
11223+
let (diags, _monad_info, _type_info, _local_types, _refine_targets, _refined_types, _from_json, _elem_pushdown, _trait_calls, _show_units, _sum_floats) = check(&mut module);
1116211224
diags
1116311225
}
1116411226

1116511227
fn type_info_for(src: &str) -> TypeInfo {
1116611228
let mut module = parse(src);
1116711229
crate::desugar::desugar(&mut module);
11168-
let (_diags, _monad_info, type_info, _local_types, _refine_targets, _refined_types, _from_json, _elem_pushdown, _trait_calls, _show_units) = check(&mut module);
11230+
let (_diags, _monad_info, type_info, _local_types, _refine_targets, _refined_types, _from_json, _elem_pushdown, _trait_calls, _show_units, _sum_floats) = check(&mut module);
1116911231
type_info
1117011232
}
1117111233

crates/knot-compiler/src/main.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ fn cmd_build(source_file: &str, output_override: Option<&std::path::Path>, overr
377377
let type_env = types::TypeEnv::from_module(&module);
378378

379379
// Type inference
380-
let (infer_diags, monad_info, type_info, _local_types, refine_targets, refined_types, from_json_targets, elem_pushdown_ok, trait_call_targets, show_unit_strings) = infer::check(&mut module);
380+
let (infer_diags, monad_info, type_info, _local_types, refine_targets, refined_types, from_json_targets, elem_pushdown_ok, trait_call_targets, show_unit_strings, sum_float_spans) = infer::check(&mut module);
381381
if !infer_diags.is_empty() {
382382
for diag in &infer_diags {
383383
eprintln!("{}", diag.render(&source, &filename));
@@ -440,7 +440,7 @@ fn cmd_build(source_file: &str, output_override: Option<&std::path::Path>, overr
440440
}
441441

442442
// Code generation
443-
let obj_bytes = match codegen::compile(&module, &type_env, source_file, &monad_info, &refine_targets, &refined_types, &from_json_targets, &type_info, &elem_pushdown_ok, &trait_call_targets, &show_unit_strings, overrides) {
443+
let obj_bytes = match codegen::compile(&module, &type_env, source_file, &monad_info, &refine_targets, &refined_types, &from_json_targets, &type_info, &elem_pushdown_ok, &trait_call_targets, &show_unit_strings, &sum_float_spans, overrides) {
444444
Ok(bytes) => bytes,
445445
Err(diags) => {
446446
for diag in &diags {

crates/knot-compiler/tests/regress_analysis_fixes.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ fn parse(src: &str) -> knot::ast::Module {
3434
fn check_src(src: &str) -> Vec<Diagnostic> {
3535
let mut module = parse(src);
3636
knot_compiler::desugar::desugar(&mut module);
37-
let (diags, _monad, _type_info, _local, _refine, _refined, _json, _elem, _trait_calls, _show_units) =
37+
let (diags, _monad, _type_info, _local, _refine, _refined, _json, _elem, _trait_calls, _show_units, _sum_floats) =
3838
knot_compiler::infer::check(&mut module);
3939
diags
4040
}

crates/knot-compiler/tests/regress_deep_do.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ fn deep_do_block_reaches_codegen_without_overflow() {
115115
knot_compiler::desugar::desugar(&mut module);
116116

117117
let type_env = knot_compiler::types::TypeEnv::from_module(&module);
118-
let (diags, monad_info, type_info, _local, refine_targets, refined, from_json, elem, trait_calls, show_units) =
118+
let (diags, monad_info, type_info, _local, refine_targets, refined, from_json, elem, trait_calls, show_units, sum_floats) =
119119
knot_compiler::infer::check(&mut module);
120120
assert!(errors(&diags).is_empty(), "{:?}", errors(&diags));
121121

@@ -131,6 +131,7 @@ fn deep_do_block_reaches_codegen_without_overflow() {
131131
&elem,
132132
&trait_calls,
133133
&show_units,
134+
&sum_floats,
134135
&std::collections::HashMap::new(),
135136
)
136137
.expect("codegen should succeed on a deep do block");

crates/knot-compiler/tests/regress_infer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ fn check_full(
2222
) -> (Vec<Diagnostic>, knot_compiler::infer::RefineTargets) {
2323
let mut module = parse(src);
2424
knot_compiler::desugar::desugar(&mut module);
25-
let (diags, _monad, _type_info, _local, refine_targets, _refined, _json, _elem, _trait_calls, _show_units) =
25+
let (diags, _monad, _type_info, _local, refine_targets, _refined, _json, _elem, _trait_calls, _show_units, _sum_floats) =
2626
knot_compiler::infer::check(&mut module);
2727
(diags, refine_targets)
2828
}

crates/knot-compiler/tests/regress_infer_fixes.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ fn parse(src: &str) -> knot::ast::Module {
2626
fn check_src(src: &str) -> Vec<Diagnostic> {
2727
let mut module = parse(src);
2828
knot_compiler::desugar::desugar(&mut module);
29-
let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls, _show_units) =
29+
let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls, _show_units, _sum_floats) =
3030
knot_compiler::infer::check(&mut module);
3131
diags
3232
}

crates/knot-compiler/tests/regress_infer_fixes2.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ fn check_src(src: &str) -> Vec<Diagnostic> {
4141
let mut module = parse(src);
4242
knot_compiler::base::inject_prelude(&mut module);
4343
knot_compiler::desugar::desugar(&mut module);
44-
let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls, _show_units) =
44+
let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls, _show_units, _sum_floats) =
4545
knot_compiler::infer::check(&mut module);
4646
diags
4747
}

crates/knot-compiler/tests/regress_infer_fixes3.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ fn check_src(src: &str) -> Vec<Diagnostic> {
2525
let mut module = parse(src);
2626
knot_compiler::base::inject_prelude(&mut module);
2727
knot_compiler::desugar::desugar(&mut module);
28-
let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls, _show_units) =
28+
let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls, _show_units, _sum_floats) =
2929
knot_compiler::infer::check(&mut module);
3030
diags
3131
}

crates/knot-compiler/tests/regress_infer_fixes4.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ fn check_src(src: &str) -> Vec<Diagnostic> {
2828
let mut module = parse(src);
2929
knot_compiler::base::inject_prelude(&mut module);
3030
knot_compiler::desugar::desugar(&mut module);
31-
let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls, _show_units) =
31+
let (diags, _monad, _type_info, _local, _targets, _refined, _json, _elem, _trait_calls, _show_units, _sum_floats) =
3232
knot_compiler::infer::check(&mut module);
3333
diags
3434
}

0 commit comments

Comments
 (0)