Revamp MotherDuck settings and secret management #668

Y-- · 2025-03-14T15:51:58Z

Revamp MotherDuck settings and secret management

This PR removes any GUC setting for MotherDuck and replaces it with a combination of SERVER and USER MAPPING for a newly implemented pg_duckdb Foreign Data Wrapper.

New MotherDuck setup:

Users may now define one (and at most one) SERVER for motherduck:

CREATE SERVER <server name> 
FOREIGN DATA WRAPPER pg_duckdb
TYPE 'motherduck'
[OPTIONS (
     [tables_owner_role 'some user'],	-- the role bgw assigns to MD tables 
										-- (by default server creator/owner) 
     [default_database]					-- Which MD DB is synced (default "my_db")
)];

Once the SERVER is created, for MotherDuck to be enabled, it needs at least one USER MAPPING:

CREATE USER MAPPING FOR <PG user> 
SERVER <FDW server name> 
OPTIONS (
  token '<some token>'
);

Removed GUC settings:

The following GUC settings were thus removed and are now obsolete:

motherduck_enabled: now implied by a SERVER and USER MAPPING
motherduck_token: now defined in a USER MAPPING
motherduck_postgres_database: now implied by the database in which the SERVER & USER MAPPING are defined
motherduck_default_database: now defined as an option of the motherduck server

Convenience function:

In order to enable MotherDuck support easily, we also provide the following helper:

-- Token will be read from environment if not provided
-- Default database is `my_db` if not provided
SELECT duckdb.enable_motherduck('<token>', '<default_motherduck_db>')

src/pgduckdb_ddl.cpp

src/pgduckdb_fdw.cpp

src/pgduckdb_userdata_cache.cpp

test/regression/expected/foreign_data_wrapper.out

src/pgduckdb_background_worker.cpp

src/pgduckdb_ddl.cpp

src/pgduckdb_duckdb.cpp

sql/pg_duckdb--0.3.0--0.4.0.sql

src/pgduckdb_fdw.cpp

test/regression/expected/foreign_data_wrapper.out

test/pycheck/conftest.py

docker/README.md

README.md

test/pycheck/conftest.py

src/pgduckdb_options.cpp

sql/pg_duckdb--0.3.0--0.4.0.sql

We were checking ret instead of `errno` for the error codes. That does not work on Linux at least. `ret` is always -1 there. So the additional background workers would continuously restart. This also uses the %m in other cases instead of manually calling `strerror`.

In #668 we changed the configuration of MotherDuck tokens to use `FOREIGN SERVER` + `USER MAPPING` under the hood, along with some helper functions for the most common usages to get people started. In this PR, we align our secrets management with the same mechanism. That way different PG users can use different S3 tokens to connect to the same bucket. We also introduce two helper functions `create_simple_secret` and `create_azure_secret` for the most basic usage. We are now validating secrets using DuckDB directly and store whatever the user provides as long as it leads to a valid secret creation. This allow to support virtually any kind of secrets type and their options that DuckDB supports. Since `SERVER` options are visible to any PG user, the sensitive ones such as `connection_string`, `secret`, `session_token` and `token` are prohibited to in its definition and must be stored in the `USER MAPPING`. DuckDB secrets are created by concatenating `SERVER` and `USER MAPPING` options (if they exists). Implements #683 Fixes #104

YuweiXiao · 2025-08-12T03:08:24Z

Do we support SCOPE option? If not, I could help implement.

JelteF · 2025-08-12T08:21:42Z

It definitely should. It now validates the options with duckdb. If it doesn't in practice then there's a bug.

YuweiXiao · 2025-08-12T08:36:12Z

Manually creating the server with option is working. But we didn't expose it through the udf create_simple_secret

CREATE SERVER my_server TYPE 's3' FOREIGN DATA WRAPPER duckdb OPTIONS (SCOPE 's3://my-other-bucket');

JelteF · 2025-08-12T08:43:56Z

The idea was that create_simple_secret was only for the most simple usecases. And then have more complicated ones use the regular create server syntax. I guess scope might be common enough that it warrants being an argument in create_simple_secret.

YuweiXiao · 2025-08-12T11:07:17Z

By the way, the foreign data wrapper DuckDB is not granted to normal users. So only superusers are able to create secrets, while normal users can only utilize them. Is this by design, or can we loosen the permissions?

Also, normal user can see secrets by select duckdb.raw_query($$FROM duckdb_secrets()$$)

Y-- force-pushed the yl/secrets-md-fdw branch from 2036a37 to 7e5137b Compare March 14, 2025 16:07

JelteF reviewed Mar 16, 2025

View reviewed changes

src/pgduckdb_ddl.cpp Show resolved Hide resolved

src/pgduckdb_fdw.cpp Outdated Show resolved Hide resolved

src/pgduckdb_fdw.cpp Outdated Show resolved Hide resolved

src/pgduckdb_fdw.cpp Outdated Show resolved Hide resolved

src/pgduckdb_userdata_cache.cpp Show resolved Hide resolved

Y-- force-pushed the yl/secrets-md-fdw branch 12 times, most recently from 645dbd1 to 2c53ebd Compare March 19, 2025 08:20

Y-- commented Mar 19, 2025

View reviewed changes

test/regression/expected/foreign_data_wrapper.out Outdated Show resolved Hide resolved

Y-- force-pushed the yl/secrets-md-fdw branch 2 times, most recently from f43c504 to 4b9eb9c Compare March 19, 2025 09:40

JelteF requested changes Mar 19, 2025

View reviewed changes

Y-- force-pushed the yl/secrets-md-fdw branch 2 times, most recently from 448567c to d822314 Compare March 19, 2025 10:07