Skip to content

Commit 17d000e

Browse files
committed
no-mistakes(review): Guard spine any/all on fact rows; compile C# test
1 parent ece2f4d commit 17d000e

5 files changed

Lines changed: 88 additions & 23 deletions

File tree

‎server/csharp/MetaObjects.Codegen.Tests/ReportRowCodegenTests.cs‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -490,11 +490,25 @@ public void A_report_row_from_an_any_measure_with_no_of_is_a_plain_nullable_bool
490490
[Fact]
491491
public void An_any_measure_with_no_of_still_compiles_through_the_real_compiler()
492492
{
493-
var files = EmitAll(RunnerContext(LoadAnyMeasureModel()));
494-
Assert.True(files.ContainsKey("CallTotals.g.cs"));
495-
var tree = CSharpSyntaxTree.ParseText(files["CallTotals.g.cs"]);
496-
var diagnostics = tree.GetDiagnostics().Where(d => d.Severity == DiagnosticSeverity.Error).ToList();
497-
Assert.Empty(diagnostics);
493+
var ctx = RunnerContext(LoadAnyMeasureModel(), Config(includeNames: true));
494+
var files = new IGenerator[]
495+
{
496+
new EntityGenerator(), new DbContextGenerator(), new FilterAllowlistGenerator(), new NamesGenerator(),
497+
}
498+
.SelectMany(g => g.Generate(ctx)).ToList();
499+
Assert.Contains(files, f => f.Path == "CallTotals.g.cs");
500+
var trees = files
501+
.Select(f => CSharpSyntaxTree.ParseText(f.Content, new CSharpParseOptions(LanguageVersion.CSharp12), path: f.Path))
502+
.ToList();
503+
var comp = CSharpCompilation.Create(
504+
"report_any_rows_" + Guid.NewGuid().ToString("N"), trees,
505+
DbContextCompileTests.BuildReferences(),
506+
new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary));
507+
var errors = comp.GetDiagnostics()
508+
.Where(d => d.Severity == DiagnosticSeverity.Error)
509+
.Select(d => $"{d.Location.GetLineSpan().Path}: {d.Id}: {d.GetMessage()}")
510+
.ToList();
511+
Assert.True(errors.Count == 0, string.Join("\n", errors));
498512
}
499513

500514
// ---------------------------------------------------------------------

‎server/typescript/packages/codegen-ts/src/projection/report-ddl-emit.ts‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ function aggFunction(agg: ReportAggregate["agg"], d: ReportDialect): string {
4646
}
4747

