Skip to content

Commit 0485cb2

Browse files
dfa1claude
andcommitted
fix(calcite): answer SUM over an all-null column as SQL NULL, not 0
The writer records an all-null zone's SUM as a sum-neutral 0 (validity placeholders are zero), so the zone-map fold cannot tell an all-null column from a genuine zero. The push-down rule therefore answered `SUM(col)` over an all-null column as 0, diverging from SQL (and the scan path), which yield NULL. Guard the SUM branch with the column's null count, mirroring the MIN/MAX provably-no-values check: emit the SQL NULL literal when the column is provably all-null (empty table, or nullCount == rows); keep a non-zero fold; and abandon a zero fold whose null count is unknown for a nullable column (a genuine 0 is then indistinguishable from all-null). A non-nullable column has no nulls, so any fold is the true sum. VortexTable.zoneSum now returns a ZoneSum(sum, nullCount, totalRows) record read in a single reader pass, so the null-aware SUM costs one file open rather than three. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 092c772 commit 0485cb2

4 files changed

Lines changed: 194 additions & 18 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12-
- The Calcite aggregate push-down rule now rewrites a whole-table `SUM(col)` to a single-row `Values` computed from the zone-map table (via `VortexTable.zoneSum`), so `SELECT SUM(col)` answers metadata-only with no data segment decoded — joining the existing `MIN`/`MAX`/`COUNT` push-down. It abandons to a normal scan when a zone carries no usable sum (no zone map, or an overflowed zone) or when a `WHERE` filters the aggregate. ([24b64b32](https://github.com/dfa1/vortex-java/commit/24b64b32))
12+
- The Calcite aggregate push-down rule now rewrites a whole-table `SUM(col)` to a single-row `Values` computed from the zone-map table (via `VortexTable.zoneSum`), so `SELECT SUM(col)` answers metadata-only with no data segment decoded — joining the existing `MIN`/`MAX`/`COUNT` push-down. `SUM` over an all-null (or empty) column answers the SQL `NULL`, and the rule abandons to a normal scan when a zone carries no usable sum (no zone map, or an overflowed zone) or when a `WHERE` filters the aggregate. ([24b64b32](https://github.com/dfa1/vortex-java/commit/24b64b32))
1313
- `ScanIterator.columnZoneStats(column)` surfaces per-zone min/max/sum/null-count from a column's `vortex.stats` zone-map table without decoding any data segment — the read side of aggregate push-down (ADR 0013 §6). `ArrayStats` gains a `sum` component, decoded from the zone-map table (where the Rust reference stores it too), so the Calcite adapter now answers `SUM`/`AVG` metadata-only when every zone carries a sum, falling back to a streaming scan only for columns without a zone map. ([05dd9204](https://github.com/dfa1/vortex-java/commit/05dd9204))
1414

1515
### Changed

calcite/src/main/java/io/github/dfa1/vortex/calcite/VortexAggregatePushDownRule.java

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,12 @@
3131
///
3232
/// Fires only when it can answer *every* aggregate from statistics: no `GROUP BY`, and each call
3333
/// is `COUNT(*)`, `COUNT(col)`, `MIN(col)`, `MAX(col)`, or `SUM(col)` over a numeric column. `SUM`
34-
/// folds the per-zone `SUM` rows via [VortexTable#zoneSum(String)]; it abandons (falling back to
35-
/// the scan) for a column whose zone-map table cannot answer it — no zone map, or an overflowed
36-
/// zone. Anything else (a grouped aggregate, `MIN` on a non-numeric column, `AVG` that was not
37-
/// reduced to `SUM`/`COUNT`) leaves the plan untouched for the normal scan path.
34+
/// folds the per-zone `SUM` rows via [VortexTable#zoneSum(String)]; it emits the SQL `NULL` of an
35+
/// all-null (or empty) column, and abandons (falling back to the scan) for a column whose zone-map
36+
/// table cannot answer it — no zone map, an overflowed zone, or a zero fold whose null count is
37+
/// unknown (a genuine zero is indistinguishable from all-null). Anything else (a grouped aggregate,
38+
/// `MIN` on a non-numeric column, `AVG` that was not reduced to `SUM`/`COUNT`) leaves the plan
39+
/// untouched for the normal scan path.
3840
// Calcite 1.40 removed RelRule.Config.EMPTY; the modern RelRule.Config path requires the
3941
// Immutables annotation processor. The classic operand() constructor is deprecated but fully
4042
// supported and far lighter for a single adapter rule — suppression is localized and justified.
@@ -163,14 +165,30 @@ private static RexLiteral evaluate(AggregateCall agg, RelDataType outType, Vorte
163165
if (col == null) {
164166
yield null;
165167
}
166-
// Fold the per-zone SUM rows (metadata-only). A null fold means a zone carries no
167-
// usable sum (no zone map, or an overflowed zone) — abandon so the scan computes it.
168-
// It also covers SQL's empty/all-null SUM = NULL: zero zones fold to null and the
169-
// scan path produces the NULL literal, so the rule need not special-case it.
170-
Number sum = table.zoneSum(col);
168+
// Fold the per-zone SUM rows (metadata-only), with the null and row counts read in
169+
// the same pass. A null fold means a zone carries no usable sum (no zone map, or an
170+
// overflowed zone) — abandon so the scan computes it.
171+
VortexTable.ZoneSum zoneSum = table.zoneSum(col);
172+
Number sum = zoneSum.sum();
171173
if (sum == null) {
172174
yield null;
173175
}
176+
// SQL SUM over zero non-null rows is NULL, not 0 — but the writer records an all-null
177+
// zone's sum as a sum-neutral 0 (validity placeholders are zero), so the fold cannot
178+
// tell an all-null column from a genuine 0. Resolve it from the null count:
179+
// - provably all-null (empty table, or nullCount == rows) → emit the NULL literal;
180+
// - fold is a non-zero value → at least one non-null row, the number is correct;
181+
// - fold is 0 but the null count is unknown for a nullable column → ambiguous
182+
// (real 0 vs all-null), so abandon and let the scan decide.
183+
// A non-nullable column has no nulls, so any fold is the true sum.
184+
Long nullCount = zoneSum.nullCount();
185+
long total = zoneSum.totalRows();
186+
if (total == 0 || (nullCount != null && nullCount == total)) {
187+
yield rexBuilder.makeNullLiteral(outType);
188+
}
189+
if (isZero(sum) && nullCount == null && isNullable(scanRowType, col)) {
190+
yield null;
191+
}
174192
yield numericLiteral(rexBuilder, sum, outType);
175193
}
176194
default -> null;
@@ -196,6 +214,16 @@ private static String resolveColumn(int aggInput, List<String> scanColumns, Proj
196214
return null;
197215
}
198216

217+
/// Returns whether a folded sum is numerically zero (`0L` or `0.0`) — the value an all-null zone
218+
/// records, indistinguishable from a genuine zero sum without a null count.
219+
private static boolean isZero(Number sum) {
220+
return switch (sum) {
221+
case Long l -> l == 0L;
222+
case Double d -> d == 0.0;
223+
default -> sum.doubleValue() == 0.0;
224+
};
225+
}
226+
199227
private static RexLiteral exact(RexBuilder rexBuilder, long value, RelDataType type) {
200228
return rexBuilder.makeExactLiteral(BigDecimal.valueOf(value), type);
201229
}

calcite/src/main/java/io/github/dfa1/vortex/calcite/VortexTable.java

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,18 +87,41 @@ public io.github.dfa1.vortex.reader.ArrayStats statsOf(String column) {
8787
}
8888
}
8989

90-
/// Folds the per-zone `SUM` statistics for `column` without decoding any data segment, or
91-
/// returns `null` when the zone-map table cannot answer the reduction — a column with no zone
92-
/// map, or a zone whose sum was not retained (e.g. an overflowed zone) — so the caller scans.
90+
/// The folded `SUM` of a column together with the two facts needed to interpret a zero fold:
91+
/// SQL `SUM` over zero non-null rows is `NULL`, but an all-null zone records a sum-neutral `0`,
92+
/// so `sum == 0` alone cannot tell an all-null column from a genuine zero. The `nullCount` and
93+
/// `totalRows` resolve it.
9394
///
94-
/// Integer columns fold into a [Long] (exact); floating columns into a [Double]. Used by the
95-
/// aggregate push-down rule to answer `SUM` (ADR 0013 §6).
95+
/// @param sum the folded column sum ([Long] for integer columns, [Double] for floating),
96+
/// or `null` when no zone carries a usable sum (no zone map, or an overflowed
97+
/// zone)
98+
/// @param nullCount the column's total null count from the zone map, or `null` if not recorded
99+
/// @param totalRows the total row count across all chunks
100+
public record ZoneSum(Number sum, Long nullCount, long totalRows) {
101+
}
102+
103+
/// Folds the per-zone `SUM` statistics for `column` without decoding any data segment, reading
104+
/// the fold, null count, and row count in a single pass over the open reader so the caller can
105+
/// answer `SUM` null-correctly. Used by the aggregate push-down rule (ADR 0013 §6).
106+
///
107+
/// Integer columns fold into a [Long] (exact); floating columns into a [Double]. The
108+
/// [ZoneSum#sum()] is `null` when the zone-map table cannot answer the reduction — a column
109+
/// with no zone map, or a zone whose sum was not retained (e.g. an overflowed zone).
96110
///
97111
/// @param column the numeric column name
98-
/// @return the column sum as a [Long] or [Double], or `null` if no zone carries a usable sum
99-
public Number zoneSum(String column) {
112+
/// @return the folded sum with the null and row counts needed to interpret a zero
113+
public ZoneSum zoneSum(String column) {
100114
try (VortexReader reader = VortexReader.open(file)) {
101-
return new ZoneReducer(reader).sum(column);
115+
Number sum = new ZoneReducer(reader).sum(column);
116+
Long nullCount = reader.columnStats()
117+
.getOrDefault(column, io.github.dfa1.vortex.reader.ArrayStats.empty()).nullCount();
118+
long total = 0;
119+
try (ScanIterator scan = reader.scan(ScanOptions.all())) {
120+
for (long c : scan.chunkRowCounts()) {
121+
total += c;
122+
}
123+
}
124+
return new ZoneSum(sum, nullCount, total);
102125
} catch (IOException e) {
103126
throw new UncheckedIOException("cannot read zone sum of " + file, e);
104127
}
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package io.github.dfa1.vortex.calcite;
2+
3+
import io.github.dfa1.vortex.core.model.DType;
4+
import io.github.dfa1.vortex.core.model.PType;
5+
import io.github.dfa1.vortex.writer.VortexWriter;
6+
import io.github.dfa1.vortex.writer.WriteOptions;
7+
import io.github.dfa1.vortex.writer.encode.NullableData;
8+
9+
import org.apache.calcite.avatica.util.Casing;
10+
import org.apache.calcite.plan.RelOptUtil;
11+
import org.apache.calcite.plan.hep.HepPlanner;
12+
import org.apache.calcite.plan.hep.HepProgram;
13+
import org.apache.calcite.plan.hep.HepProgramBuilder;
14+
import org.apache.calcite.rel.RelNode;
15+
import org.apache.calcite.rel.core.Values;
16+
import org.apache.calcite.rex.RexLiteral;
17+
import org.apache.calcite.schema.SchemaPlus;
18+
import org.apache.calcite.sql.SqlNode;
19+
import org.apache.calcite.sql.parser.SqlParser;
20+
import org.apache.calcite.tools.FrameworkConfig;
21+
import org.apache.calcite.tools.Frameworks;
22+
import org.apache.calcite.tools.Planner;
23+
import org.junit.jupiter.api.Test;
24+
import org.junit.jupiter.api.io.TempDir;
25+
26+
import java.io.IOException;
27+
import java.nio.channels.FileChannel;
28+
import java.nio.file.Path;
29+
import java.nio.file.StandardOpenOption;
30+
import java.util.List;
31+
import java.util.Map;
32+
33+
import static org.assertj.core.api.Assertions.assertThat;
34+
35+
/// Null-aware `SUM` push-down for [VortexAggregatePushDownRule]: the per-zone SUM of an all-null
36+
/// zone is recorded as a sum-neutral 0, so a naive fold would answer `SUM` over an all-null column
37+
/// as `0` instead of the SQL `NULL`. These tests pin the null-count guard that distinguishes the
38+
/// two — and prove a *genuine* zero sum (no nulls) still pushes down to the literal `0`.
39+
class AggregateSumNullTest {
40+
41+
@TempDir
42+
Path tmp;
43+
44+
/// Single nullable `I64` column `v`, written as one chunk (one zone) so the file carries exactly
45+
/// one per-zone SUM row. `values` holds the raw longs; `valid[i] == false` marks row `i` null.
46+
private SchemaPlus tableOf(long[] values, boolean[] valid) throws IOException {
47+
DType.Struct schema = new DType.Struct(
48+
List.of("v"), List.of(new DType.Primitive(PType.I64, true)), false);
49+
Path file = tmp.resolve("sum-nulls.vortex");
50+
// Large chunk so the whole column is one chunk; zone maps on so the SUM stat is emitted.
51+
WriteOptions opts = new WriteOptions(1024, true, 0.90, 0, false, false);
52+
try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE);
53+
var writer = VortexWriter.create(ch, schema, opts)) {
54+
writer.writeChunk(Map.of("v", new NullableData(values, valid)));
55+
}
56+
SchemaPlus root = Frameworks.createRootSchema(true);
57+
return root.add("vtx", new VortexSchema(Map.of("t", file)));
58+
}
59+
60+
@Test
61+
void sumOverAllNullColumn_rewritesToNullNotZero() throws Exception {
62+
// Given a column whose every row is null — SQL SUM is NULL, yet the writer records the zone
63+
// sum as a sum-neutral 0; the null-count guard must override the 0 fold
64+
SchemaPlus schema = tableOf(new long[]{0, 0, 0}, new boolean[]{false, false, false});
65+
66+
// When the rule runs over SUM(v)
67+
Values values = pushDown(schema, "select sum(v) from t");
68+
69+
// Then it answers from stats (a single-row Values, no scan) but the value is SQL NULL, not 0
70+
assertThat(values).isNotNull();
71+
RexLiteral sum = values.getTuples().getFirst().getFirst();
72+
assertThat(sum.isNull()).isTrue();
73+
}
74+
75+
@Test
76+
void sumOverGenuineZero_rewritesToZeroLiteral() throws Exception {
77+
// Given a column with no nulls whose values cancel to 0 (1 + -1) — the fold is 0 but the
78+
// null count is 0, so the guard must NOT mistake it for all-null; SUM is the literal 0
79+
SchemaPlus schema = tableOf(new long[]{1, -1}, new boolean[]{true, true});
80+
81+
// When the rule runs over SUM(v)
82+
Values values = pushDown(schema, "select sum(v) from t");
83+
84+
// Then the rewrite succeeds with an exact 0, not NULL and not abandoned
85+
assertThat(values).isNotNull();
86+
RexLiteral sum = values.getTuples().getFirst().getFirst();
87+
assertThat(sum.isNull()).isFalse();
88+
assertThat(sum.getValueAs(Long.class)).isZero();
89+
}
90+
91+
/// Optimizes `sql` with the push-down rules and returns the single-row [Values] the rewrite
92+
/// produced, asserting no scan or aggregate survived. Fails the test if the rule abandoned.
93+
private static Values pushDown(SchemaPlus schema, String sql) throws Exception {
94+
FrameworkConfig config = Frameworks.newConfigBuilder()
95+
.defaultSchema(schema)
96+
.parserConfig(SqlParser.config().withUnquotedCasing(Casing.UNCHANGED))
97+
.build();
98+
Planner planner = Frameworks.getPlanner(config);
99+
SqlNode parsed = planner.parse(sql);
100+
RelNode logical = planner.rel(planner.validate(parsed)).rel;
101+
HepProgram program = new HepProgramBuilder()
102+
.addRuleCollection(VortexAggregatePushDownRule.RULES)
103+
.build();
104+
HepPlanner hep = new HepPlanner(program);
105+
hep.setRoot(logical);
106+
RelNode optimized = hep.findBestExp();
107+
108+
String plan = RelOptUtil.toString(optimized);
109+
assertThat(plan).contains("LogicalValues").doesNotContain("TableScan").doesNotContain("Aggregate");
110+
return findValues(optimized);
111+
}
112+
113+
private static Values findValues(RelNode node) {
114+
if (node instanceof Values values) {
115+
return values;
116+
}
117+
for (RelNode input : node.getInputs()) {
118+
Values found = findValues(input);
119+
if (found != null) {
120+
return found;
121+
}
122+
}
123+
return null;
124+
}
125+
}

0 commit comments

Comments
 (0)