JingsongLi commented on code in PR #513:
URL: https://github.com/apache/paimon-rust/pull/513#discussion_r4177941396
##########
crates/paimon/src/table/table_read.rs:
##########
@@ -942,21 +943,124 @@ impl<'a> PaimonTableRead<'a> {
scan",
));
}
- if !grant.is_unrestricted() {
+ // The server ruled on these columns only, whether or not it set
rules.
+ if let Some(select) = grant.select() {
+ if let Some(outside) = self
+ .read_type
+ .iter()
+ .map(|f| f.name())
+ .chain(filter_columns.iter().map(String::as_str))
+ .find(|name| !select.iter().any(|s| s == name))
+ {
+ return Err(super::query_auth::unsupported(&format!(
+ "'{outside}' is outside the columns this plan was
authorized for; plan \
+ with it"
+ )));
+ }
+ }
+ if !grant.is_unrestricted() && restricted.is_none() {
+ restricted = Some(grant);
+ }
+ }
+ if let Some(grant) = restricted {
+ if data_splits
+ .iter()
+ .any(|s| s.query_auth_grant().is_none_or(|g| **g != **grant))
+ {
return Err(super::query_auth::unsupported(
- "this client cannot apply a row filter or column masking,
so it refuses \
- rather than return unfiltered rows",
+ "the splits were planned under different rules; re-plan
the scan",
));
}
}
- Ok(())
+ Ok(restricted.cloned())
}
/// Returns an [`ArrowRecordBatchStream`].
pub fn to_arrow(&self, data_splits: &[DataSplit]) ->
crate::Result<ArrowRecordBatchStream> {
- let has_primary_keys = !self.table.schema.primary_keys().is_empty();
let core_options = self.table.schema.core_options();
- self.ensure_authorized_by_splits(&core_options, data_splits)?;
+ match self.ensure_authorized_by_splits(&core_options, data_splits)? {
+ Some(grant) => self.read_restricted(data_splits, &grant),
+ None => self.read_splits(data_splits, &core_options),
+ }
+ }
+
+ /// Reads what the row filter needs, filters on stored values and projects
+ /// back (Java `doAuth`).
+ fn read_restricted(
+ &self,
+ data_splits: &[DataSplit],
+ grant: &super::query_auth::QueryAuthGrant,
+ ) -> crate::Result<ArrowRecordBatchStream> {
+ use super::query_auth::{filter_batch, unsupported};
+
+ let schema_fields = self.table.schema().fields().to_vec();
+ let rules = grant.rules();
+ let index_of = |field: &DataField| schema_fields.iter().position(|s|
s.id() == field.id());
+ let needed = rules.filter_columns();
+ // A partly projected column would feed the filter a partial value
(Java
+ // `validateReadType`).
+ for field in &self.read_type {
+ if let Some(index) = index_of(field) {
+ if needed.contains(&index) && field.data_type() !=
schema_fields[index].data_type()
+ {
+ return Err(unsupported(&format!(
+ "the server's row filter reads '{}', which the read
projects only in part",
+ field.name()
+ )));
+ }
+ }
+ }
+
+ let mut physical = self.read_type.clone();
+ let mut needed: Vec<usize> = needed.into_iter().collect();
+ needed.sort_unstable();
+ for index in needed {
+ let field = &schema_fields[index];
+ if !physical.iter().any(|f| f.id() == field.id()) {
+ physical.push(field.clone());
+ }
+ }
+ let mut inner = self.clone();
+ inner.read_type = physical.clone();
+ inner.limit = None;
+ let stream = inner.read_splits(data_splits,
&self.table.schema.core_options())?;
Review Comment:
[P2] Keep throwing Variant SQL extraction above row authorization
The restricted reader forwards the requested partial Variant type to
`read_splits` before `filter_batch` runs. This is reproducible with INT,
without relying on FLOAT: with actual Parquet rows `(1, {"x":"bad"})` and `(2,
{"x":7})` and the server rule `id > 1`, `SELECT id` correctly returns only 2,
but `SELECT variant_get(payload, '$.x', 'INT') FROM paimon.default.vguard`
fails with `Cannot cast Variant value to Int` instead of returning 7.
DataFusion automatically pushes the strict extraction into the read type and
evaluates the denied row. Java Spark `VariantPushDownUtils` rejects pushdown
when `failOnError` can throw, keeping this SQL expression above the authorized
scan. Deferring throwing extraction until after authorization, or disabling
this automatic pushdown for restricted scans, fixes the actual probe; disabling
it for the auth-enabled table passed all 4 query-auth SQL tests (the INT
regression plus the 3 existing SQL tests). Please add this denied-row
projection regressi
on.
--
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]
