[pull] master from cube-js:master

packages/cubejs-schema-compiler/test/integration/postgres/calculated-measure-multi-fact.test.ts

@@ -0,0 +1,211 @@
+import { getEnv } from '@cubejs-backend/shared';
+import { PostgresQuery } from '../../../src/adapter/PostgresQuery';
+import { prepareJsCompiler } from '../../unit/PrepareCompiler';
+import { dbRunner } from './PostgresDBRunner';
+
+// Calculated measures (`type: number` over other measures of the same cube)
+// combined with a dimension reached through a hasMany join. The fan-out forces
+// every measure to pick a strategy: countDistinct survives row multiplication
+// and is aggregated in place, sum has to go through the keys subquery. A
+// calculated measure is neither - it is an expression over aggregates and can
+// only be evaluated once its components have been re-aggregated.
+describe('Calculated measure on the multi-fact path', () => {
+  jest.setTimeout(200000);
+
+  const { compiler, joinGraph, cubeEvaluator } = prepareJsCompiler(`
+cube(\`Payments\`, {
+  // id is TEXT on purpose: it is the operand that ends up in arithmetic when a
+  // calculated measure loses the aggregation around its components.
+  sql: \`
+    SELECT 'p1' AS id, 'SUCCESS' AS status, 100 AS amount, 'EUR' AS currency UNION ALL
+    SELECT 'p2' AS id, 'SUCCESS' AS status, 200 AS amount, 'EUR' AS currency UNION ALL
+    SELECT 'p3' AS id, 'DECLINED' AS status, 300 AS amount, 'EUR' AS currency UNION ALL
+    SELECT 'p4' AS id, 'SUCCESS' AS status, 400 AS amount, 'USD' AS currency
+  \`,
+
+  joins: {
+    Meta: {
+      relationship: \`hasMany\`,
+      sql: \`\${CUBE}.id = \${Meta}.payment_id\`,
+    },
+    Rates: {
+      relationship: \`belongsTo\`,
+      sql: \`\${CUBE}.currency = \${Rates}.currency\`,
+    },
+  },
+
+  measures: {
+    count: {
+      sql: \`id\`,
+      type: \`countDistinct\`,
+    },
+    successCount: {
+      sql: \`id\`,
+      type: \`countDistinct\`,
+      filters: [{ sql: \`\${CUBE}.status = 'SUCCESS'\` }],
+    },
+    totalAmount: {
+      sql: \`amount\`,
+      type: \`sum\`,
+    },
+    successAmount: {
+      sql: \`amount\`,
+      type: \`sum\`,
+      filters: [{ sql: \`\${CUBE}.status = 'SUCCESS'\` }],
+    },
+    // Needs a join to Rates, and sum is not immune to the Meta fan-out, so it
+    // takes the keys-subquery path.
+    convertedValue: {
+      sql: \`\${CUBE}.amount / nullif(\${Rates.fxRate}, 0)\`,
+      type: \`sum\`,
+    },
+    // Calculated measures over components of the same cube. The components
+    // differ in whether they survive row multiplication on their own:
+    // countDistinct does, sum does not.
+    successRate: {
+      sql: \`100.0 * \${successCount} / nullif(\${count}, 0)\`,
+      type: \`number\`,
+    },
+    successAmountRate: {
+      sql: \`100.0 * \${successAmount} / nullif(\${totalAmount}, 0)\`,
+      type: \`number\`,
+    },
+  },
+
+  dimensions: {
+    id: { sql: \`id\`, type: \`string\`, primaryKey: true },
+    status: { sql: \`status\`, type: \`string\` },
+  },
+});
+
+cube(\`Meta\`, {
+  // p1 carries two meta rows so grouping by Meta.value multiplies it.
+  sql: \`
+    SELECT 'm1' AS id, 'p1' AS payment_id, 'A' AS value UNION ALL
+    SELECT 'm1b' AS id, 'p1' AS payment_id, 'A' AS value UNION ALL
+    SELECT 'm2' AS id, 'p2' AS payment_id, 'A' AS value UNION ALL
+    SELECT 'm3' AS id, 'p3' AS payment_id, 'A' AS value UNION ALL
+    SELECT 'm4' AS id, 'p4' AS payment_id, 'B' AS value
+  \`,
+  dimensions: {
+    id: { sql: \`id\`, type: \`string\`, primaryKey: true },
+    paymentId: { sql: \`payment_id\`, type: \`string\` },
+    value: { sql: \`value\`, type: \`string\` },
+  },
+});
+
+cube(\`Rates\`, {
+  sql: \`
+    SELECT 'EUR' AS currency, 1.0 AS fx_rate UNION ALL
+    SELECT 'USD' AS currency, 2.0 AS fx_rate
+  \`,
+  dimensions: {
+    currency: { sql: \`currency\`, type: \`string\`, primaryKey: true },
+    fxRate: { sql: \`fx_rate\`, type: \`number\` },
+  },
+});
+    `);
+
+  async function runQuery(q) {
+    await compiler.compile();
+    const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, q);
+    return dbRunner.testQuery(query.buildSqlAndParams());
+  }
+
+  async function expectQueryToFail(q) {
+    await compiler.compile();
+    try {
+      const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, q);
+      await dbRunner.testQuery(query.buildSqlAndParams());
+    } catch (e: any) {
+      return e.message as string;
+    }
+    throw new Error('Expected the query to fail, but it succeeded');
+  }
+
+  it('calculated measure alone, grouped by a fan-out dimension', async () => {
+    expect(await runQuery({
+      measures: ['Payments.successRate'],
+      dimensions: ['Meta.value'],
+      order: [{ id: 'Meta.value' }],
+    })).toEqual([
+      { meta__value: 'A', payments__success_rate: '66.6666666666666667' },
+      { meta__value: 'B', payments__success_rate: '100.0000000000000000' },
+    ]);
+  });
+
+  it('calculated measure components, grouped by a fan-out dimension', async () => {
+    expect(await runQuery({
+      measures: ['Payments.successCount', 'Payments.count'],
+      dimensions: ['Meta.value'],
+      order: [{ id: 'Meta.value' }],
+    })).toEqual([
+      { meta__value: 'A', payments__success_count: '2', payments__count: '3' },
+      { meta__value: 'B', payments__success_count: '1', payments__count: '1' },
+    ]);
+  });
+
+  it('joined measure next to a distinct count, grouped by a fan-out dimension', async () => {
+    expect(await runQuery({
+      measures: ['Payments.convertedValue', 'Payments.count'],
+      dimensions: ['Meta.value'],
+      order: [{ id: 'Meta.value' }],
+    })).toEqual([
+      { meta__value: 'A', payments__converted_value: '600.0000000000000000', payments__count: '3' },
+      { meta__value: 'B', payments__converted_value: '200.0000000000000000', payments__count: '1' },
+    ]);
+  });
+
+  it('calculated measure next to a joined measure, grouped by a fan-out dimension', async () => {
+    const query = {
+      measures: ['Payments.successRate', 'Payments.convertedValue'],
+      dimensions: ['Meta.value'],
+      order: [{ id: 'Meta.value' }],
+    };
+
+    if (!getEnv('nativeSqlPlanner')) {
+      // The calculated measure is inlined into the ungrouped measure-join with
+      // the aggregation around its components removed, leaving the TEXT id
+      // column in arithmetic.
+      expect(await expectQueryToFail(query)).toContain('operator does not exist: numeric * text');
+      return;
+    }
+
+    expect(await runQuery(query)).toEqual([
+      { meta__value: 'A', payments__success_rate: '66.6666666666666667', payments__converted_value: '600.0000000000000000' },
+      { meta__value: 'B', payments__success_rate: '100.0000000000000000', payments__converted_value: '200.0000000000000000' },
+    ]);
+  });
+
+  it('calculated measure over sums next to a joined measure, grouped by a fan-out dimension', async () => {
+    const query = {
+      measures: ['Payments.successAmountRate', 'Payments.convertedValue'],
+      dimensions: ['Meta.value'],
+      order: [{ id: 'Meta.value' }],
+    };
+
+    if (!getEnv('nativeSqlPlanner')) {
+      // Types line up here, so the failure surfaces one step later: the
+      // calculated measure is projected without an aggregate and without being
+      // grouped.
+      expect(await expectQueryToFail(query)).toContain('must appear in the GROUP BY clause');
+      return;
+    }
+
+    expect(await runQuery(query)).toEqual([
+      { meta__value: 'A', payments__success_amount_rate: '50.0000000000000000', payments__converted_value: '600.0000000000000000' },
+      { meta__value: 'B', payments__success_amount_rate: '100.0000000000000000', payments__converted_value: '200.0000000000000000' },
+    ]);
+  });
+
+  it('calculated measure next to a joined measure, without a fan-out dimension', async () => {
+    expect(await runQuery({
+      measures: ['Payments.successRate', 'Payments.convertedValue'],
+      dimensions: ['Payments.status'],
+      order: [{ id: 'Payments.status' }],
+    })).toEqual([
+      { payments__status: 'DECLINED', payments__success_rate: '0.00000000000000000000', payments__converted_value: '300.0000000000000000' },
+      { payments__status: 'SUCCESS', payments__success_rate: '100.0000000000000000', payments__converted_value: '500.0000000000000000' },
+    ]);
+  });
+});

rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/processors/aggregate_multiplied_subquery.rs

@@ -9,9 +9,35 @@ use crate::physical_plan::{
 };
 use crate::physical_plan_builder::PhysicalPlanBuilder;
 use crate::planner::MeasureRenderModifier;
+use crate::planner::{AggregateWrap, MemberSymbol};
 use cubenativeutils::CubeError;
 use std::rc::Rc;
 
+/// The measure subquery renders measures without their aggregate for the select
+/// above to re-apply. A measure carrying none of its own has nothing to
+/// re-apply and would come out neither aggregated nor grouped.
+///
+/// Member expressions are left out on purpose: the SQL API builds them ad-hoc
+/// and their aggregation is not described by a measure kind.
+fn check_measures_survive_measure_subquery(measures: &[Rc<MemberSymbol>]) -> Result<(), CubeError> {
+    for measure in measures.iter() {
+        let Ok(symbol) = measure.as_measure() else {
+            continue;
+        };
+        if matches!(symbol.kind().aggregate_wrap(), AggregateWrap::PassThrough) {
+            return Err(CubeError::user(format!(
+                "{} has no aggregate of its own, so it cannot be re-aggregated over the \
+                 deduplicated rows this query needs - a measure of its group reaches another \
+                 cube, under a dimension that multiplies its rows. Please drop the multiplying \
+                 dimension, request the measures that reach out separately, or move the \
+                 aggregation into a measure.",
+                measure.full_name()
+            )));
+        }
+    }
+    Ok(())
+}
+
 pub struct AggregateMultipliedSubqueryProcessor<'a> {
     builder: &'a PhysicalPlanBuilder,
 }
