duckdb-r icon indicating copy to clipboard operation
duckdb-r copied to clipboard

Creating new databases fails on network drives if user has no privileges on underlying folder due to `normalizePath` with `mustWork = TRUE`

Open erikvona opened this issue 4 months ago • 0 comments

Recently, in https://github.com/duckdb/duckdb-r/commit/27bba58e12d265527e0c64da97dd3ab63d2b2c8a, the R package for duckdb has been updated to normalize paths before connecting, using normalizePath with mustWork = TRUE

However, normalizePath will fail on network drives if the user does not have sufficient permissions on all underlying directories, even when the path is valid and the user can access it. This causes existing code to break with the new package version.

Quoting from the docs of normalizePath:

Converting to an absolute file path can fail for a large number of reasons. The most common are

  • ...
  • A component before the last is not a directory, or there is insufficient permission to read the directory.
  • ...

I can work around this by monkeypatching the package, e.g.

path_normalize_noerror <- function(path) {
  DBDIR_MEMORY <- ":memory:"
  if (path == "" || path == DBDIR_MEMORY) {
    return(DBDIR_MEMORY)
  }

  out <- normalizePath(path, mustWork = FALSE)

  # Stable results are only guaranteed if the file exists
  if (!file.exists(out)) {
    on.exit(unlink(out))
    writeLines(character(), out)
    out <- normalizePath(out, mustWork = FALSE)
  }
  out
}
assignInNamespace("path_normalize", path_normalize_noerror, "duckdb")

However, I'd rather not do any monkeypatching. After monkeypatching, everything works fine.

Note that this operation is safe in that either file.exists(out) is TRUE and thus normalizePath(out, mustWork = TRUE) would never be called, or that writeLines(character(), out) is called and thus the path must be valid (as a file can be created on that path).

I'm not entirely sure why mustWork = TRUE was used there. I'd gladly submit a pull request changing that to FALSE if there's no good reason for it.

erikvona avatar Oct 04 '24 11:10 erikvona