Repository navigation
fix(auth): stop BLOB view predicate lookups at LIMIT under query-auth - #1092
plusplusjiajia wants to merge 3 commits into
Conversation
A query-auth read with BLOB columns resolves the views its predicates read before LIMIT applies, a whole batch at a time, so it could look up references past the quota and fail on one no row needed. Read one candidate at a time in that case, as the unrestricted read already does.
7ca0b92 to
a2a42bb
Compare
JingsongLi
left a comment
JingsongLi
left a comment
There was a problem hiding this comment.
Requirement fit: SUPPORTED. Implementation: FINDINGS at e4d5697. Stopping predicate BLOB-view lookups when LIMIT is satisfied fixes a real authorized-read failure, and all 120 REST tests pass. However, the new unconditional one-row slicing regresses grouped dependency reads even for larger limits. An independent real HTTP / Parquet / BLOB probe with 64 distinct references confirms 64 upstream table requests with LIMIT 64 versus 1 before this change, while returning the same 64 matching rows. Please preserve bounded batching while retaining the new small-limit correctness.
| while let Some(batch) = stream.next().await { | ||
| let batch = batch?; | ||
| for row in 0..batch.num_rows() { | ||
| yield batch.slice(row, 1); |
There was a problem hiding this comment.
[P2] Keep dependency lookups batched up to the remaining output quota
Every LIMIT now slices authorized candidates into one-row batches. For distinct references, BlobViewLookup::load_missing calls load once per row; load re-fetches the upstream table and re-plans/scans it, so the existing descriptor cache does not preserve grouped reads. Reproduced with real REST HTTP and actual Parquet/BLOB files: 64 authorized rows, 64 distinct references to one upstream table, picture IS NOT NULL, projection id, and LIMIT 64 produces 64 GetTable requests and 64 output batches (about 122ms locally). Removing only this new slicing block produces the identical rows with 1 request and 1 batch (about 8ms); the unlimited path also uses 1 request. A larger result quota therefore converts batched remote metadata/data reads into a serial per-row path. Please cap candidate chunks by the remaining output quota and decrement that quota after predicate evaluation, allowing grouped lookups while still never resolving references past the first LIMIT qualifying rows; keep the new rejected-candidate/missing-reference tests and add a request-count regression.
There was a problem hiding this comment.
Thanks, updated: chunks are now capped by the remaining LIMIT quota, which only rows that pass the predicates consume, so lookups stay grouped without resolving past LIMIT. Kept the existing tests and added a request-count regression.
Cut authorized candidates into chunks of at most the rows LIMIT still needs instead of single rows, and subtract the rows that pass the predicates from that quota. Distinct references in a chunk load with one upstream request, and no reference past the first LIMIT qualifying rows is resolved.
2 participants
Purpose
Follow-up to #1089. Under query-auth, predicates on BLOB views can resolve references beyond LIMIT and fail on an upstream row the result never needs. PK merge output and readers that ignore the requested batch size make a decoder batch-size override insufficient.
Brief change log
After merging, row authorization and masking, cut candidates into chunks of at most the rows LIMIT still needs before resolving predicate views. Rows that pass the predicates are subtracted from that quota right after predicate evaluation, so a rejected candidate consumes none. Distinct references in a chunk still load together, and no reference past the last qualifying row is looked up. The stages between the cut and the predicates handle one batch at a time, so the quota is current before the next chunk is cut.
Remove the
read.batch-size = 1override and reuse the view-resolution condition infinish_authorized_blobs. File decoding, authorization and masking retain normal batches.Tests
Retain the append data-evolution regression. Add overlapping-file PK regressions for deduplicate and partial-update, covering masked aliases, descriptor/value reads, projections, batch sizes 1/1024, zero/small limits, rejected candidates and errors when missing references are required.
Add a request-count regression: four distinct references to one upstream table load with a single get-table request under LIMIT 2, LIMIT 4 and no LIMIT, with and without a payload filter. The mock REST server now counts get-table requests per table.
Scope
Deferring output-only views until after BLOB payload predicates and LIMIT remains separate follow-up work. Per-row OR short-circuiting of view lookups is also outside this change.