Merge pull request duckdb#9932 from taniabogatsch/attach-schema-bug

Fix ATTACH of foreign key
Tmonster · Dec 9, 2023 · e117c34 · e117c34
2 parents d423e96 + ec1f092
commit e117c34
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 24 deletions.
diff --git a/src/catalog/catalog_entry/duck_table_entry.cpp b/src/catalog/catalog_entry/duck_table_entry.cpp
@@ -302,7 +302,7 @@ unique_ptr<CatalogEntry> DuckTableEntry::RenameColumn(ClientContext &context, Re
 		create_info->constraints.push_back(std::move(copy));
 	}
 	auto binder = Binder::CreateBinder(context);
-	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info));
+	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info), schema);
 	return make_uniq<DuckTableEntry>(catalog, schema, *bound_create_info, storage);
 }
 
@@ -331,7 +331,7 @@ unique_ptr<CatalogEntry> DuckTableEntry::AddColumn(ClientContext &context, AddCo
 	create_info->columns.AddColumn(std::move(col));
 
 	auto binder = Binder::CreateBinder(context);
-	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info));
+	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info), schema);
 	auto new_storage =
 	    make_shared<DataTable>(context, *storage, info.new_column, *bound_create_info->bound_defaults.back());
 	return make_uniq<DuckTableEntry>(catalog, schema, *bound_create_info, new_storage);
@@ -463,7 +463,7 @@ unique_ptr<CatalogEntry> DuckTableEntry::RemoveColumn(ClientContext &context, Re
 	UpdateConstraintsOnColumnDrop(removed_index, adjusted_indices, info, *create_info, dropped_column_is_generated);
 
 	auto binder = Binder::CreateBinder(context);
-	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info));
+	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info), schema);
 	if (columns.GetColumn(LogicalIndex(removed_index)).Generated()) {
 		return make_uniq<DuckTableEntry>(catalog, schema, *bound_create_info, storage);
 	}
@@ -498,7 +498,7 @@ unique_ptr<CatalogEntry> DuckTableEntry::SetDefault(ClientContext &context, SetD
 	}
 
 	auto binder = Binder::CreateBinder(context);
-	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info));
+	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info), schema);
 	return make_uniq<DuckTableEntry>(catalog, schema, *bound_create_info, storage);
 }
 
@@ -526,7 +526,7 @@ unique_ptr<CatalogEntry> DuckTableEntry::SetNotNull(ClientContext &context, SetN
 		create_info->constraints.push_back(make_uniq<NotNullConstraint>(not_null_idx));
 	}
 	auto binder = Binder::CreateBinder(context);
-	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info));
+	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info), schema);
 
 	// Early return
 	if (has_not_null) {
@@ -557,7 +557,7 @@ unique_ptr<CatalogEntry> DuckTableEntry::DropNotNull(ClientContext &context, Dro
 	}
 
 	auto binder = Binder::CreateBinder(context);
-	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info));
+	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info), schema);
 	return make_uniq<DuckTableEntry>(catalog, schema, *bound_create_info, storage);
 }
 
@@ -633,7 +633,7 @@ unique_ptr<CatalogEntry> DuckTableEntry::ChangeColumnType(ClientContext &context
 	AlterBinder expr_binder(*binder, context, *this, bound_columns, info.target_type);
 	auto expression = info.expression->Copy();
 	auto bound_expression = expr_binder.Bind(expression);
-	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info));
+	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info), schema);
 	vector<column_t> storage_oids;
 	for (idx_t i = 0; i < bound_columns.size(); i++) {
 		storage_oids.push_back(columns.LogicalToPhysical(bound_columns[i]).index);
@@ -668,7 +668,7 @@ unique_ptr<CatalogEntry> DuckTableEntry::AddForeignKeyConstraint(ClientContext &
 	    make_uniq<ForeignKeyConstraint>(info.pk_columns, info.fk_columns, std::move(fk_info)));
 
 	auto binder = Binder::CreateBinder(context);
-	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info));
+	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info), schema);
 
 	return make_uniq<DuckTableEntry>(catalog, schema, *bound_create_info, storage);
 }
@@ -691,7 +691,7 @@ unique_ptr<CatalogEntry> DuckTableEntry::DropForeignKeyConstraint(ClientContext
 	}
 
 	auto binder = Binder::CreateBinder(context);
-	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info));
+	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info), schema);
 
 	return make_uniq<DuckTableEntry>(catalog, schema, *bound_create_info, storage);
 }
@@ -706,7 +706,7 @@ unique_ptr<CatalogEntry> DuckTableEntry::Copy(ClientContext &context) const {
 	}
 
 	auto binder = Binder::CreateBinder(context);