4848
/** One aggregate (Table C): the bare aggregate, the condition by dialect, the cast, then the
49-
* Table D default around all of it. */
50-
function aggregate(a: ReportAggregate, d: ReportDialect): string {
49+
* Table D default around all of it. `factRef` is set under `@spine`: a column of @from that is
50+
* null only on the padding row a spine row with no facts gets. */
51+
function aggregate(a: ReportAggregate, d: ReportDialect, factRef?: string): string {
5152
const refs = a.refs.map((r) => ref(r, d));
5253
const c = a.filter === undefined ? undefined : cond(a.filter, d);
5354
const fn = aggFunction(a.agg, d);
@@ -73,7 +74,11 @@ function aggregate(a: ReportAggregate, d: ReportDialect): string {
7374
// (every row) passes". A comparison is a boolean on Postgres and 0 / 1 / NULL elsewhere, so
7475
// MAX / MIN of it is the OR / AND, and a row whose predicate is NULL is skipped as bool_or skips it.
7576
if (c === undefined) throw new Error(`report-ddl-emit: @agg '${a.agg}' has neither @of nor a condition.`);
76-
sql = `${fn}(${c})`;
77+
// Under @spine the padding row's condition can still be true (`col IS NULL`), so only fact
78+
// rows are aggregated, and a spine row with no facts reads null as every other measure does.
79+
if (factRef === undefined) sql = `${fn}(${c})`;
80+
else if (d === "postgres") sql = `${fn}(${c}) FILTER (WHERE ${ref(factRef, d)} IS NOT NULL)`;
81+
else sql = `${fn}(CASE WHEN ${ref(factRef, d)} IS NOT NULL THEN ${c} END)`;
7782
} else {
7883
const x = refs[0]!;
7984
const distinct = a.distinct ? "DISTINCT " : "";
@@ -92,20 +97,20 @@ interface RenderedColumn {
9297
readonly grouped: boolean;
9398
}
9499

95-
function column(c: ReportColumn, d: ReportDialect): RenderedColumn {
100+
function column(c: ReportColumn, d: ReportDialect, factRef: string | undefined): RenderedColumn {
96101
const alias = q(c.dbColAlias, d);
97102
switch (c.kind) {
98103
case "dimension":
99104
return { expr: ref(c.ref, d), alias, grouped: true };
100105
case "timeDimension":
101106
return { expr: truncateToGrain(ref(c.ref, d), c.grain, c.temporal, d), alias, grouped: true };
102107
case "aggregate":
103-
return { expr: aggregate(c.aggregate, d), alias, grouped: false };
108+
return { expr: aggregate(c.aggregate, d, factRef), alias, grouped: false };
104109
case "ratio": {
105110
// Each operand is its FULL Table C expression (condition and cast included), and its own
106111
// Table D default; the ratio's default wraps the whole quotient, which is always real.
107-
const num = aggregate(c.numerator, d);
108-
const den = aggregate(c.denominator, d);
112+
const num = aggregate(c.numerator, d, factRef);
113+
const den = aggregate(c.denominator, d, factRef);
109114
const top = d === "postgres" ? "NUMERIC" : d === "sqlite" ? "REAL" : undefined;
110115
const quotient = `${top === undefined ? num : `CAST(${num} AS ${top})`} / NULLIF(${den}, 0)`;
111116
const dv = c.defaultValue === undefined ? undefined : { value: c.defaultValue, real: true };
@@ -180,8 +185,15 @@ function spineFrom(spec: ReportViewSpec, depth: number, options: ReportEmitOptio
180185

181186
export function emitReportViewDdl(spec: ReportViewSpec, options: ReportEmitOptions): string {
182187
const d = options.dialect;
188+
// Under @spine, @from's side of the first hop's ON equality: non-null on every fact row the
189+
// join keeps, null on the padding row of a spine row with no facts.
190+
const first = spec.spineDepth === undefined ? undefined : spec.joinTree.joins[0];
191+
const factRef =
192+
first === undefined
193+
? undefined
194+
: `${spec.joinTree.baseAlias}.${first.referenceHolder === "source" ? first.fkColumn : first.pkColumn}`;
183195
// Rendered once: a dimension's SELECT expression is its GROUP BY term, so they cannot differ.
184-
const cols = spec.columns.map((c) => column(c, d));
196+
const cols = spec.columns.map((c) => column(c, d, factRef));
185197
const select = cols.map((c) => ` ${c.expr} AS ${c.alias}`).join(",\n");
186198
const groupBy = cols.filter((c) => c.grouped).map((c) => c.expr);
187199

‎server/typescript/packages/codegen-ts/test/projection/report-ddl-emit.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -977,6 +977,41 @@ describe("@agg any / all without @of, end to end", () => {
977977
});
978978
});
979979

980+
describe("@agg any / all without @of under @spine", () => {
981+
// A program with no weeks gets one padding row whose `label` is null, so `label IS NULL` is
982+
// true on it: only fact rows are aggregated, so that program reads null, not true.
983+
const unlabelled = (): Json[] => fitness([
984+
rpt("AnyUnlabelled", {
985+
"@from": "Week", "@spine": "Week.fkProgram", "@dimensions": ["programKey"], "@measures": ["anyUnlabelled", "allUnlabelled"],
986+
}, "v_any_unlabelled"),
987+
], [], [
988+
{ "measure.aggregate": { name: "anyUnlabelled", "@agg": "any", "@filter": { label: { isNull: true } } } },
989+
{ "measure.aggregate": { name: "allUnlabelled", "@agg": "all", "@filter": { label: { isNull: true } } } },
990+
]);
991+
992+
test("postgres: the condition is aggregated over the fact rows only", async () => {
993+
const sql = (await viewsOf(unlabelled(), "postgres"))["v_any_unlabelled"];
994+
expect(sql).toContain(` bool_or(w."label" IS NULL) FILTER (WHERE w."programId" IS NOT NULL) AS "anyUnlabelled",`);
995+
expect(sql).toContain(` bool_and(w."label" IS NULL) FILTER (WHERE w."programId" IS NOT NULL) AS "allUnlabelled"`);
996+
});
997+
998+
test("sqlite and mysql: the padding row's condition is NULL, which MAX / MIN skip", async () => {
999+
const lite = (await viewsOf(unlabelled(), "sqlite"))["v_any_unlabelled"];
1000+
expect(lite).toContain(`MAX(CASE WHEN w."programId" IS NOT NULL THEN w."label" IS NULL END) AS "anyUnlabelled"`);
1001+
expect(lite).toContain(`MIN(CASE WHEN w."programId" IS NOT NULL THEN w."label" IS NULL END) AS "allUnlabelled"`);
1002+
const my = (await viewsOf(unlabelled(), "mysql"))["v_any_unlabelled"];
1003+
expect(my).toContain("MAX(CASE WHEN w.`programId` IS NOT NULL THEN w.`label` IS NULL END) AS `anyUnlabelled`");
1004+
});
1005+
1006+
test("without @spine the SQL is unchanged", async () => {
1007+
const plain = fitness([
1008+
rpt("AnyUnlabelledPlain", { "@from": "Week", "@dimensions": ["programKey"], "@measures": ["anyUnlabelled"] }, "v_any_unlabelled_plain"),
1009+
], [], [{ "measure.aggregate": { name: "anyUnlabelled", "@agg": "any", "@filter": { label: { isNull: true } } } }]);
1010+
const sql = (await viewsOf(plain, "postgres"))["v_any_unlabelled_plain"];
1011+
expect(sql).toContain(` bool_or(w."label" IS NULL) AS "anyUnlabelled"`);
1012+
});
1013+
});
1014+
9801015
// ── A bare @of owner after @via resolves in the package of the entity @via reaches ─────
9811016

9821017
describe("a via dimension's bare @of owner across packages", () => {

‎server/typescript/packages/integration-tests/test/report-views-pg.test.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -450,9 +450,10 @@ const UNLABELLED_MODEL = JSON.stringify({ "metadata.root": { package: "acme", ch
450450
{ "dimension.attribute": { name: "programKey", "@of": "Program.id", "@via": "Week.fkProgram" } },
451451
{ "measure.aggregate": { name: "unlabelled", "@agg": "count", "@of": "Week.id", "@filter": { label: { isNull: true } } } },
452452
{ "measure.aggregate": { name: "unlabelledMinutes", "@agg": "sum", "@of": "Week.durationMinutes", "@filter": { label: { isNull: true } } } },
453+
{ "measure.aggregate": { name: "anyUnlabelled", "@agg": "any", "@filter": { label: { isNull: true } } } },
453454
] } },
454455
{ "object.report": { name: "UnlabelledWeeks", "@from": "Week", "@spine": "Week.fkProgram",
455-
"@dimensions": ["programKey"], "@measures": ["unlabelled", "unlabelledMinutes"], children: [
456+
"@dimensions": ["programKey"], "@measures": ["unlabelled", "unlabelledMinutes", "anyUnlabelled"], children: [
456457
{ "source.rdb": { "@kind": "view", "@view": "v_unlabelled_weeks" } } ] } },
457458
]}});
458459

@@ -619,11 +620,12 @@ describe("report views — inline models on real Postgres", () => {
619620

620621
await applyRaw(UNLABELLED_ROWS);
621622
// Program 3's null-extended row has a null label, and the count still reads 0: it counts
622-
// the fact's id, which is null there too. The sum of nothing is null (no @default).
623+
// the fact's id, which is null there too. The sum of nothing is null (no @default), and so
624+
// is an `any` over no facts, though `label IS NULL` holds on the null-extended row.
623625
expect(await select(`SELECT * FROM "v_unlabelled_weeks" ORDER BY "programKey"`)).toEqual([
624-
{ programKey: "1", unlabelled: "1", unlabelledMinutes: "60" },
625-
{ programKey: "2", unlabelled: "0", unlabelledMinutes: null },
626-
{ programKey: "3", unlabelled: "0", unlabelledMinutes: null },
626+
{ programKey: "1", unlabelled: "1", unlabelledMinutes: "60", anyUnlabelled: "t" },
627+
{ programKey: "2", unlabelled: "0", unlabelledMinutes: null, anyUnlabelled: "f" },
628+
{ programKey: "3", unlabelled: "0", unlabelledMinutes: null, anyUnlabelled: null },
627629
]);
628630
}, 60_000);
629631

‎server/typescript/packages/integration-tests/test/report-views-sqlite.test.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -417,9 +417,10 @@ const UNLABELLED_MODEL = JSON.stringify({ "metadata.root": { package: "acme", ch
417417
{ "dimension.attribute": { name: "programKey", "@of": "Program.id", "@via": "Week.fkProgram" } },
418418
{ "measure.aggregate": { name: "unlabelled", "@agg": "count", "@of": "Week.id", "@filter": { label: { isNull: true } } } },
419419
{ "measure.aggregate": { name: "unlabelledMinutes", "@agg": "sum", "@of": "Week.durationMinutes", "@filter": { label: { isNull: true } } } },
420+
{ "measure.aggregate": { name: "anyUnlabelled", "@agg": "any", "@filter": { label: { isNull: true } } } },
420421
] } },
421422
{ "object.report": { name: "UnlabelledWeeks", "@from": "Week", "@spine": "Week.fkProgram",
422-
"@dimensions": ["programKey"], "@measures": ["unlabelled", "unlabelledMinutes"], children: [
423+
"@dimensions": ["programKey"], "@measures": ["unlabelled", "unlabelledMinutes", "anyUnlabelled"], children: [
423424
{ "source.rdb": { "@kind": "view", "@view": "v_unlabelled_weeks" } } ] } },
424425
]}});
425426

@@ -544,11 +545,12 @@ describe("report views — inline model on real SQLite", () => {
544545
(10, 1, 'Week 1', 30), (11, 1, 'Week 2', 60), (12, 1, 'Week 2', 90), (13, 1, NULL, 60),
545546
(20, 2, 'Solo', 45)`);
546547
// Program 3's null-extended row has a null label, and the count still reads 0: it counts
547-
// the fact's id, which is null there too. The sum of nothing is null (no @default).
548+
// the fact's id, which is null there too. The sum of nothing is null (no @default), and so
549+
// is an `any` over no facts, though `label IS NULL` holds on the null-extended row.
548550
expect(await select(`SELECT * FROM "v_unlabelled_weeks" ORDER BY "programKey"`)).toEqual([
549-
{ programKey: 1, unlabelled: 1, unlabelledMinutes: 60 },
550-
{ programKey: 2, unlabelled: 0, unlabelledMinutes: null },
551-
{ programKey: 3, unlabelled: 0, unlabelledMinutes: null },
551+
{ programKey: 1, unlabelled: 1, unlabelledMinutes: 60, anyUnlabelled: 1 },
552+
{ programKey: 2, unlabelled: 0, unlabelledMinutes: null, anyUnlabelled: 0 },
553+
{ programKey: 3, unlabelled: 0, unlabelledMinutes: null, anyUnlabelled: null },
552554
]);
553555
});
554556

0 commit comments

Comments
 (0)