Skip to content

Commit 3c2cd09

Browse files
authored
feat: allow COPY to be disabled independently (#25824)
## Which issue does this PR close? - Closes #25823. ## Rationale for this change Applications embedding DataFusion may want to allow DML operations such as INSERT into registered table providers while disallowing COPY, which writes query results to a file or object store. SQLOptions currently controls COPY through allow_dml, so COPY cannot be disabled without also disabling other DML. ## What changes are included in this PR? - Add SQLOptions::with_allow_copy to control COPY independently, defaulting to true. - Keep with_allow_dml(false) behavior unchanged so it continues to reject COPY. - Add an integration test showing that disabling COPY does not disable INSERT. ## What is the testing strategy for this PR? - cargo fmt --all -- --check - cargo clippy --all-targets --all-features -- -D warnings - cargo test -p datafusion --test core_integration -- sql::sql_api::copy_can_be_disabled_independently_from_dml --exact - cargo test -p datafusion --test core_integration -- sql::sql_api::unsupported_copy_returns_error --exact ## Are there any user-facing changes? Yes. This adds the public SQLOptions::with_allow_copy builder method. It is backwards compatible because COPY remains enabled by default and with_allow_dml(false) continues to reject COPY.
1 parent b5e5c0e commit 3c2cd09

2 files changed

Lines changed: 55 additions & 1 deletion

File tree

‎datafusion/core/src/execution/context/mod.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2358,6 +2358,8 @@ pub struct SQLOptions {
23582358
allow_ddl: bool,
23592359
/// See [`Self::with_allow_dml`]
23602360
allow_dml: bool,
2361+
/// See [`Self::with_allow_copy`]
2362+
allow_copy: bool,
23612363
/// See [`Self::with_allow_statements`]
23622364
allow_statements: bool,
23632365
}
@@ -2367,6 +2369,7 @@ impl Default for SQLOptions {
23672369
Self {
23682370
allow_ddl: true,
23692371
allow_dml: true,
2372+
allow_copy: true,
23702373
allow_statements: true,
23712374
}
23722375
}
@@ -2390,6 +2393,15 @@ impl SQLOptions {
23902393
self
23912394
}
23922395

2396+
/// Should `COPY` commands be run? Defaults to `true`.
2397+
///
2398+
/// `COPY` commands are also disabled when DML is disabled with
2399+
/// [`Self::with_allow_dml`].
2400+
pub fn with_allow_copy(mut self, allow: bool) -> Self {
2401+
self.allow_copy = allow;
2402+
self
2403+
}
2404+
23932405
/// Should Statements such as (e.g. `SET VARIABLE and `BEGIN TRANSACTION` ...`) be run?. Defaults to `true`
23942406
pub fn with_allow_statements(mut self, allow: bool) -> Self {
23952407
self.allow_statements = allow;
@@ -2424,6 +2436,9 @@ impl<'n> TreeNodeVisitor<'n> for BadPlanVisitor<'_> {
24242436
LogicalPlan::Dml(dml) if !self.options.allow_dml => {
24252437
plan_err!("DML not supported: {}", dml.op)
24262438
}
2439+
LogicalPlan::Copy(_) if !self.options.allow_copy => {
2440+
plan_err!("COPY not supported")
2441+
}
24272442
LogicalPlan::Copy(_) if !self.options.allow_dml => {
24282443
plan_err!("DML not supported: COPY")
24292444
}

‎datafusion/core/tests/sql/sql_api.rs‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
use datafusion::prelude::*;
1919
use datafusion_common::tree_node::{TreeNode, TreeNodeRecursion};
20-
use datafusion_common::{TableReference, assert_contains};
20+
use datafusion_common::{TableReference, assert_batches_eq, assert_contains};
2121
use datafusion_expr::dml::MergeIntoOp;
2222
use datafusion_expr::{Expr, LogicalPlan, WriteOp};
2323

@@ -116,6 +116,45 @@ async fn unsupported_copy_returns_error() {
116116
ctx.sql_with_options(&sql, options).await.unwrap();
117117
}
118118

119+
#[tokio::test]
120+
async fn copy_can_be_disabled_independently_from_dml() {
121+
let tmpdir = TempDir::new().unwrap();
122+
let tmpfile = tmpdir.path().join("foo.parquet");
123+
124+
let ctx = SessionContext::new();
125+
ctx.sql("CREATE TABLE test (x int)").await.unwrap();
126+
127+
let options = SQLOptions::new().with_allow_copy(false);
128+
let sql = format!(
129+
"COPY (values(1)) TO '{}' STORED AS parquet",
130+
tmpfile.to_string_lossy()
131+
);
132+
let df = ctx.sql_with_options(&sql, options).await;
133+
assert_eq!(
134+
df.unwrap_err().strip_backtrace(),
135+
"Error during planning: COPY not supported"
136+
);
137+
138+
ctx.sql_with_options("INSERT INTO test VALUES (1)", options)
139+
.await
140+
.unwrap()
141+
.collect()
142+
.await
143+
.unwrap();
144+
145+
let results = ctx
146+
.sql_with_options("SELECT x FROM test", options)
147+
.await
148+
.unwrap()
149+
.collect()
150+
.await
151+
.unwrap();
152+
assert_batches_eq!(["+---+", "| x |", "+---+", "| 1 |", "+---+"], &results);
153+
154+
let options = options.with_allow_copy(true);
155+
ctx.sql_with_options(&sql, options).await.unwrap();
156+
}
157+
119158
#[tokio::test]
120159
async fn unsupported_statement_returns_error() {
121160
let ctx = SessionContext::new();

0 commit comments

Comments
 (0)