Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove FORMAT <..> backwards compatibility options from COPY #9985

Merged
merged 4 commits into from
Apr 8, 2024
Merged

Remove FORMAT <..> backwards compatibility options from COPY #9985

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e70d8dd
Revert "Add test for reading back file created with FORMAT options (#…
tinfoil-knight Apr 7, 2024
6b507f7
Revert "support format in options of COPY command (#9744)"
tinfoil-knight Apr 7, 2024
1878098
Merge branch 'main' into 9905-remove-format
tinfoil-knight Apr 8, 2024
48ae7fb
update docs and example to remove old syntax
tinfoil-knight Apr 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions datafusion/sql/src/statement.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
return plan_err!("Unsupported Value in COPY statement {}", value);
}
};
if !(key.contains('.') || key == "format") {
if !(&key.contains('.')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

// If config does not belong to any namespace, assume it is
// a format option and apply the format prefix for backwards
// compatibility.
Expand All @@ -866,16 +866,12 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
FileType::from_str(&file_type).map_err(|_| {
DataFusionError::Configuration(format!("Unknown FileType {}", file_type))
})?
} else if let Some(format) = options.remove("format") {
// try to infer file format from the "format" key in options
FileType::from_str(&format)
.map_err(|e| DataFusionError::Configuration(format!("{}", e)))?
} else {
let e = || {
DataFusionError::Configuration(
"Format not explicitly set and unable to get file extension! Use STORED AS to define file format."
.to_string(),
)
"Format not explicitly set and unable to get file extension! Use STORED AS to define file format."
.to_string(),
)
};
// try to infer file format from file extension
let extension: &str = &Path::new(&statement.target)
Expand Down
12 changes: 0 additions & 12 deletions datafusion/sql/tests/sql_integration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,18 +444,6 @@ CopyTo: format=csv output_url=output.csv options: ()
quick_test(sql, plan);
}

#[test]
fn plan_copy_stored_as_priority() {
let sql = "COPY (select * from (values (1))) to 'output/' STORED AS CSV OPTIONS (format json)";
let plan = r#"
CopyTo: format=csv output_url=output/ options: (format json)
Projection: column1
Values: (Int64(1))
"#
.trim();
quick_test(sql, plan);
}

#[test]
fn plan_insert() {
let sql =
Expand Down
59 changes: 0 additions & 59 deletions datafusion/sqllogictest/test_files/copy.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,65 +514,6 @@ OPTIONS (
);


# Format Options Support with format in OPTIONS
#
# i.e. COPY { table_name | query } TO 'file_name' OPTIONS (format <format-name>, ...)

# Ensure that the format is set in the OPTIONS, not extension
query I
COPY (select * from (values (1))) to 'test_files/scratch/copy/foo.dat'
OPTIONS (format parquet);
----
1

statement ok
CREATE EXTERNAL TABLE foo_dat STORED AS PARQUET LOCATION 'test_files/scratch/copy/foo.dat';

query I
select * from foo_dat;
----
1

statement ok
DROP TABLE foo_dat;


query I
COPY (select * from (values (1))) to 'test_files/scratch/copy'
OPTIONS (format parquet);
----
1

query I
COPY (select * from (values (1))) to 'test_files/scratch/copy/'
OPTIONS (format parquet, compression 'zstd(10)');
----
1

query I
COPY (select * from (values (1))) to 'test_files/scratch/copy/'
OPTIONS (format json, compression gzip);
----
1

query I
COPY (select * from (values (1))) to 'test_files/scratch/copy/'
OPTIONS (
format csv,
has_header false,
compression xz,
datetime_format '%FT%H:%M:%S.%9f',
delimiter ';',
null_value 'NULLVAL'
);
----
1

query error DataFusion error: Invalid or Unsupported Configuration: This feature is not implemented: Unknown FileType: NOTVALIDFORMAT
COPY (select * from (values (1))) to 'test_files/scratch/copy/'
OPTIONS (format notvalidformat, compression 'zstd(5)');


# Error cases:

# Copy from table with options
Expand Down
Loading