You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "shyjsarah (via GitHub)" <gi...@apache.org> on 2026/09/28 06:22:55 UTC

[PR] feat: allow COPY to be disabled independently [datafusion]

shyjsarah opened a new pull request, #25824:
URL: https://github.com/apache/datafusion/pull/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.
   


-- 
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: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: allow COPY to be disabled independently [datafusion]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25824:
URL: https://github.com/apache/datafusion/pull/25824#issuecomment-6304962462

   ## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25824?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 82.93%. Comparing base ([`e1aa7d9`](https://app.codecov.io/gh/apache/datafusion/commit/e1aa7d956a5aa67452c9e8bd2a033599767055d8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`1ea9bcb`](https://app.codecov.io/gh/apache/datafusion/commit/1ea9bcb4f7f915f9b20e1a12c2e91ab016da6fde?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 302 commits behind head on main.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##             main   #25824      +/-   ##
   ==========================================
   + Coverage   82.55%   82.93%   +0.37%     
   ==========================================
     Files        1141     1154      +13     
     Lines      440209   457940   +17731     
     Branches   440209   457940   +17731     
   ==========================================
   + Hits       363403   379780   +16377     
   - Misses      54834    55136     +302     
   - Partials    21972    23024    +1052     
   ```
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25824?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: allow COPY to be disabled independently [datafusion]

Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #25824:
URL: https://github.com/apache/datafusion/pull/25824#discussion_r4323642454


##########
datafusion/core/tests/sql/sql_api.rs:
##########
@@ -116,6 +116,30 @@ async fn unsupported_copy_returns_error() {
     ctx.sql_with_options(&sql, options).await.unwrap();
 }
 
+#[tokio::test]
+async fn copy_can_be_disabled_independently_from_dml() {
+    let tmpdir = TempDir::new().unwrap();
+    let tmpfile = tmpdir.path().join("foo.parquet");
+
+    let ctx = SessionContext::new();
+    ctx.sql("CREATE TABLE test (x int)").await.unwrap();
+
+    let options = SQLOptions::new().with_allow_copy(false);
+    let sql = format!(
+        "COPY (values(1)) TO '{}' STORED AS parquet",
+        tmpfile.to_string_lossy()
+    );
+    let df = ctx.sql_with_options(&sql, options).await;
+    assert_eq!(
+        df.unwrap_err().strip_backtrace(),
+        "Error during planning: COPY not supported"
+    );
+
+    ctx.sql_with_options("INSERT INTO test VALUES (1)", options)
+        .await
+        .unwrap();

Review Comment:
   Could we also collect the INSERT DataFrame and query `test` to verify the row was written? The current test only checks that planning succeeds, so a readback would confirm that INSERT actually works with COPY disabled.



##########
datafusion/core/tests/sql/sql_api.rs:
##########
@@ -116,6 +116,30 @@ async fn unsupported_copy_returns_error() {
     ctx.sql_with_options(&sql, options).await.unwrap();
 }
 
+#[tokio::test]
+async fn copy_can_be_disabled_independently_from_dml() {
+    let tmpdir = TempDir::new().unwrap();
+    let tmpfile = tmpdir.path().join("foo.parquet");
+
+    let ctx = SessionContext::new();
+    ctx.sql("CREATE TABLE test (x int)").await.unwrap();
+
+    let options = SQLOptions::new().with_allow_copy(false);

Review Comment:
   Could we also test re-enabling COPY with `options.with_allow_copy(true)` and verify that the same COPY statement is accepted? This would add coverage for both settings of the new flag.



-- 
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: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: allow COPY to be disabled independently [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #25824:
URL: https://github.com/apache/datafusion/pull/25824


-- 
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: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: allow COPY to be disabled independently [datafusion]

Posted by "shyjsarah (via GitHub)" <gi...@apache.org>.
shyjsarah commented on code in PR #25824:
URL: https://github.com/apache/datafusion/pull/25824#discussion_r4323981126


##########
datafusion/core/tests/sql/sql_api.rs:
##########
@@ -116,6 +116,30 @@ async fn unsupported_copy_returns_error() {
     ctx.sql_with_options(&sql, options).await.unwrap();
 }
 
+#[tokio::test]
+async fn copy_can_be_disabled_independently_from_dml() {
+    let tmpdir = TempDir::new().unwrap();
+    let tmpfile = tmpdir.path().join("foo.parquet");
+
+    let ctx = SessionContext::new();
+    ctx.sql("CREATE TABLE test (x int)").await.unwrap();
+
+    let options = SQLOptions::new().with_allow_copy(false);
+    let sql = format!(
+        "COPY (values(1)) TO '{}' STORED AS parquet",
+        tmpfile.to_string_lossy()
+    );
+    let df = ctx.sql_with_options(&sql, options).await;
+    assert_eq!(
+        df.unwrap_err().strip_backtrace(),
+        "Error during planning: COPY not supported"
+    );
+
+    ctx.sql_with_options("INSERT INTO test VALUES (1)", options)
+        .await
+        .unwrap();

Review Comment:
   Thanks! Updated the test to collect the INSERT DataFrame, then query `test` with COPY still disabled and assert that it contains exactly one row with `x = 1`.



##########
datafusion/core/tests/sql/sql_api.rs:
##########
@@ -116,6 +116,30 @@ async fn unsupported_copy_returns_error() {
     ctx.sql_with_options(&sql, options).await.unwrap();
 }
 
+#[tokio::test]
+async fn copy_can_be_disabled_independently_from_dml() {
+    let tmpdir = TempDir::new().unwrap();
+    let tmpfile = tmpdir.path().join("foo.parquet");
+
+    let ctx = SessionContext::new();
+    ctx.sql("CREATE TABLE test (x int)").await.unwrap();
+
+    let options = SQLOptions::new().with_allow_copy(false);

Review Comment:
   Added a check that re-enables COPY with `options.with_allow_copy(true)` and verifies that the same COPY statement is accepted.



-- 
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: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org