You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "plusplusjiajia (via GitHub)" <gi...@apache.org> on 2026/07/03 17:23:10 UTC

[PR] [table] Fail closed when reading a query-auth.enabled table [paimon-rust]

plusplusjiajia opened a new pull request, #447:
URL: https://github.com/apache/paimon-rust/pull/447

     ### Purpose
   
     A `query-auth.enabled` table needs a per-user row filter / column masking that
     the Rust client can't yet enforce. Reading it would return unfiltered data, so
     this makes reads fail closed.
   
     ### Brief change log
   
     - Add `CoreOptions::query_auth_enabled()`.
     - Guard `TableRead::to_arrow` (the single row-data exit) to return
       `Error::Unsupported` when enabled — non-bypassable for any construction path.
   
     ### Tests
   
     - Builder path and direct `TableRead::new(..)` path both fail closed
       (`read_builder` + `table_read` unit tests).
   
     ### API and Format
   
     Adds `CoreOptions::query_auth_enabled()`. 


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [table] Fail closed when reading a query-auth.enabled table [paimon-rust]

Posted by "plusplusjiajia (via GitHub)" <gi...@apache.org>.
plusplusjiajia commented on code in PR #447:
URL: https://github.com/apache/paimon-rust/pull/447#discussion_r3522462765