-	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info));
+	auto bound_create_info = binder->BindCreateTableInfo(std::move(create_info), schema);
 	return make_uniq<DuckTableEntry>(catalog, schema, *bound_create_info, storage);
 }
 

diff --git a/src/include/duckdb/planner/binder.hpp b/src/include/duckdb/planner/binder.hpp
@@ -112,8 +112,6 @@ class Binder : public std::enable_shared_from_this<Binder> {
 	unique_ptr<BoundCreateTableInfo> BindCreateTableInfo(unique_ptr<CreateInfo> info);
 	unique_ptr<BoundCreateTableInfo> BindCreateTableInfo(unique_ptr<CreateInfo> info, SchemaCatalogEntry &schema);
 
-	vector<unique_ptr<Expression>> BindCreateIndexExpressions(TableCatalogEntry &table, CreateIndexInfo &info);
-
 	void BindCreateViewInfo(CreateViewInfo &base);
 	SchemaCatalogEntry &BindSchema(CreateInfo &info);
 	SchemaCatalogEntry &BindCreateFunctionInfo(CreateInfo &info);

diff --git a/src/parser/transform/statement/transform_alter_table.cpp b/src/parser/transform/statement/transform_alter_table.cpp
@@ -95,7 +95,7 @@ unique_ptr<AlterStatement> Transformer::TransformAlter(duckdb_libpgquery::PGAlte
 		}
 		case duckdb_libpgquery::PG_AT_DropConstraint:
 		default:
-			throw NotImplementedException("ALTER TABLE option not supported yet!");
+			throw NotImplementedException("No support for that ALTER TABLE option yet!");
 		}
 	}
 

diff --git a/src/planner/binder/statement/bind_create_table.cpp b/src/planner/binder/statement/bind_create_table.cpp
@@ -305,15 +305,4 @@ unique_ptr<BoundCreateTableInfo> Binder::BindCreateTableInfo(unique_ptr<CreateIn
 	return BindCreateTableInfo(std::move(info), schema);
 }
 
-vector<unique_ptr<Expression>> Binder::BindCreateIndexExpressions(TableCatalogEntry &table, CreateIndexInfo &info) {
-	auto index_binder = IndexBinder(*this, this->context, &table, &info);
-	vector<unique_ptr<Expression>> expressions;
-	expressions.reserve(info.expressions.size());
-	for (auto &expr : info.expressions) {
-		expressions.push_back(index_binder.Bind(expr));
-	}
-
-	return expressions;
-}
-
 } // namespace duckdb
diff --git a/test/sql/attach/attach_dependencies.test b/test/sql/attach/attach_dependencies.test
@@ -0,0 +1,54 @@
+# name: test/sql/attach/attach_dependencies.test
+# description: Test that we can ATTACH databases with dependencies in the schema
+# group: [attach]
+
+# foreign key
+
+load __TEST_DIR__/fk.db
+
+statement ok
+CREATE TABLE pk_tbl (id INTEGER PRIMARY KEY, name VARCHAR UNIQUE);
+
+statement ok
+CREATE TABLE fk_tbl (id INTEGER REFERENCES pk_tbl(id));
+
+# alter columns
+
+load __TEST_DIR__/alter_column.db
+
+statement ok
+CREATE TABLE tbl_alter_column (id INT, other INT, nn_col INT NOT NULL, rm INT, rename_c INT, my_def INT, drop_def INT DEFAULT 10, new_null_col INT);
+
+statement ok
+ALTER TABLE tbl_alter_column ADD COLUMN k INTEGER;
+
+statement ok
+ALTER TABLE tbl_alter_column ALTER other SET DATA TYPE VARCHAR USING concat(other, '_', 'yay');
+
+statement ok
+ALTER TABLE tbl_alter_column ALTER COLUMN nn_col DROP NOT NULL;
+
+statement ok
+ALTER TABLE tbl_alter_column DROP rm;
+
+statement ok
+ALTER TABLE tbl_alter_column RENAME rename_c TO my_new_col;
+
+statement ok
+ALTER TABLE tbl_alter_column ALTER COLUMN my_def SET DEFAULT 10;
+
+statement ok
+ALTER TABLE tbl_alter_column ALTER COLUMN drop_def DROP DEFAULT;
+
+statement ok
+ALTER TABLE tbl_alter_column ALTER COLUMN new_null_col SET NOT NULL;
+
+# now attach all databases
+
+load __TEST_DIR__/other.db
+
+statement ok
+ATTACH '__TEST_DIR__/fk.db';
+
+statement ok
+ATTACH '__TEST_DIR__/alter_column.db';