@@ -120,6 +146,7 @@ impl<'a> LogicalNodeProcessor<'a, AggregateMultipliedSubquery>
                 }
             }
             AggregateMultipliedSubquerySource::MeasureSubquery(measure_subquery) => {
+                check_measures_survive_measure_subquery(&measure_subquery.schema.measures)?;
                 let subquery = self
                     .builder
                     .process_node(measure_subquery.as_ref(), context)?;

rust/cube/cubesqlplanner/cubesqlplanner/src/planner/multi_fact_join_groups.rs

@@ -559,6 +559,49 @@ mod tests {
         assert!(groups.single_join().is_err());
     }
 
+    /// Two measures of the same cube can still need different join trees, and
+    /// then they are two groups like any other multi-fact pair - the owning cube
+    /// says nothing about which joins to build.
+    ///
+    /// This is the precondition callers who slice by owning cube depend on, not
+    /// a claim about what they do with it: what each planner emits per group is
+    /// its own to cover.
+    #[test]
+    fn test_two_groups_for_measures_of_one_cube() {
+        let schema = MockSchema::from_yaml_file("common/integration_calculated_multi_fact.yaml");
+        let ctx = TestContext::new(schema).unwrap();
+
+        let total_amount = ctx.create_symbol("payments.total_amount").unwrap();
+        let converted_value = ctx.create_symbol("payments.converted_value").unwrap();
+        let meta_value = ctx.create_symbol("payment_meta.value").unwrap();
+
+        assert_eq!(total_amount.cube_name(), converted_value.cube_name());
+
+        let hints = MeasuresJoinHints::builder(&JoinHints::new())
+            .add_dimensions(&[meta_value])
+            .build(&[total_amount.clone(), converted_value.clone()])
+            .unwrap();
+
+        let groups = MultiFactJoinGroups::try_new(ctx.query_tools().clone(), hints).unwrap();
+
+        assert!(groups.is_multi_fact());
+        assert_eq!(groups.num_groups(), 2);
+        assert!(groups.single_join().is_err());
+
+        let grouped = groups
+            .groups()
+            .iter()
+            .map(|(_, measures)| measures.iter().map(|m| m.full_name()).collect::<Vec<_>>())
+            .collect::<Vec<_>>();
+        assert_eq!(
+            grouped,
+            vec![
+                vec!["payments.total_amount"],
+                vec!["payments.converted_value"]
+            ]
+        );
+    }
+
     #[test]
     fn test_resolve_join_path_for_measure() {
         let schema = MockSchema::from_yaml_file("common/multi_fact.yaml");