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.

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

Fix #12225: revert OVERWRITE_OR_IGNORE to previous behavior, move new behavior to OVERWRITE flag #12240

Merged
merged 3 commits into from
May 26, 2024

Conversation

Mytherin
Copy link
Collaborator

Fixes #12225

This PR reverts OVERWRITE_OR_IGNORE to the previous behavior where files are overwritten if they exist, or ignored (left alone) if they do not. The new behavior introduced in #11787 is instead moved to a new flag (OVERWRITE). The new flag is more explicit in what it does, and is also explicitly disabled for remote file systems (e.g. S3) where we don't support removing files yet.

@duckdb-draftbot duckdb-draftbot marked this pull request as draft May 25, 2024 12:17
@Mytherin Mytherin marked this pull request as ready for review May 25, 2024 12:17
@@ -39,7 +39,7 @@ unique_ptr<LogicalOperator> LogicalCopyToFile::Deserialize(Deserializer &deseria
auto file_path = deserializer.ReadProperty<string>(200, "file_path");
auto use_tmp_file = deserializer.ReadProperty<bool>(201, "use_tmp_file");
auto filename_pattern = deserializer.ReadProperty<FilenamePattern>(202, "filename_pattern");
auto overwrite_or_ignore = deserializer.ReadProperty<bool>(203, "overwrite_or_ignore");
Copy link
Contributor

@Tishj Tishj May 25, 2024

Choose a reason for hiding this comment

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

Don't we need some way to map true to CopyOverwriteMode::COPY_OVERWRITE_OR_IGNORE at deserialization for backwards compatibility?
Or is that not as important for LogicalCopyToFile?

Copy link
Collaborator Author

@Mytherin Mytherin May 25, 2024

Choose a reason for hiding this comment

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

Booleans are serialized as single bytes - where 0 is false and 1 is true. The new enum is also serialized as a single byte (CopyOverwriteMode : uint8_t), where 0 is COPY_ERROR_ON_CONFLICT and 1 is COPY_OVERWRITE . So this should work correctly already.

@Mytherin Mytherin merged commit ae48651 into duckdb:main May 26, 2024
41 checks passed
@Mytherin Mytherin added the Needs Documentation Use for issues or PRs that require changes in the documentation label May 26, 2024
github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request Jun 5, 2024
Merge pull request duckdb/duckdb#12240 from Mytherin/overwriteorignore
@Mytherin Mytherin deleted the overwriteorignore branch June 7, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Documentation Use for issues or PRs that require changes in the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hive partitioned append/write 0.10.3
2 participants