fix(tesseract): keep pre-aggregations for RBAC access-denied 1=0 segment (cube-js#11123)

Author: waralexrom

packages/cubejs-schema-compiler/test/integration/postgres/pre-aggregations-access-denied-segment.test.ts

@@ -0,0 +1,81 @@
+import { PostgresQuery } from '../../../src/adapter/PostgresQuery';
+import { prepareJsCompiler } from '../../unit/PrepareCompiler';
+
+// When a cube's access policy denies the queried members, RBAC
+// (CompilerApi.applyRowLevelSecurity) appends a member-expression segment
+// `{ expression: () => '1 = 0', cubeName, name: 'rlsAccessDenied' }`. The rollup
+// must still be selected (the `1 = 0` is just a constant filter on top of it).
+// This guards that the segment doesn't disqualify pre-aggregation matching.
+describe('PreAggregations access-denied segment', () => {
+  jest.setTimeout(200000);
+
+  const { compiler, joinGraph, cubeEvaluator } = prepareJsCompiler(`
+  cube('rls_visitors', {
+    sql: 'select * from visitors',
+    sqlAlias: 'rlsv',
+
+    measures: {
+      count: {
+        type: 'count'
+      }
+    },
+
+    dimensions: {
+      id: {
+        type: 'number',
+        sql: 'id',
+        primaryKey: true
+      },
+      status: {
+        type: 'number',
+        sql: 'status'
+      }
+    },
+
+    preAggregations: {
+      statusRollup: {
+        type: 'rollup',
+        measures: [CUBE.count],
+        dimensions: [CUBE.status],
+      }
+    }
+  });
+
+  view('rls_visitors_view', {
+    cubes: [
+      {
+        join_path: 'rls_visitors',
+        includes: '*',
+      },
+    ]
+  });
+  `);
+
+  it('selects the rollup despite the access-denied segment', async () => {
+    await compiler.compile();
+
+    const query = new PostgresQuery(
+      { joinGraph, cubeEvaluator, compiler },
+      {
+        measures: ['rls_visitors_view.count'],
+        // Byte-for-byte the segment CompilerApi.applyRowLevelSecurity injects on denial.
+        segments: [
+          {
+            expression: () => '1 = 0',
+            cubeName: 'rls_visitors',
+            name: 'rlsAccessDenied',
+          },
+        ],
+        timezone: 'America/Los_Angeles',
+        preAggregationsSchema: '',
+      }
+    );
+
+    const preAggregationsDescription: any = query.preAggregations?.preAggregationsDescription();
+    const [sql] = query.buildSqlAndParams();
+
+    expect(preAggregationsDescription[0].tableName).toEqual('rlsv_status_rollup');
+    expect(sql).toContain('rlsv_status_rollup');
+    expect(sql).toContain('1 = 0');
+  });
+});

rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/dimension_matcher.rs

@@ -315,7 +315,16 @@ impl<'a> DimensionMatcher<'a> {
                 Ok(result)
             }
             FilterItem::Segment(segment) => {
-                self.try_match_symbol(&segment.member_evaluator(), add_to_matched_dimension)
+                let symbol = segment.member_evaluator();
+                // An ad-hoc member-expression segment that references no members
+                // (a constant like the `1 = 0` rlsAccessDenied segment RBAC injects
+                // on access denial) is a filter pushable on top of any rollup, so it
+                // doesn't need pre-aggregation coverage. Named segments still must be
+                // covered and fall through to the regular matcher.
+                if segment.is_member_expression() && symbol.get_dependencies().is_empty() {
+                    return Ok(MatchState::Full);
+                }
+                self.try_match_symbol(&symbol, add_to_matched_dimension)
             }
         }
     }
@@ -339,26 +348,55 @@ impl<'a> DimensionMatcher<'a> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::cube_bridge::member_expression::MemberExpressionExpressionDef;
+    use crate::cube_bridge::member_sql::MemberSql;
+    use crate::cube_bridge::options_member::OptionsMember;
     use crate::logical_plan::optimizers::pre_aggregation::{
         PreAggregationFullName, PreAggregationsCompiler,
     };
-    use crate::test_fixtures::cube_bridge::MockSchema;
+    use crate::test_fixtures::cube_bridge::{
+        MockMemberExpressionDefinition, MockMemberSql, MockSchema,
+    };
     use crate::test_fixtures::test_utils::TestContext;
     use indoc::indoc;
+    use std::rc::Rc;
 
     fn create_test_context() -> TestContext {
         let schema = MockSchema::from_yaml_file("common/pre_aggregation_matching_test.yaml");
         TestContext::new(schema).unwrap()
     }
 
