Skip to content

Commit

Permalink
fix: remove non projected members from order clause
Browse files Browse the repository at this point in the history
  • Loading branch information
retro committed Apr 8, 2024
1 parent 30c158e commit d92b553
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 8 deletions.
7 changes: 7 additions & 0 deletions .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"Aerosmith",
"Afrociberdelia",
"Alanis",
"alves",
"Arquivo",
"Audioslave",
"Battlestar",
Expand All @@ -19,12 +20,18 @@
"esbuild",
"fanout",
"Favourites",
"Fernanda",
"Fernandes",
"Finlandia",
"Galactica",
"gjuchault",
"Goyer",
"graphlib",
"hler",
"ilike",
"Leonie",
"Leppard",
"Mercier",
"Morissette",
"Negra",
"octocat",
Expand Down
71 changes: 71 additions & 0 deletions src/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,77 @@ await describe("semantic layer", async () => {
]);
});

await it("will remove non projected members from order clause", async () => {
const query = queryBuilder.buildQuery({
dimensions: [
"customers.customer_id",
"customers.full_name",
"invoice_lines.invoice_id",
],
metrics: [],
limit: 10,
order: { "invoices.invoice_date": "asc" },
});

const result = await client.query<InferSqlQueryResultType<typeof query>>(
query.sql,
query.bindings,
);

assert.deepEqual(result.rows, [
{
customers___customer_id: 1,
customers___full_name: "Lu�s Gon�alves",
invoice_lines___invoice_id: 121,
},
{
customers___customer_id: 1,
customers___full_name: "Lu�s Gon�alves",
invoice_lines___invoice_id: 316,
},
{
customers___customer_id: 1,
customers___full_name: "Lu�s Gon�alves",
invoice_lines___invoice_id: 143,
},
{
customers___customer_id: 1,
customers___full_name: "Lu�s Gon�alves",
invoice_lines___invoice_id: 195,
},
{
customers___customer_id: 1,
customers___full_name: "Lu�s Gon�alves",
invoice_lines___invoice_id: 327,
},
{
customers___customer_id: 1,
customers___full_name: "Lu�s Gon�alves",
invoice_lines___invoice_id: 98,
},
{
customers___customer_id: 1,
customers___full_name: "Lu�s Gon�alves",
invoice_lines___invoice_id: 382,
},
{
customers___customer_id: 2,
customers___full_name: "Leonie K�hler",
invoice_lines___invoice_id: 1,
},
{
customers___customer_id: 2,
customers___full_name: "Leonie K�hler",
invoice_lines___invoice_id: 219,
},
{
customers___customer_id: 2,
customers___full_name: "Leonie K�hler",
invoice_lines___invoice_id: 67,
},
]);
});

await it("can query one dimension and metric and filter by a different metric", async () => {
const query = queryBuilder.buildQuery({
dimensions: ["customers.customer_id"],
Expand Down
10 changes: 4 additions & 6 deletions src/lib/query-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import { z } from "zod";
import { Simplify } from "type-fest";
import { BaseDialect } from "./dialect/base.js";
import { buildQuery } from "./query-builder/build-query.js";
import { expandQueryToSegments } from "./query-builder/expand-query.js";
import { findOptimalJoinGraph } from "./query-builder/optimal-join-graph.js";
import { processQueryAndExpandToSegments } from "./query-builder/process-query-and-expand-to-segments.js";
import type { AnyRepository } from "./repository.js";

function isNonEmptyArray<T>(arr: T[]): arr is [T, ...T[]] {
Expand Down Expand Up @@ -95,12 +95,10 @@ export class QueryBuilder<
}

unsafeBuildQuery(payload: unknown, context: unknown) {
const query: AnyQuery = this.querySchema.parse(payload);
const parsedQuery: AnyQuery = this.querySchema.parse(payload);

const { referencedModels, segments } = expandQueryToSegments(
this.repository,
query,
);
const { query, referencedModels, segments } =
processQueryAndExpandToSegments(this.repository, parsedQuery);

const joinGraph = findOptimalJoinGraph(
this.repository.graph,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {

import { AnyRepository } from "../repository.js";

// biome-ignore lint/complexity/noExcessiveCognitiveComplexity: <explanation>
function analyzeQuery(repository: AnyRepository, query: AnyQuery) {
const allModels = new Set<string>();
const dimensionModels = new Set<string>();
Expand All @@ -15,10 +16,12 @@ function analyzeQuery(repository: AnyRepository, query: AnyQuery) {
const dimensionsByModel: Record<string, Set<string>> = {};
const projectedMetricsByModel: Record<string, Set<string>> = {};
const metricsByModel: Record<string, Set<string>> = {};
const allMemberNames = new Set<string>();

for (const dimension of query.dimensions || []) {
const modelName = repository.getDimension(dimension).model.name;
allModels.add(modelName);
allMemberNames.add(dimension);
dimensionModels.add(modelName);
dimensionsByModel[modelName] ||= new Set<string>();
dimensionsByModel[modelName]!.add(dimension);
Expand All @@ -29,6 +32,7 @@ function analyzeQuery(repository: AnyRepository, query: AnyQuery) {
for (const metric of query.metrics || []) {
const modelName = repository.getMetric(metric).model.name;
allModels.add(modelName);
allMemberNames.add(metric);
metricModels.add(modelName);
metricsByModel[modelName] ||= new Set<string>();
metricsByModel[modelName]!.add(metric);
Expand All @@ -47,6 +51,7 @@ function analyzeQuery(repository: AnyRepository, query: AnyQuery) {
const modelName = member.model.name;

allModels.add(modelName);
allMemberNames.add(filter.member);

if (member.isDimension()) {
// dimensionModels are used for join of query segments
Expand All @@ -65,6 +70,15 @@ function analyzeQuery(repository: AnyRepository, query: AnyQuery) {
}
}

const orderByWithoutNonProjectedMembers = Object.entries(
query.order ?? {},
).reduce<Record<string, "asc" | "desc">>((acc, [member, direction]) => {
if (allMemberNames.has(member)) {
acc[member] = direction ?? "asc";
}
return acc;
}, {});

return {
allModels,
dimensionModels,
Expand All @@ -73,6 +87,10 @@ function analyzeQuery(repository: AnyRepository, query: AnyQuery) {
projectedDimensionsByModel,
metricsByModel,
projectedMetricsByModel,
order:
Object.keys(orderByWithoutNonProjectedMembers).length > 0
? orderByWithoutNonProjectedMembers
: undefined,
};
}

Expand Down Expand Up @@ -210,7 +228,7 @@ function mergeQuerySegmentWithFilters(
};
}

export function expandQueryToSegments(
export function processQueryAndExpandToSegments(
repository: AnyRepository,
query: AnyQuery,
) {
Expand All @@ -232,7 +250,7 @@ export function expandQueryToSegments(
);

return {
query,
query: { ...query, order: queryAnalysis.order },
referencedModels: {
all: Array.from(queryAnalysis.allModels),
dimensions: Array.from(queryAnalysis.dimensionModels),
Expand Down

0 comments on commit d92b553

Please sign in to comment.