[pull] master from cube-js:master

packages/cubejs-schema-compiler/src/adapter/MemberSqlTemplateCompiler.js

@@ -11,15 +11,19 @@
  *   {
  *     template: string | string[],
  *     symbolPaths: string[][],                       // {arg:N}
- *     filterParams: [{ cube_name, name, column }],   // {fp:N}, column = fn|string
+ *     filterParams: [{ cube_name, name, column }],   // {fp:N}
  *     filterGroups: [{ filterParams: [...] }],       // {fg:N}
  *     securityContextValues: string[]                // {sv:N}
  *   }
  *
  * Member references are returned as recorded paths — the caller resolves them
- * to symbols. FILTER_PARAMS column callbacks are deferred (returned as the raw
- * JS function for the caller to invoke at render time); SECURITY_CONTEXT is
- * resolved eagerly here against the provided context.
+ * to symbols. SECURITY_CONTEXT is resolved eagerly here against the provided
+ * context.
+ *
+ * A FILTER_PARAMS `column` is a plain string, or — when the data model gave a
+ * callback — a recording of its own in the same shape, with the filter values it
+ * takes as `{fpv:N}` placeholders and `valueParamsCount` of them. A callback
+ * taking its values through a rest parameter is returned as the bare function.
  *
  * The module holds no planner state — `securityContext` and `sqlUtils` are
  * passed in — so it can be unit-tested in isolation.
@@ -29,11 +33,25 @@ const ARG_PREFIX = 'arg';
 const FILTER_PARAM_PREFIX = 'fp';
 const FILTER_GROUP_PREFIX = 'fg';
 const SECURITY_VALUE_PREFIX = 'sv';
+const FILTER_VALUE_PREFIX = 'fpv';
 
 function placeholder(prefix, index) {
   return `{${prefix}:${index}}`;
 }
 
+// The proxies handed to the member's `sql` function are captured by any
+// `FILTER_PARAMS` column callback it declares, so a nested compile cannot hand
+// them a different state. They record into `state.target`, which
+// `compileColumnCallback` swaps for the duration of that compile.
+function emptyRecording() {
+  return {
+    symbolPaths: [],
+    filterParams: [],
+    filterGroups: [],
+    securityContextValues: [],
+  };
+}
+
 // Returns the index of an equal path if it already exists, otherwise appends
 // and returns the new index. Paths are short arrays of strings, compared by
 // value, so repeated references collapse to a single placeholder.
@@ -68,12 +86,12 @@ function memberReferenceProxy(path, state) {
         return undefined;
       }
       if (prop === 'sql') {
-        const index = uniqueInsertPath(state.symbolPaths, [...path, '__sql_fn']);
+        const index = uniqueInsertPath(state.target.symbolPaths, [...path, '__sql_fn']);
         const ph = placeholder(ARG_PREFIX, index);
         return () => ph;
       }
       if (prop === 'toString' || prop === 'valueOf') {
-        const index = uniqueInsertPath(state.symbolPaths, path);
+        const index = uniqueInsertPath(state.target.symbolPaths, path);
         const ph = placeholder(ARG_PREFIX, index);
         return () => ph;
       }
