gortiz opened a new pull request, #20183:
URL: https://github.com/apache/pinot/pull/20183

   ## Summary
   
   With row-level security (RLS), a user filter can run on rows that the RLS 
filter hides. This PR makes the server apply the RLS filter first, per segment. 
The user filter then runs only on the rows that the RLS filter allows. The PR 
also hides the response stats that show the layout of the whole table.
   
   ## Background
   
   The broker ANDs the RLS filter into the WHERE clause, and it also sends the 
filter in the `rlsFilters-<rawTableName>` query option. `AndDocIdSet` does not 
always evaluate the RLS predicate before the user expressions:
   
   1. `ExpressionScanDocIdIterator` evaluates its expression on blocks of 10k 
documents, not only on the documents that the other children match.
   2. A composite child under OR or NOT evaluates the whole segment.
   3. The AND that `deriveNulls` builds for null handling has no restriction 
push-down.
   
   As a result, a user expression ran on hidden rows. Errors, scan stats and 
other side channels of that expression showed hidden values. For example, 
`CAST(col AS INT) = 1` failed on a hidden row that holds a string.
   
   Related: [apache/pinot#20169](https://github.com/apache/pinot/pull/20169) 
closes another RLS gap (cross-table functions). This PR uses the AND 
restriction push-down of 
[apache/pinot#19408](https://github.com/apache/pinot/pull/19408).
   
   ## Changes
   
   ### Server
   
   - `QueryContextConverterUtils` reads the RLS filter from the 
`rlsFilters-<rawTableName>` query option into `QueryContext`. It uses the same 
key and the same parser as `RlsFiltersRewriter`. If the filter cannot be 
parsed, the query fails.
   - `InstancePlanMakerImplV2` optimizes the RLS filter with the table schema 
(`QueryOptimizer#optimizeFilter`). The broker, or the multi-stage leaf, 
optimizes the WHERE copy of the filter in the same way. Then a policy such as 
`intCol = 2.0` works on the dictionary of an INT column.
   - `FilterPlanNode` wraps the user filter in the new 
`RowLevelSecurityFilterOperator`:
     - It materializes the RLS bitmap once per segment.
     - It limits the bitmap to the `numDocs` that the plan read, for consuming 
segments.
     - It ANDs the bitmap with the valid-document snapshot (upsert).
     - It ANDs the result with the user filter, with the restriction push-down 
on.
     - It never reports a match-all result, an optimized count, or bitmaps. So 
`FastFilteredCountOperator`, `NonScanBasedAggregationOperator` and other short 
cuts cannot skip the RLS bitmap.
   - `QueryContext#isAndRestrictionPushdownEnabled()` is always `true` under 
RLS, whatever the query option and the server default say. `AndFilterOperator` 
and `OrFilterOperator` pass the flag to `deriveNulls`.
   - `ServerQueryExecutorV1Impl` removes the table-layout stats from the 
response: `totalDocs`, and the counts of segments queried, processed, matched 
and pruned. A table that holds only the rows of the tenant has a different 
layout, so these values cannot be emulated. The server query log still shows 
them (`InstanceResponseBlock#getServerMetadata`).
   - New server timer `RLS_FILTER_TIME_MS` and new `rlsFilterTimeMs` field in 
the server query log. They record the time spent on the RLS bitmap.
   - `numEntriesScannedInFilter` does not include the cost of the RLS bitmap. 
Without an index, that cost is the number of documents in the segment.
   
   ### Broker
   
   - If the broker cannot apply the RLS filter, the query fails with 
`QUERY_VALIDATION`. Before, the query ran without the filter. The message does 
not show the policy.
   - `RlsResponseUtils` hides the table-layout stats on both engines, also for 
responses from older servers. The broker query log keeps the real values.
   - The single-stage broker skips the materialized view rewrite under RLS. 
After the rewrite, the server finds no RLS filter for the materialized view 
table. A better solution is a follow-up.
   
   ## Compatibility
   
   - No change to the wire format. The query option already exists.
   - A new broker with an old server: the broker hides the table-layout stats, 
but the old server keeps the old filter order.
   - An old broker with a new server: the server applies the RLS filter from 
the query option.
   
   ## Known limitations
   
   - With an old server, the multi-stage `stageStats` still show the 
table-layout stats of the leaf stage.
   - The `explainPlanNum*FilterSegments` stats are not hidden.
   - `TRACE_INFO` and the `SERVER_SEGMENT_MISSING` error can show the layout of 
the table.
   - In the multi-stage engine, the error for a policy that cannot be parsed 
contains the policy text. This behavior existed before this PR.
   - The server query log writes `rlsFilterTimeMs=-1` for queries without RLS.
   
   ## Tests
   
   - `RowLevelSecurityScanQueriesTest` (new): a payload that is numeric for 
tenant A and a string for tenant B. A query fails if it evaluates the user 
expression on a tenant B row. It covers top-level expressions, expressions 
inside OR and NOT, with and without an index on the RLS column, with 
`andRestrictionPushdownMode=NEVER` and `skipIndexes`, with null handling, 
filtered aggregations, star-tree, consuming segments, upsert snapshots, plan 
short cuts and stats.
   - `RowLevelSecurityFilterOperatorTest` (new): limit to `numDocs`, 
valid-document AND, one materialization, no short cuts.
   - `QueryExecutorTest`: the server hides the table-layout stats and records 
the timer.
   - `BaseSingleStageBrokerRequestHandlerTest`: the query fails if the RLS 
rewrite fails, the stats are hidden, and the materialized view rewrite is 
skipped under RLS.
   - `RlsResponseUtilsTest` (new): the multi-stage broker hides the stats.
   - `RowLevelSecurityIntegrationTest`: on both engines, a user filter that 
fails on hidden rows succeeds and returns the same rows as the admin query, and 
the stats, including the multi-stage `stageStats`, are hidden.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to