Skip to content

Conversation

teo-tsirpanis
Copy link
Member

@teo-tsirpanis teo-tsirpanis commented Jul 10, 2025

Fixes CORE-268.

Tests added. The Windows implementation already creates parent directories.


TYPE: BUG
DESC: Fixed the tiledb_vfs_create_dir function to automatically create non-existent parent directories on POSIX.


TYPE: BUG
DESC: Fixed the tiledb_vfs_create_dir function to not fail when the directory already exists.

@teo-tsirpanis teo-tsirpanis force-pushed the teo/create-dir-recursive branch from f9b7a73 to d3b62d0 Compare July 10, 2025 19:06
@teo-tsirpanis teo-tsirpanis force-pushed the teo/create-dir-recursive branch 2 times, most recently from e4a886f to a31223f Compare July 17, 2025 17:52
@teo-tsirpanis teo-tsirpanis changed the title [POSIX] Create directories recursively if a parent directory doesn't exist. Fix semantics of tiledb_vfs_create_dir on local filesystem. Jul 17, 2025
@teo-tsirpanis teo-tsirpanis force-pushed the teo/create-dir-recursive branch from a31223f to 63781f6 Compare July 17, 2025 18:13
@teo-tsirpanis teo-tsirpanis force-pushed the teo/create-dir-recursive branch from 63781f6 to 728e9b4 Compare July 17, 2025 18:14
@teo-tsirpanis teo-tsirpanis marked this pull request as ready for review July 17, 2025 18:14
if (status != 0) {
auto err = errno;
if (err == EEXIST) {
// Do not fail if directory already existed.
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the documented behavior, so I'm treating this change as a bug fix.

* - On all other backends, if the directory exists, the function
* just succeeds without doing anything.

Copy link
Member

@kounelisagis kounelisagis left a comment

Choose a reason for hiding this comment

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

LGTM! The example makes sense, and I also played around a bit in TileDB-Py before and after this change and everything works fine.

auto status = mkdir(path.c_str(), directory_permissions_);
if (status != 0) {
auto err = errno;
if (err == EEXIST) {
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to check whether err != EEXIST and throw the error in that case, rather than returning when err == EEXIST.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the current way is better; it indicates that throwing on an error is the general case, and ignoring the error is the special case.

@teo-tsirpanis teo-tsirpanis merged commit b3e087f into main Jul 18, 2025
56 checks passed
@teo-tsirpanis teo-tsirpanis deleted the teo/create-dir-recursive branch July 18, 2025 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants