Merge pull request #168 from Mytherin/explicitschema

Fix #148: correctly pass schema into Postgres for many DDL operations

src/include/storage/postgres_catalog_set.hpp

@@ -19,7 +19,7 @@ class PostgresTransaction;
 
 class PostgresCatalogSet {
 public:
-	PostgresCatalogSet(Catalog &catalog);
+	PostgresCatalogSet(Catalog &catalog, bool is_loaded);
 
 	optional_ptr<CatalogEntry> GetEntry(ClientContext &context, const string &name);
 	void DropEntry(ClientContext &context, DropInfo &info);

src/storage/postgres_catalog_set.cpp

@@ -3,7 +3,7 @@
 #include "duckdb/parser/parsed_data/drop_info.hpp"
 namespace duckdb {
 
-PostgresCatalogSet::PostgresCatalogSet(Catalog &catalog) : catalog(catalog), is_loaded(false) {
+PostgresCatalogSet::PostgresCatalogSet(Catalog &catalog, bool is_loaded_p) : catalog(catalog), is_loaded(is_loaded_p) {
 }
 
 optional_ptr<CatalogEntry> PostgresCatalogSet::GetEntry(ClientContext &context, const string &name) {

src/storage/postgres_delete.cpp

@@ -15,9 +15,11 @@ PostgresDelete::PostgresDelete(LogicalOperator &op, TableCatalogEntry &table, id
 //===--------------------------------------------------------------------===//
 // States
 //===--------------------------------------------------------------------===//
-string GetDeleteSQL(const string &table_name, const string &ctid_list) {
+string GetDeleteSQL(const PostgresTableEntry &table, const string &ctid_list) {
 	string result;
-	result = "DELETE FROM " + KeywordHelper::WriteOptionallyQuoted(table_name);
+	result = "DELETE FROM ";
+	result += KeywordHelper::WriteQuoted(table.schema.name, '"') + ".";
+	result += KeywordHelper::WriteOptionallyQuoted(table.name);
 	result += " WHERE ctid IN (" + ctid_list + ")";
 	return result;
 }
@@ -36,7 +38,7 @@ class PostgresDeleteGlobalState : public GlobalSinkState {
 			return;
 		}
 		auto &transaction = PostgresTransaction::Get(context, table.catalog);
-		transaction.Query(GetDeleteSQL(table.name, ctid_list));
+		transaction.Query(GetDeleteSQL(table, ctid_list));
 		ctid_list = "";
 	}
 };

src/storage/postgres_index.cpp

@@ -3,6 +3,7 @@
 #include "duckdb/parser/statement/create_statement.hpp"
 #include "duckdb/planner/operator/logical_extension_operator.hpp"
 #include "duckdb/catalog/catalog_entry/table_catalog_entry.hpp"
+#include "duckdb/parser/parsed_data/drop_info.hpp"
 
 namespace duckdb {
 
@@ -16,10 +17,26 @@ PostgresCreateIndex::PostgresCreateIndex(unique_ptr<CreateIndexInfo> info, Table
 SourceResultType PostgresCreateIndex::GetData(ExecutionContext &context, DataChunk &chunk,
                                               OperatorSourceInput &input) const {
 	auto &catalog = table.catalog;
-	if (info->catalog == INVALID_CATALOG && info->schema == catalog.GetName()) {
-		info->schema = DEFAULT_SCHEMA;
+	auto &schema = table.schema;
+	auto existing = schema.GetEntry(catalog.GetCatalogTransaction(context.client), CatalogType::INDEX_ENTRY, info->index_name);
+	if (existing) {
+		switch(info->on_conflict) {
+			case OnCreateConflict::IGNORE_ON_CONFLICT:
+				return SourceResultType::FINISHED;
+			case OnCreateConflict::ERROR_ON_CONFLICT:
+				throw BinderException("Index with name \"%s\" already exists in schema \"%s\"", info->index_name, table.schema.name);
+			case OnCreateConflict::REPLACE_ON_CONFLICT: {
+				DropInfo drop_info;
+				drop_info.type = CatalogType::INDEX_ENTRY;
+				drop_info.schema = info->schema;
+				drop_info.name = info->index_name;
+				schema.DropEntry(context.client, drop_info);
+				break;
+			}
+			default:
+				throw InternalException("Unsupported on create conflict");
+		}
 	}
-	auto &schema = catalog.GetSchema(context.client, info->schema);
 	schema.CreateIndex(context.client, *info, table);
 
 	return SourceResultType::FINISHED;

src/storage/postgres_index_set.cpp

@@ -8,7 +8,7 @@
 namespace duckdb {
 
 PostgresIndexSet::PostgresIndexSet(PostgresSchemaEntry &schema, unique_ptr<PostgresResultSlice> index_result_p)
-    : PostgresCatalogSet(schema.ParentCatalog()), schema(schema), index_result(std::move(index_result_p)) {
+    : PostgresCatalogSet(schema.ParentCatalog(), !index_result_p), schema(schema), index_result(std::move(index_result_p)) {
 }
 
 string PostgresIndexSet::GetInitializeQuery() {
@@ -57,6 +57,7 @@ string PGGetCreateIndexSQL(CreateIndexInfo &info, TableCatalogEntry &tbl) {
 	sql += " INDEX ";
 	sql += KeywordHelper::WriteOptionallyQuoted(info.index_name);
 	sql += " ON ";
+	sql += KeywordHelper::WriteOptionallyQuoted(tbl.schema.name) + ".";
 	sql += KeywordHelper::WriteOptionallyQuoted(tbl.name);
 	sql += "(";
 	for (idx_t i = 0; i < info.parsed_expressions.size(); i++) {

src/storage/postgres_schema_entry.cpp

@@ -63,9 +63,10 @@ optional_ptr<CatalogEntry> PostgresSchemaEntry::CreateIndex(ClientContext &conte
 	return indexes.CreateIndex(context, info, table);
 }
 
-string PGGetCreateViewSQL(CreateViewInfo &info) {
+string PGGetCreateViewSQL(PostgresSchemaEntry &schema, CreateViewInfo &info) {
 	string sql;
 	sql = "CREATE VIEW ";
+	sql += KeywordHelper::WriteOptionallyQuoted(schema.name) + ".";
 	sql += KeywordHelper::WriteOptionallyQuoted(info.view_name);
 	sql += " ";
 	if (!info.aliases.empty()) {
@@ -100,7 +101,7 @@ optional_ptr<CatalogEntry> PostgresSchemaEntry::CreateView(CatalogTransaction tr
 		}
 	}
 	auto &postgres_transaction = GetPostgresTransaction(transaction);
-	postgres_transaction.Query(PGGetCreateViewSQL(info));
+	postgres_transaction.Query(PGGetCreateViewSQL(*this, info));
 	return tables.ReloadEntry(transaction.GetContext(), info.view_name);
 }
 

src/storage/postgres_schema_set.cpp

@@ -8,7 +8,7 @@
 
 namespace duckdb {
 
-PostgresSchemaSet::PostgresSchemaSet(Catalog &catalog) : PostgresCatalogSet(catalog) {
+PostgresSchemaSet::PostgresSchemaSet(Catalog &catalog) : PostgresCatalogSet(catalog, false) {
 }
 
 vector<unique_ptr<PostgresResultSlice>> SliceResult(PostgresResult &schemas, unique_ptr<PostgresResult> to_slice_ptr) {

src/storage/postgres_table_set.cpp

@@ -15,7 +15,7 @@
 namespace duckdb {
 
 PostgresTableSet::PostgresTableSet(PostgresSchemaEntry &schema, unique_ptr<PostgresResultSlice> table_result_p)
-    : PostgresCatalogSet(schema.ParentCatalog()), schema(schema), table_result(std::move(table_result_p)) {
+    : PostgresCatalogSet(schema.ParentCatalog(), !table_result_p), schema(schema), table_result(std::move(table_result_p)) {
 }
 
 string PostgresTableSet::GetInitializeQuery() {
@@ -286,6 +286,7 @@ optional_ptr<CatalogEntry> PostgresTableSet::CreateTable(ClientContext &context,
 void PostgresTableSet::AlterTable(ClientContext &context, RenameTableInfo &info) {
 	auto &transaction = PostgresTransaction::Get(context, catalog);
 	string sql = "ALTER TABLE ";
+	sql += KeywordHelper::WriteOptionallyQuoted(schema.name) + ".";
 	sql += KeywordHelper::WriteOptionallyQuoted(info.name);
 	sql += " RENAME TO ";
 	sql += KeywordHelper::WriteOptionallyQuoted(info.new_table_name);
@@ -295,6 +296,7 @@ void PostgresTableSet::AlterTable(ClientContext &context, RenameTableInfo &info)
 void PostgresTableSet::AlterTable(ClientContext &context, RenameColumnInfo &info) {
 	auto &transaction = PostgresTransaction::Get(context, catalog);
 	string sql = "ALTER TABLE ";
+	sql += KeywordHelper::WriteOptionallyQuoted(schema.name) + ".";
 	sql += KeywordHelper::WriteOptionallyQuoted(info.name);
 	sql += " RENAME COLUMN  ";
 	sql += KeywordHelper::WriteOptionallyQuoted(info.old_name);
@@ -307,6 +309,7 @@ void PostgresTableSet::AlterTable(ClientContext &context, RenameColumnInfo &info
 void PostgresTableSet::AlterTable(ClientContext &context, AddColumnInfo &info) {
 	auto &transaction = PostgresTransaction::Get(context, catalog);
 	string sql = "ALTER TABLE ";
+	sql += KeywordHelper::WriteOptionallyQuoted(schema.name) + ".";
 	sql += KeywordHelper::WriteOptionallyQuoted(info.name);
 	sql += " ADD COLUMN  ";
 	if (info.if_column_not_exists) {
@@ -321,6 +324,7 @@ void PostgresTableSet::AlterTable(ClientContext &context, AddColumnInfo &info) {
 void PostgresTableSet::AlterTable(ClientContext &context, RemoveColumnInfo &info) {
 	auto &transaction = PostgresTransaction::Get(context, catalog);
 	string sql = "ALTER TABLE ";
+	sql += KeywordHelper::WriteOptionallyQuoted(schema.name) + ".";
 	sql += KeywordHelper::WriteOptionallyQuoted(info.name);
 	sql += " DROP COLUMN  ";
 	if (info.if_column_exists) {

src/storage/postgres_type_set.cpp

@@ -16,7 +16,7 @@ struct PGTypeInfo {
 
 PostgresTypeSet::PostgresTypeSet(PostgresSchemaEntry &schema, unique_ptr<PostgresResultSlice> enum_result_p,
                                  unique_ptr<PostgresResultSlice> composite_type_result_p)
-    : PostgresCatalogSet(schema.ParentCatalog()), schema(schema), enum_result(std::move(enum_result_p)),
+    : PostgresCatalogSet(schema.ParentCatalog(), !enum_result_p), schema(schema), enum_result(std::move(enum_result_p)),
       composite_type_result(std::move(composite_type_result_p)) {
 }
 

src/storage/postgres_update.cpp

@@ -48,7 +48,9 @@ string CreateUpdateTable(const string &name, PostgresTableEntry &table, const ve
 
 string GetUpdateSQL(const string &name, PostgresTableEntry &table, const vector<PhysicalIndex> &index) {
 	string result;
-	result = "UPDATE " + KeywordHelper::WriteQuoted(table.name, '"');
+	result = "UPDATE ";
+	result += KeywordHelper::WriteQuoted(table.schema.name, '"') + ".";
+	result += KeywordHelper::WriteQuoted(table.name, '"');
 	result += " SET ";
 	for (idx_t i = 0; i < index.size(); i++) {
 		if (i > 0) {

test/sql/storage/attach_create_index.test

@@ -15,6 +15,9 @@ CREATE OR REPLACE TABLE s.test(i INTEGER);
 statement ok
 INSERT INTO s.test VALUES (1), (2), (3);
 
+statement ok
+DROP INDEX IF EXISTS s.i_index
+
 statement ok
 CREATE INDEX i_index ON s.test(i);
 
@@ -58,7 +61,10 @@ SELECT * FROM s.test WHERE i=2 AND j=20
 2	20
 
 statement ok
-DROP TABLE s.test CASCADE
+DROP INDEX s.i_index
+
+statement ok
+DROP TABLE s.test
 
 # index with a function
 statement ok

test/sql/storage/attach_explicit_schema.test

@@ -0,0 +1,96 @@
+# name: test/sql/storage/attach_explicit_schema.test
+# description: Test various DDL operations in an explicit schema
+# group: [storage]
+
+require postgres_scanner
+
+require-env POSTGRES_TEST_DATABASE_AVAILABLE
+
+statement ok
+ATTACH 'dbname=postgresscanner' AS simple (TYPE POSTGRES)
+
+statement ok
+DROP SCHEMA IF EXISTS simple.new_schema CASCADE
+
+statement ok
+create schema simple.new_schema;
+
+statement ok
+create table simple.new_schema.newtable(i int);
+
+statement ok
+insert into simple.new_schema.newtable values (42)
+
+query I
+SELECT * FROM simple.new_schema.newtable
+----
+42
+
+statement ok
+update simple.new_schema.newtable set i=84
+
+query I
+SELECT * FROM simple.new_schema.newtable
+----
+84
+
+statement ok
+ALTER TABLE simple.new_schema.newtable ADD COLUMN j INTEGER
+
+query II
+SELECT * FROM simple.new_schema.newtable
+----
+84	NULL
+
+statement ok
+ALTER TABLE simple.new_schema.newtable RENAME COLUMN j TO k
+
+query I
+SELECT k FROM simple.new_schema.newtable
+----
+NULL
+
+statement ok
+ALTER TABLE simple.new_schema.newtable DROP COLUMN k
+
+query I
+SELECT * FROM simple.new_schema.newtable
+----
+84
+
+statement ok
+ALTER TABLE simple.new_schema.newtable RENAME TO newtable2
+
+query I
+SELECT * FROM simple.new_schema.newtable2
+----
+84
+
+statement ok
+CREATE INDEX i_index ON simple.new_schema.newtable2(i)
+
+statement error
+CREATE INDEX i_index ON simple.new_schema.newtable2(i)
+----
+already exists
+
+statement ok
+CREATE INDEX IF NOT EXISTS i_index ON simple.new_schema.newtable2(i)
+
+statement ok
+DROP INDEX simple.new_schema.i_index
+
+statement ok
+delete from simple.new_schema.newtable2
+
+statement ok
+create view simple.new_schema.newview as select 42
+
+statement ok
+drop table simple.new_schema.newtable2;
+
+statement ok
+drop view simple.new_schema.newview
+
+statement ok
+drop schema simple.new_schema