fix: do not allow relative path in DuckDB COPY statements #827
Conversation
There was a problem hiding this comment.
Seems reasonable to align here with postgres. It's indeed confusion that can easily lead to bad data corruption, because people often don't know the relative path of their postgres install.
Tests are failing though, so that one needs to be updated. Also, would be good to add a test for this behaviour.
src/pgduckdb_ddl.cpp
Outdated
| CopyStmt *stmt = castNode(CopyStmt, parsetree); | ||
| char *filename = stmt->filename; | ||
| if (!is_absolute_path(filename)) | ||
| ereport(ERROR, (errcode(ERRCODE_INVALID_NAME), errmsg("relative path not allowed for COPY to file"))); | ||
|
|
There was a problem hiding this comment.
Can you move this to the MakeDuckdbCopyQuery function? Also relative paths should still be allowed for reading, just not for writing. That's what posgres does.
There was a problem hiding this comment.
For me looks more reasonable to this part of code to IsAllowedStatement, is that ok?
There was a problem hiding this comment.
Yeah, sounds good (I'm assuming you mean the IsAllowedStatement function in src/utility/copy.cpp, because there's also one in pgduckdb_hooks.cpp)
Vanilla PostgreSQL does not allow COPY statements with relative path as it may lead to overriding database files and result in database corruption. This puts the same restriction on pg_duckdb. It's important to continue to allow COPY to URIs though.