Update default settings

include/pgduckdb/pgduckdb_duckdb.hpp

@@ -67,6 +67,7 @@ class DuckDBManager {
 		extensions_table_current_seq = seq;
 	}
 
+	bool disabled_filesystems_is_set;
 	int secret_table_num_rows;
 	int64 secret_table_current_seq;
 	int64 extensions_table_current_seq;

src/pgduckdb.cpp

@@ -20,8 +20,8 @@ char *duckdb_motherduck_postgres_database = strdup("postgres");
 char *duckdb_postgres_role = strdup("");
 
 int duckdb_maximum_threads = -1;
-char *duckdb_maximum_memory = NULL;
-char *duckdb_disabled_filesystems = NULL;
+char *duckdb_maximum_memory = strdup("4GB");
+char *duckdb_disabled_filesystems = strdup("LocalFileSystem");
 bool duckdb_enable_external_access = true;
 bool duckdb_allow_unsigned_extensions = false;
 

src/pgduckdb_duckdb.cpp

@@ -86,11 +86,6 @@ DuckDBManager::Initialize() {
 
 	auto &context = *connection->context;
 
-	if (duckdb_disabled_filesystems != NULL) {
-		pgduckdb::DuckDBQueryOrThrow(context,
-		                             "SET disabled_filesystems='" + std::string(duckdb_disabled_filesystems) + "'");
-	}
-
 	auto &db_manager = duckdb::DatabaseManager::Get(context);
 	default_dbname = db_manager.GetDefaultDatabase(context);
 	pgduckdb::DuckDBQueryOrThrow(context, "ATTACH DATABASE 'pgduckdb' (TYPE pgduckdb)");
@@ -198,7 +193,6 @@ DuckDBManager::LoadExtensions(duckdb::ClientContext &context) {
 			continue;
 		}
 
-		DuckDBQueryOrThrow(context, "INSTALL " + extension.name);
 		DuckDBQueryOrThrow(context, "LOAD " + extension.name);
 	}
 }
@@ -230,6 +224,21 @@ DuckDBManager::CreateConnection() {
 	    duckdb::StringUtil::Format("SET http_file_cache_dir TO '%s';", CreateOrGetDirectoryPath("duckdb_cache"));
 	context.Query(http_file_cache_set_dir_query, false);
 
+	if (duckdb_disabled_filesystems != NULL && !superuser()) {
+		/*
+		 * DuckDB does not allow us to disable this setting on the
+		 * database after the DuckDB connection is created for a non
+		 * superuser, any further connections will inherit this
+		 * restriction. This means that once a non-superuser used a
+		 * DuckDB connection in aside a Postgres backend, any loter
+		 * connection will inherit these same filesystem restrictions.
+		 * This shouldn't be a problem in practice.
+		 */
+		pgduckdb::DuckDBQueryOrThrow(context,
+		                             "SET disabled_filesystems='" + std::string(duckdb_disabled_filesystems) + "'");
+		instance.disabled_filesystems_is_set = true;
+	}
+
 	return connection;
 }
 

src/pgduckdb_options.cpp