@@ -84,13 +102,112 @@ function memberReferenceProxy(path, state) {
 
 // ---- FILTER_PARAMS / FILTER_GROUP ------------------------------------------
 
+// Declared parameters of a column callback: how many filter values it can take,
+// and whether it takes them as a rest parameter. `Function.length` stops at the
+// first defaulted parameter, so the parameter list is read from the source.
+function declaredValueParams(fn) {
+  const source = fn.toString();
+  const open = source.indexOf('(');
+  const arrow = source.indexOf('=>');
+  if (open === -1 || (arrow !== -1 && arrow < open)) {
+    // `v => …`, a single parameter without parentheses.
+    return { count: 1, rest: false, inner: '' };
+  }
+
+  let depth = 0;
+  let close = -1;
+  for (let i = open; i < source.length; i++) {
+    const ch = source[i];
+    if (ch === '(' || ch === '[' || ch === '{') depth++;
+    else if (ch === ')' || ch === ']' || ch === '}') {
+      depth--;
+      if (depth === 0 && ch === ')') {
+        close = i;
+        break;
+      }
+    }
+  }
+  if (close === -1) {
+    return { count: 0, rest: false, inner: source };
+  }
+
+  const inner = source.slice(open + 1, close);
+  const params = [];
+  let start = 0;
+  depth = 0;
+  for (let i = 0; i <= inner.length; i++) {
+    const ch = inner[i];
+    if (ch === '(' || ch === '[' || ch === '{') depth++;
+    else if (ch === ')' || ch === ']' || ch === '}') depth--;
+    if (i === inner.length || (ch === ',' && depth === 0)) {
+      const param = inner.slice(start, i).trim();
+      if (param) params.push(param);
+      start = i + 1;
+    }
+  }
+
+  return { count: params.length, rest: params.some(p => p.startsWith('...')), inner };
+}
+
+// Parsing the parameter list can come up short — a bound or native function
+// exposes no list, and a `)` inside a comment or a string default ends the scan
+// early. Too few placeholders would render the missing values as `undefined`, so
+// a count is trusted only when nothing could have thrown the scan off.
+//
+// `Function.length` counts the parameters before the first defaulted one, so it
+// catches a scan that stopped short of that point and nothing after it. Past
+// there only the text can vouch: a list holding neither a string nor a comment
+// has nothing for the scan to trip over. A list that does holds a callback back
+// to the render-time path, where every unreadable list already goes.
+function valueParamsAreCertain(fn, count, inner) {
+  if (fn.toString().includes('[native code]')) {
+    return false;
+  }
+  if (count < fn.length) {
+    return false;
+  }
+  return !/['"`]|\/\*|\/\//.test(inner);
+}
+
+// Compiles a column callback into a template of its own: its filter values
+// become `{fpv:N}` placeholders and whatever it references is recorded into its
+// own lists, so the placeholders it emits index its own dependencies rather than
+// the enclosing member's. A rest parameter consumes as many values as the query
+// happens to supply, which a fixed set of placeholders cannot express, so such a
+// callback is left for the caller to invoke at render time.
+function compileColumnCallback(column, state) {
+  const { count, rest, inner } = declaredValueParams(column);
+  if (rest || !valueParamsAreCertain(column, count, inner)) {
+    return column;
+  }
+
+  const values = [];
+  for (let i = 0; i < count; i++) {
+    values.push(placeholder(FILTER_VALUE_PREFIX, i));
+  }
+
+  const recording = emptyRecording();
+  const outer = state.target;
+  state.target = recording;
+  try {
+    const template = parseTemplateResult(column(...values));
+    return { template, valueParamsCount: count, ...recording };
+  } finally {
+    state.target = outer;
+  }
+}
+
 function filterParamsItemProxy(cubeName, name, state) {
   return {
     filter(column) {
-      const item = { cube_name: cubeName, name, column };
+      const item = {
+        cube_name: cubeName,
+        name,
+        column: typeof column === 'function' ? compileColumnCallback(column, state) : column,
+      };
       const toString = () => {
-        const index = state.filterParams.length;
-        state.filterParams.push(item);
+        const index = state.target.filterParams.length;
+        state.target.filterParams.push(item);
         return placeholder(FILTER_PARAM_PREFIX, index);
       };
       // `__member` lets FILTER_GROUP recover the item; `toString` records and
@@ -120,8 +237,8 @@ function filterGroupFn(state) {
       }
       return arg.__member;
     });
-    const index = state.filterGroups.length;
-    state.filterGroups.push({ filterParams });
+    const index = state.target.filterGroups.length;
+    state.target.filterGroups.push({ filterParams });
     return placeholder(FILTER_GROUP_PREFIX, index);
   };
 }
@@ -164,7 +281,7 @@ function coerceToStringValue(value) {
 }
 
 function recordSecurityValue(value, state) {
-  return placeholder(SECURITY_VALUE_PREFIX, uniqueInsertString(state.securityContextValues, value));
+  return placeholder(SECURITY_VALUE_PREFIX, uniqueInsertString(state.target.securityContextValues, value));
 }
 
 function securityFilterFn(value, required, state) {
@@ -247,23 +364,13 @@ function parseTemplateResult(result) {
  * @param {object} sqlUtils the SQL_UTILS object passed through to the template
  */
 function compileMemberSql(sqlFn, argNames, securityContext, sqlUtils) {
-  const state = {
-    symbolPaths: [],
-    filterParams: [],
-    filterGroups: [],
-    securityContextValues: [],
-  };
+  const root = emptyRecording();
+  const state = { target: root };
 
   const args = argNames.map(name => buildArg(name, state, securityContext, sqlUtils));
   const template = parseTemplateResult(sqlFn(...args));
 
-  return {
-    template,
-    symbolPaths: state.symbolPaths,
-    filterParams: state.filterParams,
-    filterGroups: state.filterGroups,
-    securityContextValues: state.securityContextValues,
-  };
+  return { template, ...root };
 }
 
 exports.compileMemberSql = compileMemberSql;

packages/cubejs-schema-compiler/test/integration/postgres/multi-stage-grain.test.ts

@@ -133,6 +133,22 @@ cubes:
             - orders.status
           include:
             - orders.id
+
+      # ── grain.exclude + filter.exclude on the same dim ────────────
+      # "Share of the whole universe" denominator: the value ignores both
+      # the query filter on status and the status partition. The result row
+      # set must stay the query's — dropping the filter may not resurrect
+      # rows the query filtered out.
+      - name: amount_grain_and_filter_exclude_status
+        sql: "{CUBE.total_amount}"
+        type: sum
+        multi_stage: true
+        grain:
+          exclude:
+            - orders.status
+        filter:
+          exclude:
+            - orders.status
     `);
 
   if (getEnv('nativeSqlPlanner')) {
@@ -257,6 +273,23 @@ cubes:
       { orders__status: 'pending', orders__category: 'books', orders__total_amount: '50', orders__amount_grain_keep_status_include_id: '125' },
       { orders__status: 'pending', orders__category: 'electronics', orders__total_amount: '75', orders__amount_grain_keep_status_include_id: '125' },
     ], { joinGraph, cubeEvaluator, compiler }));
+
+    // ── grain.exclude + filter.exclude on the same dim ────────────
+    it('grain.exclude + filter.exclude: keeps the query row set', async () => dbRunner.runQueryTest({
+      measures: ['orders.total_amount', 'orders.amount_grain_and_filter_exclude_status'],
+      dimensions: ['orders.status', 'orders.category'],
+      filters: [
+        { member: 'orders.status', operator: 'equals', values: ['completed'] },
+      ],
+      order: [{ id: 'orders.status' }, { id: 'orders.category' }],
+      timezone: 'UTC',
+    }, [
+      // The denominator is the per-category total over every status (books=250,
+      // electronics=275), while the rows stay the filtered ones — no pending or
+      // cancelled row may come back through the join.
+      { orders__status: 'completed', orders__category: 'books', orders__total_amount: '170', orders__amount_grain_and_filter_exclude_status: '250' },
+      { orders__status: 'completed', orders__category: 'electronics', orders__total_amount: '200', orders__amount_grain_and_filter_exclude_status: '275' },
+    ], { joinGraph, cubeEvaluator, compiler }));
   } else {
     // These tests rely on Tesseract; v1 planner does not implement the directive.
     test.skip('exclude: drops a dim from the partition', () => { expect(1).toBe(1); });
@@ -267,5 +300,6 @@ cubes:
     test.skip('keep_only: two-element array narrows to the (status, category) cell', () => { expect(1).toBe(1); });
     test.skip('include: two-element array extends the leaf grain', () => { expect(1).toBe(1); });
     test.skip('keep_only + include: keep narrows, include extends', () => { expect(1).toBe(1); });
+    test.skip('grain.exclude + filter.exclude: keeps the query row set', () => { expect(1).toBe(1); });
   }
 });