+    /// Builds an ad-hoc member-expression segment (no registered `segments:`
+    /// path) from constant SQL — mirrors the `rlsAccessDenied` `1 = 0` segment.
+    fn const_expr_segment(name: &str, cube: &str, sql: &str) -> OptionsMember {
+        let member_sql: Rc<dyn MemberSql> = Rc::new(MockMemberSql::new(sql).unwrap());
+        let expr = MockMemberExpressionDefinition::builder()
+            .expression_name(Some(name.to_string()))
+            .cube_name(Some(cube.to_string()))
+            .expression(MemberExpressionExpressionDef::Sql(member_sql))
+            .build();
+        OptionsMember::MemberExpression(Rc::new(expr))
+    }
+
     fn match_pre_agg(ctx: &TestContext, pre_agg_name: &str, query_yaml: &str) -> MatchState {
+        match_pre_agg_with_segments(ctx, pre_agg_name, query_yaml, vec![])
+    }
+
+    fn match_pre_agg_with_segments(
+        ctx: &TestContext,
+        pre_agg_name: &str,
+        query_yaml: &str,
+        extra_segments: Vec<OptionsMember>,
+    ) -> MatchState {
         let cube_names = vec!["orders".to_string()];
         let mut compiler =
             PreAggregationsCompiler::try_new(ctx.query_tools().clone(), &cube_names).unwrap();
         let name = PreAggregationFullName::new("orders".to_string(), pre_agg_name.to_string());
         let pre_agg = compiler.compile_pre_aggregation(&name).unwrap();
 
-        let qp = ctx.create_query_properties(query_yaml).unwrap();
+        let qp = ctx
+            .create_query_properties_with_segments(query_yaml, extra_segments)
+            .unwrap();
         let mut matcher = DimensionMatcher::new(ctx.query_tools().query_tools().clone(), &pre_agg);
         matcher
             .try_match(
@@ -743,6 +781,30 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_constant_member_expression_segment_full_match() {
+        let ctx = create_test_context();
+        // The ad-hoc `1 = 0` rlsAccessDenied segment references no members, so it's a
+        // constant filter pushable on top of any rollup and imposes no coverage
+        // requirement: with both rollup dimensions selected the match stays Full
+        // (without the fix the bare segment forces NotMatched).
+        assert_eq!(
+            match_pre_agg_with_segments(
+                &ctx,
+                "main_rollup",
+                indoc! {"
+                    measures:
+                      - orders.count
+                    dimensions:
+                      - orders.status
+                      - orders.city
+                "},
+                vec![const_expr_segment("rlsAccessDenied", "orders", "1 = 0")],
+            ),
+            MatchState::Full,
+        );
+    }
+
     #[test]
     fn test_segment_not_matched_missing_in_pre_agg() {
         let ctx = create_test_context();

rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/base_segment.rs

@@ -14,6 +14,9 @@ pub struct BaseSegment {
     member_evaluator: Rc<MemberSymbol>,
     cube_name: String,
     name: String,
+    /// True when this segment is an ad-hoc query-level member expression (no
+    /// registered `segments:` path), as opposed to a named cube segment.
+    is_member_expression: bool,
 }
 
 impl PartialEq for BaseSegment {
@@ -38,6 +41,7 @@ impl BaseSegment {
             None,
             vec![cube_name.clone()],
         )?;
+        let is_member_expression = full_name.is_none();
         let full_name = full_name.unwrap_or(member_expression_symbol.full_name());
         let member_evaluator = MemberSymbol::new_member_expression(member_expression_symbol);
 
@@ -46,8 +50,13 @@ impl BaseSegment {
             member_evaluator,
             cube_name,
             name,
+            is_member_expression,
         }))
     }
+
+    pub fn is_member_expression(&self) -> bool {
+        self.is_member_expression
+    }
     pub fn full_name(&self) -> String {
         self.full_name.clone()
     }

rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/test_utils/test_context.rs

@@ -1,5 +1,6 @@
 use crate::cube_bridge::base_query_options::{BaseQueryOptions, MaskedMemberItem};
 use crate::cube_bridge::join_hints::JoinHintItem;
+use crate::cube_bridge::options_member::OptionsMember;
 use crate::logical_plan::PreAggregationUsage;
 #[cfg(feature = "integration-postgres")]
 use crate::logical_plan::{PreAggregation, PreAggregationSource, PreAggregationTable};
@@ -347,6 +348,17 @@ impl TestContext {
     ///
     /// Panics if YAML cannot be parsed.
     pub fn create_query_options_from_yaml(&self, yaml: &str) -> Rc<dyn BaseQueryOptions> {
+        self.create_query_options_from_yaml_with_segments(yaml, vec![])
+    }
+
+    /// Like `create_query_options_from_yaml`, but appends extra (typically
+    /// member-expression) segments that can't be expressed as YAML member
+    /// names — e.g. the constant `1 = 0` `rlsAccessDenied` segment RBAC injects.
+    pub fn create_query_options_from_yaml_with_segments(
+        &self,
+        yaml: &str,
+        extra_segments: Vec<OptionsMember>,
+    ) -> Rc<dyn BaseQueryOptions> {
         let yaml_options: YamlBaseQueryOptions = serde_yaml::from_str(yaml)
             .unwrap_or_else(|e| panic!("Failed to parse YAML query options: {}", e));
 
@@ -360,10 +372,14 @@ impl TestContext {
             .map(|d| members_from_strings(d))
             .filter(|d| !d.is_empty());
 
-        let segments = yaml_options
-            .segments
-            .map(|s| members_from_strings(s))
-            .filter(|s| !s.is_empty());
+        let segments = {
+            let mut segments = yaml_options
+                .segments
+                .map(|s| members_from_strings(s))
+                .unwrap_or_default();
+            segments.extend(extra_segments);
+            Some(segments).filter(|s| !s.is_empty())
+        };
 
         let order = yaml_options
             .order
@@ -444,7 +460,15 @@ impl TestContext {
     }
 
     pub fn create_query_properties(&self, yaml: &str) -> Result<Rc<QueryProperties>, CubeError> {
-        let options = self.create_query_options_from_yaml(yaml);
+        self.create_query_properties_with_segments(yaml, vec![])
+    }
+
+    pub fn create_query_properties_with_segments(
+        &self,
+        yaml: &str,
+        extra_segments: Vec<OptionsMember>,
+    ) -> Result<Rc<QueryProperties>, CubeError> {
+        let options = self.create_query_options_from_yaml_with_segments(yaml, extra_segments);
         QueryPropertiesCompiler::new(self.query_tools.clone()).build(options)
     }
 
@@ -469,7 +493,15 @@ impl TestContext {
         &self,
         query: &str,
     ) -> Result<(String, Vec<PreAggregationUsage>), cubenativeutils::CubeError> {
-        let options = self.create_query_options_from_yaml(query);
+        self.build_sql_with_used_pre_aggregations_with_segments(query, vec![])
+    }
+
+    pub fn build_sql_with_used_pre_aggregations_with_segments(
+        &self,
+        query: &str,
+        extra_segments: Vec<OptionsMember>,
+    ) -> Result<(String, Vec<PreAggregationUsage>), cubenativeutils::CubeError> {
+        let options = self.create_query_options_from_yaml_with_segments(query, extra_segments);
         let ctx = self.for_options(options.as_ref())?;
         let request = QueryPropertiesCompiler::new(ctx.query_tools.clone()).build(options)?;
         let planner = TopLevelPlanner::new(request, ctx.query_tools.clone(), true);

rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/sql_generation.rs

@@ -3,9 +3,15 @@
 //! These tests verify that queries correctly match and use pre-aggregations,
 //! checking that the generated SQL contains references to pre-aggregation tables.
 
-use crate::test_fixtures::cube_bridge::MockSchema;
+use crate::cube_bridge::member_expression::MemberExpressionExpressionDef;
+use crate::cube_bridge::member_sql::MemberSql;
+use crate::cube_bridge::options_member::OptionsMember;
+use crate::test_fixtures::cube_bridge::{
+    MockMemberExpressionDefinition, MockMemberSql, MockSchema,
+};
 use crate::test_fixtures::test_utils::TestContext;
 use indoc::indoc;
+use std::rc::Rc;
 
 #[tokio::test(flavor = "multi_thread")]
 async fn test_basic_pre_agg_sql() {
@@ -495,6 +501,46 @@ async fn test_base_and_calculated_measure_parital_match() {
 
 // --- Segment matching tests ---
 
+// When a cube's access policy denies the queried members, RBAC
+// (CompilerApi.applyRowLevelSecurity) appends a member-expression segment
+// `{ expression: () => '1 = 0', cubeName, name: 'rlsAccessDenied' }`. It
+// references no members (empty dependencies), so it's a constant filter on top
+// of the rollup and must not disqualify pre-aggregation matching.
+#[tokio::test(flavor = "multi_thread")]
+async fn test_constant_member_expression_segment_keeps_pre_aggregation() {
+    let schema = MockSchema::from_yaml_file("common/pre_aggregation_matching_test.yaml")
+        .only_pre_aggregations(&["main_rollup"]);
+    let ctx = TestContext::new(schema).unwrap();
+
+    let query_yaml = indoc! {"
+        measures:
+          - orders.count
+        dimensions:
+          - orders.status
+    "};
+
+    let access_denied_segment = {
+        let sql: Rc<dyn MemberSql> = Rc::new(MockMemberSql::new("1 = 0").unwrap());
+        let expr = MockMemberExpressionDefinition::builder()
+            .expression_name(Some("rlsAccessDenied".to_string()))
+            .cube_name(Some("orders".to_string()))
+            .expression(MemberExpressionExpressionDef::Sql(sql))
+            .build();
+        OptionsMember::MemberExpression(Rc::new(expr))
+    };
+
+    let (sql, pre_aggrs) = ctx
+        .build_sql_with_used_pre_aggregations_with_segments(query_yaml, vec![access_denied_segment])
+        .unwrap();
+
+    assert_eq!(pre_aggrs.len(), 1);
+    assert_eq!(pre_aggrs[0].name(), "main_rollup");
+    assert!(
+        sql.contains("1 = 0"),
+        "expected the constant access-denied segment in SQL, got:\n{sql}"
+    );
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn test_segment_full_match() {
     let schema = MockSchema::from_yaml_file("common/pre_aggregation_matching_test.yaml")