##########
crates/paimon/src/table/table_read.rs:
##########
@@ -72,8 +72,20 @@ impl<'a> TableRead<'a> {
 
     /// 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 = CoreOptions::new(self.table.schema.options());
+        // Fail closed: this client can't yet enforce query-auth row filtering / column masking,
+        // so refuse to read rather than return unfiltered data. Guarding this single data exit
+        // (not the `ReadBuilder`) makes the check non-bypassable.
+        if core_options.query_auth_enabled() {

Review Comment:
   Update: dropped the !scan_all_files exemption — it wasn't a clean read/write split (the files system table also uses it to surface metadata). plan() now fails closed unconditionally, so writes/compaction are blocked too until the client can enforce the auth filter.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [table] Fail closed when reading a query-auth.enabled table [paimon-rust]

Posted by "QuakeWang (via GitHub)" <gi...@apache.org>.
QuakeWang commented on code in PR #447:
URL: https://github.com/apache/paimon-rust/pull/447#discussion_r3522378274


##########
crates/paimon/src/table/table_read.rs:
##########
@@ -72,8 +72,20 @@ impl<'a> TableRead<'a> {
 
     /// 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 = CoreOptions::new(self.table.schema.options());
+        // Fail closed: this client can't yet enforce query-auth row filtering / column masking,
+        // so refuse to read rather than return unfiltered data. Guarding this single data exit
+        // (not the `ReadBuilder`) makes the check non-bypassable.
+        if core_options.query_auth_enabled() {

Review Comment:
   This guards row materialization, but scan planning still succeeds for a `query-auth.enabled` table. `TableScan::plan()` / `plan_with_trace()` can still expose splits, file metadata, row counts, and stats before `to_arrow()` is reached, and DataFusion also plans eagerly. To make this truly fail closed, please add the same `Unsupported` guard at the scan planning boundary as well.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [table] Fail closed when reading a query-auth.enabled table [paimon-rust]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #447:
URL: https://github.com/apache/paimon-rust/pull/447


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [table] Fail closed when reading a query-auth.enabled table [paimon-rust]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #447:
URL: https://github.com/apache/paimon-rust/pull/447#discussion_r3523406307


##########
crates/integrations/datafusion/tests/system_tables.rs:
##########
@@ -72,6 +72,41 @@ async fn run_sql(ctx: &SQLContext, sql: &str) -> Vec<RecordBatch> {
         .unwrap_or_else(|e| panic!("Failed to execute `{sql}`: {e}"))
 }
 
+// Error text of `sql`, whether it surfaces at planning or execution.
+async fn query_error(ctx: &SQLContext, sql: &str) -> String {
+    match ctx.sql(sql).await {
+        Err(e) => e.to_string(),
+        Ok(df) => df
+            .collect()
+            .await
+            .expect_err(&format!("`{sql}` must fail"))
+            .to_string(),
+    }
+}
+
+#[tokio::test]
+async fn test_query_auth_table_fails_closed() {
+    let (ctx, _catalog, _tmp) = create_context().await;
+    run_sql(
+        &ctx,
+        "CREATE TABLE paimon.default.qa (id INT) WITH ('query-auth.enabled' = 'true')",
+    )

Review Comment:
   This new integration test currently fails on the first query. Running `CARGO_TARGET_DIR=/tmp/paimon-rust-shared-target cargo test -p paimon-datafusion test_query_auth_table_fails_closed --test system_tables` returns `[]` for `SELECT * FROM paimon.default.qa` instead of an error containing `query-auth.enabled`, so the DataFusion base-table read path is still not fail-closed for a `query-auth.enabled` table. Please enforce the guard at the DataFusion provider/catalog boundary (or otherwise make this public SQL path hit the core guard) and keep this test passing; as written, the PR still allows this read to succeed silently.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [table] Fail closed when reading a query-auth.enabled table [paimon-rust]

Posted by "plusplusjiajia (via GitHub)" <gi...@apache.org>.
plusplusjiajia commented on code in PR #447:
URL: https://github.com/apache/paimon-rust/pull/447#discussion_r3522410710


##########
crates/paimon/src/table/table_read.rs:
##########
@@ -72,8 +72,20 @@ impl<'a> TableRead<'a> {
 
     /// 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 = CoreOptions::new(self.table.schema.options());
+        // Fail closed: this client can't yet enforce query-auth row filtering / column masking,
+        // so refuse to read rather than return unfiltered data. Guarding this single data exit
+        // (not the `ReadBuilder`) makes the check non-bypassable.
+        if core_options.query_auth_enabled() {

Review Comment:
   @QuakeWang Good catch. Added the same Unsupported guard at scan planning (plan() / plan_with_trace()), gated on !scan_all_files so only reads are blocked (writes/compaction stay allowed).



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [table] Fail closed when reading a query-auth.enabled table [paimon-rust]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #447:
URL: https://github.com/apache/paimon-rust/pull/447#discussion_r3523406307


##########
crates/integrations/datafusion/tests/system_tables.rs:
##########
@@ -72,6 +72,41 @@ async fn run_sql(ctx: &SQLContext, sql: &str) -> Vec<RecordBatch> {
         .unwrap_or_else(|e| panic!("Failed to execute `{sql}`: {e}"))
 }
 
+// Error text of `sql`, whether it surfaces at planning or execution.
+async fn query_error(ctx: &SQLContext, sql: &str) -> String {
+    match ctx.sql(sql).await {
+        Err(e) => e.to_string(),
+        Ok(df) => df
+            .collect()
+            .await
+            .expect_err(&format!("`{sql}` must fail"))
+            .to_string(),
+    }
+}
+
+#[tokio::test]
+async fn test_query_auth_table_fails_closed() {
+    let (ctx, _catalog, _tmp) = create_context().await;
+    run_sql(
+        &ctx,
+        "CREATE TABLE paimon.default.qa (id INT) WITH ('query-auth.enabled' = 'true')",
+    )

Review Comment:
   This new integration test currently fails on the first query. Running `CARGO_TARGET_DIR=/tmp/paimon-rust-shared-target cargo test -p paimon-datafusion test_query_auth_table_fails_closed --test system_tables` returns `[]` for `SELECT * FROM paimon.default.qa` instead of an error containing `query-auth.enabled`, so the DataFusion base-table read path is still not fail-closed for a `query-auth.enabled` table. Please enforce the guard at the DataFusion provider/catalog boundary (or otherwise make this public SQL path hit the core guard) and keep this test passing; as written, the PR still allows this read to succeed silently.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org