@@ -7,6 +7,7 @@ extern "C" {
 #include "access/relation.h"
 #include "access/table.h"
 #include "access/xact.h"
+#include "executor/spi.h"
 #include "catalog/indexing.h"
 #include "catalog/namespace.h"
 #include "utils/builtins.h"
@@ -115,36 +116,25 @@ ReadDuckdbExtensions() {
 }
 
 static bool
-DuckdbInstallExtension(Datum name) {
+DuckdbInstallExtension(Datum name_datum) {
 
-	auto extension_name = DatumToString(name);
+	auto extension_name = DatumToString(name_datum);
 	auto install_extension_command = duckdb::StringUtil::Format("INSTALL %s;", extension_name.c_str());
-	{
-		auto connection = pgduckdb::DuckDBManager::CreateConnection();
-		auto res = connection->context->Query(install_extension_command, false);
-
-		if (res->HasError()) {
-			elog(WARNING, "(PGDuckDB/DuckdbInstallExtension) %s", res->GetError().c_str());
-			return false;
-		}
-	}
-
-	bool nulls[Natts_duckdb_extension] = {0};
-	Datum values[Natts_duckdb_extension] = {0};
-
-	values[Anum_duckdb_extension_name - 1] = name;
-	values[Anum_duckdb_extension_enable - 1] = 1;
-
-	/* create heap tuple and insert into catalog table */
-	Relation duckdb_extensions_relation = relation_open(ExtensionsTableRelationId(), RowExclusiveLock);
-	TupleDesc tuple_descr = RelationGetDescr(duckdb_extensions_relation);
-
-	/* inserting extension record */
-	HeapTuple new_tuple = heap_form_tuple(tuple_descr, values, nulls);
-	CatalogTupleInsert(duckdb_extensions_relation, new_tuple);
-
-	CommandCounterIncrement();
-	relation_close(duckdb_extensions_relation, RowExclusiveLock);
+	pgduckdb::DuckDBQueryOrThrow(install_extension_command);
+
+	Oid arg_types[] = {TEXTOID};
+	Datum values[] = {name_datum};
+
+	SPI_connect();
+	auto ret = SPI_execute_with_args(R"(
+		INSERT INTO duckdb.extensions (name, enabled)
+		VALUES ($1, true)
+		ON CONFLICT (name) DO UPDATE SET enabled = true
+		)",
+	                                 lengthof(arg_types), arg_types, values, NULL, false, 0);
+	if (ret != SPI_OK_INSERT)
+		elog(ERROR, "SPI_exec failed: error code %s", SPI_result_code_string(ret));
+	SPI_finish();
 
 	return true;
 }

test/regression/expected/extensions.out

@@ -21,11 +21,18 @@ SELECT last_value FROM duckdb.extensions_table_seq;
 (1 row)
 
 -- INSERT SHOULD TRIGGER UPDATE OF EXTENSIONS
-INSERT INTO duckdb.extensions (name, enabled) VALUES ('icu', TRUE);
+SELECT duckdb.install_extension('icu');
+ install_extension 
+-------------------
+ t
+(1 row)
+
+-- Increases the sequence twice because we use ON CONFLICT DO UPDATE. So
+-- the trigger fires for both INSERT and UPDATE internally.
 SELECT last_value FROM duckdb.extensions_table_seq;
  last_value 
 ------------
-          2
+          3
 (1 row)
 
 SELECT * FROM duckdb.raw_query($$ SELECT extension_name, loaded, installed FROM duckdb_extensions() WHERE loaded and extension_name != 'jemalloc' $$);
@@ -44,11 +51,47 @@ pgduckdb	true	false
 
 (1 row)
 
-INSERT INTO duckdb.extensions (name, enabled) VALUES ('aws', TRUE);
+-- Check that we can rerun this without issues
+SELECT duckdb.install_extension('icu');
+ install_extension 
+-------------------
+ t
+(1 row)
+
+-- Increases the sequence twice because we use ON CONFLICT DO UPDATE. So
+-- the trigger fires for both INSERT and UPDATE internally.
 SELECT last_value FROM duckdb.extensions_table_seq;
  last_value 
 ------------
-          3
+          5
+(1 row)
+
+SELECT * FROM duckdb.raw_query($$ SELECT extension_name, loaded, installed FROM duckdb_extensions() WHERE loaded and extension_name != 'jemalloc' $$);
+NOTICE:  result: extension_name	loaded	installed	
+VARCHAR	BOOLEAN	BOOLEAN	
+[ Rows: 5]
+cached_httpfs	true	true
+icu	true	true
+json	true	true
+parquet	true	true
+pgduckdb	true	false
+
+
+ raw_query 
+-----------
+
+(1 row)
+
+SELECT duckdb.install_extension('aws');
+ install_extension 
+-------------------
+ t
+(1 row)
+
+SELECT last_value FROM duckdb.extensions_table_seq;
+ last_value 
+------------
+          7
 (1 row)
 
 SELECT * FROM duckdb.raw_query($$ SELECT extension_name, loaded, installed FROM duckdb_extensions() WHERE loaded and extension_name != 'jemalloc' $$);
@@ -74,7 +117,7 @@ DELETE FROM duckdb.extensions WHERE name = 'aws';
 SELECT last_value FROM duckdb.extensions_table_seq;
  last_value 
 ------------
-          4
+          8
 (1 row)
 
 SELECT * FROM duckdb.raw_query($$ SELECT extension_name, loaded, installed FROM duckdb_extensions() WHERE loaded and extension_name != 'jemalloc' $$);

test/regression/expected/non_superuser.out

@@ -32,6 +32,9 @@ ERROR:  permission denied for function install_extension
 SELECT * FROM duckdb.cache('some hacky sql', 'some more hacky sql');
 ERROR:  permission denied for function cache
 SET duckdb.force_execution = true;
+-- read_csv from the local filesystem should be disallowed
+SELECT count("sepal.length") FROM read_csv('../../data/iris.csv') AS ("sepal.length" FLOAT);
+ERROR:  (PGDuckDB/CreatePlan) Prepared query returned an error: 'Permission Error: File system LocalFileSystem has been disabled by configuration
 -- Should fail because DuckDB execution is not allowed for this user
 SET ROLE user3;
 SELECT * FROM t;

test/regression/sql/extensions.sql

@@ -7,13 +7,25 @@ SELECT last_value FROM duckdb.extensions_table_seq;
 
 -- INSERT SHOULD TRIGGER UPDATE OF EXTENSIONS
 
-INSERT INTO duckdb.extensions (name, enabled) VALUES ('icu', TRUE);
+SELECT duckdb.install_extension('icu');
 
+-- Increases the sequence twice because we use ON CONFLICT DO UPDATE. So
+-- the trigger fires for both INSERT and UPDATE internally.
 SELECT last_value FROM duckdb.extensions_table_seq;
 
 SELECT * FROM duckdb.raw_query($$ SELECT extension_name, loaded, installed FROM duckdb_extensions() WHERE loaded and extension_name != 'jemalloc' $$);
 
-INSERT INTO duckdb.extensions (name, enabled) VALUES ('aws', TRUE);
+-- Check that we can rerun this without issues
+SELECT duckdb.install_extension('icu');
+
+-- Increases the sequence twice because we use ON CONFLICT DO UPDATE. So
+-- the trigger fires for both INSERT and UPDATE internally.
+SELECT last_value FROM duckdb.extensions_table_seq;
+
+SELECT * FROM duckdb.raw_query($$ SELECT extension_name, loaded, installed FROM duckdb_extensions() WHERE loaded and extension_name != 'jemalloc' $$);
+
+
+SELECT duckdb.install_extension('aws');
 
 SELECT last_value FROM duckdb.extensions_table_seq;
 

test/regression/sql/non_superuser.sql

@@ -24,6 +24,8 @@ SELECT * FROM duckdb.install_extension('some hacky sql');
 SELECT * FROM duckdb.cache('some hacky sql', 'some more hacky sql');
 SET duckdb.force_execution = true;
 
+-- read_csv from the local filesystem should be disallowed
+SELECT count("sepal.length") FROM read_csv('../../data/iris.csv') AS ("sepal.length" FLOAT);
 -- Should fail because DuckDB execution is not allowed for this user
 SET ROLE user3;
 SELECT * FROM t;