From a47edbd4662de8d805d837a3a2b92996610efa7c Mon Sep 17 00:00:00 2001 From: James Long Date: Wed, 7 Sep 2022 10:34:03 -0400 Subject: [PATCH] Route aggregate queries in transaction grouped mode through the correct layer to remove deleted transactions --- packages/loot-core/src/server/aql/compiler.js | 4 +-- .../src/server/aql/schema/executors.js | 32 ++++++++++++------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/packages/loot-core/src/server/aql/compiler.js b/packages/loot-core/src/server/aql/compiler.js index dfd4179..3686e1e 100644 --- a/packages/loot-core/src/server/aql/compiler.js +++ b/packages/loot-core/src/server/aql/compiler.js @@ -951,7 +951,7 @@ function isAggregateFunction(expr) { return true; } - return argExprs.find(ex => isAggregateFunction(ex)); + return !!argExprs.find(ex => isAggregateFunction(ex)); } export function isAggregateQuery(queryState) { @@ -963,7 +963,7 @@ export function isAggregateQuery(queryState) { return true; } - return queryState.selectExpressions.find(expr => { + return !!queryState.selectExpressions.find(expr => { if (typeof expr !== 'string') { let [name, value] = Object.entries(expr)[0]; return isAggregateFunction(value); diff --git a/packages/loot-core/src/server/aql/schema/executors.js b/packages/loot-core/src/server/aql/schema/executors.js index 4c06bcc..f38a492 100644 --- a/packages/loot-core/src/server/aql/schema/executors.js +++ b/packages/loot-core/src/server/aql/schema/executors.js @@ -86,18 +86,28 @@ async function execTransactionsGrouped( let { table: tableName, withDead } = queryState; let whereDead = withDead ? '' : `AND ${sql.from}.tombstone = 0`; + // Aggregate queries don't make sense for a grouped transactions + // query. We never should include both parent and children + // transactions as it would duplicate amounts and the final number + // would never make sense. In this case, switch back to the "inline" + // type where only non-parent transactions are considered if (isAggregateQuery(queryState)) { - let allSql = ` - SELECT ${sql.select} - FROM ${sql.from} - ${sql.joins} - ${sql.where} AND is_parent = 0 ${whereDead} - ${sql.groupBy} - ${sql.orderBy} - ${sql.limit != null ? `LIMIT ${sql.limit}` : ''} - ${sql.offset != null ? `OFFSET ${sql.offset}` : ''} - `; - return db.all(allSql); + let s = { ...sql }; + + // Modify the where to only include non-parents + s.where = `${s.where} AND ${s.from}.is_parent = 0`; + + // We also want to exclude deleted transactions. Normally we + // handle this manually down below, but now that we are doing a + // normal query we want to rely on the view. Unfortunately, SQL + // has already been generated so we can't easily change the view + // name here; instead, we change it and map it back to the name + // used elsewhere in the query. Ideally we'd improve this + if (!withDead) { + s.from = 'v_transactions_internal_alive v_transactions_internal'; + } + + return execQuery(queryState, state, s, params, outputTypes); } let rows;