Catalog/data split: host-catalog hooks, external indexes, fork-surface reduction (v2026.07.10) - #73
Merged
Conversation
This was referenced Aug 26, 2026
Closed
Open
pashandor789
force-pushed
the
mbkkt/split-catalog-data-3
branch
from
August 29, 2026 21:59
548ed1a to
c1a57b9
Compare
…e reduction The hooks a host catalog needs to keep its own definitions while duckdb runs the machinery: entry lookup and creation seams, external index deserialization, dependency walks that survive schema-less entries and foreign attachments, alter and drop dispatch a host can extend, and the fork-surface reductions that came out of aligning the host with duckdb's own roads.
pashandor789
force-pushed
the
mbkkt/split-catalog-data-3
branch
from
August 29, 2026 22:38
c1a57b9 to
388aaf0
Compare
pashandor789
pushed a commit
that referenced
this pull request
Sep 4, 2026
The catalog/data split (#73) changed several duckdb-core behaviors that CI never exercised, because classify-changes does not select the `core` suite on a bare submodule pin bump. Every fix here is facade-safe (verified against serenedb's own sqllogic): - macro dependencies: wire MacroFunction::UnionDependencies, which the split defined but never called, so a CREATE MACRO under enable_macro_dependencies actually records its body's dependencies again -- restoring the self-cycle refusal and the drop-of-referenced-object refusal (test_recursive_macro, test_macro_default_arg_with_dependencies). serenedb enables this globally and its DDL tests already expect the enforcement. - CREATE OR REPLACE: a definition replace only breaks an *owning* dependent (a sequence its table owns), not a plain one (a view over the replaced function, which rebinds). Refuse just the ownership case in DependencyManager::AlterObject and report it the way a DROP does; SetPermissionsInfo::BreaksDependent stays false so ordinary dependents rebind as before (test_alter_dependency_ownership). - COPY DATABASE: rewrite a copied entry's dependency onto the target database (catalog and oid, not only the outer LogicalDependency), so a copied index resolves against the destination rather than the source (copy_database). - ALTER SCHEMA RENAME: reject cleanly with "Altering schemas is not yet supported" instead of crashing on an internal assertion (rename_schema). Three upstream tests asserted stock-duckdb behavior that serenedb deliberately made PG-compatible -- unknown search_path schemas accepted silently, a single-quoted string kept as one atomic schema name (only unquoted bare names are a list), schema-only search_path display, PG-shaped current_schemas. Update them to the serenedb behavior: test_set_search_path, test_querying_from_detached_catalog, peg_parser/transformer/set_statement.
pashandor789
pushed a commit
that referenced
this pull request
Sep 4, 2026
The catalog/data split (#73) changed several duckdb-core behaviors that CI never exercised, because classify-changes does not select the `core` suite on a bare submodule pin bump. Every fix here is facade-safe (verified against serenedb's own sqllogic): - macro dependencies: wire MacroFunction::UnionDependencies, which the split defined but never called, so a CREATE MACRO under enable_macro_dependencies actually records its body's dependencies again -- restoring the self-cycle refusal and the drop-of-referenced-object refusal (test_recursive_macro, test_macro_default_arg_with_dependencies). The same union has to run when overloads are merged: MacroCatalogEntry::MergedWith starts from the superseded version's info, so recompute its top-level dependencies from the merged overload set, or a CREATE OR REPLACE MACRO keeps the old body's subjects and never retires them. serenedb enables this globally and its DDL tests already expect the enforcement. - CREATE OR REPLACE: a definition replace only breaks an *owning* dependent (a sequence its table owns), not a plain one (a view over the replaced function, which rebinds). Refuse just the ownership case in DependencyManager::AlterObject and report it the way a DROP does; SetPermissionsInfo::BreaksDependent stays false so ordinary dependents rebind as before (test_alter_dependency_ownership). - COPY DATABASE: rewrite a copied entry's dependency onto the target database (catalog and oid, not only the outer LogicalDependency), so a copied index resolves against the destination rather than the source (copy_database). - ALTER SCHEMA RENAME: reject cleanly with "Altering schemas is not yet supported" instead of crashing on an internal assertion (rename_schema). Three upstream tests asserted stock-duckdb behavior that serenedb deliberately made PG-compatible -- unknown search_path schemas accepted silently, a single-quoted string kept as one atomic schema name (only unquoted bare names are a list), schema-only search_path display, PG-shaped current_schemas. Update them to the serenedb behavior: test_set_search_path, test_querying_from_detached_catalog, peg_parser/transformer/set_statement.
pashandor789
pushed a commit
that referenced
this pull request
Sep 4, 2026
The catalog/data split (#73) changed several duckdb-core behaviors that CI never exercised, because classify-changes does not select the `core` suite on a bare submodule pin bump. Every fix here is facade-safe (verified against serenedb's own sqllogic): - macro dependencies: wire MacroFunction::UnionDependencies, which the split defined but never called, so a CREATE MACRO under enable_macro_dependencies actually records its body's dependencies again -- restoring the self-cycle refusal and the drop-of-referenced-object refusal (test_recursive_macro, test_macro_default_arg_with_dependencies). The same union has to run when overloads are merged: MacroCatalogEntry::MergedWith starts from the superseded version's info, so recompute its top-level dependencies from the merged overload set, or a CREATE OR REPLACE MACRO keeps the old body's subjects and never retires them. serenedb enables this globally and its DDL tests already expect the enforcement. - CREATE OR REPLACE: a definition replace only breaks an *owning* dependent (a sequence its table owns), not a plain one (a view over the replaced function, which rebinds). Refuse just the ownership case in DependencyManager::AlterObject and report it the way a DROP does; SetPermissionsInfo::BreaksDependent stays false so ordinary dependents rebind as before (test_alter_dependency_ownership). - COPY DATABASE: rewrite a copied entry's dependency onto the target database (catalog and oid, not only the outer LogicalDependency), so a copied index resolves against the destination rather than the source (copy_database). - ALTER SCHEMA RENAME: the split left RenameSchemaInfo with no store execution path, so it crashed on an internal assertion. Reject it in the base Catalog::Alter with "Altering schemas is not yet supported" (rename_schema). A catalog subclass that implements schema rename overrides Alter and returns before reaching this, so the rejection is store-only and the transformer stays untouched. Three upstream tests asserted stock-duckdb behavior that serenedb deliberately made PG-compatible -- unknown search_path schemas accepted silently, a single-quoted string kept as one atomic schema name (only unquoted bare names are a list), schema-only search_path display, PG-shaped current_schemas. Update them to the serenedb behavior: test_set_search_path, test_querying_from_detached_catalog, peg_parser/transformer/set_statement.
pashandor789
added a commit
that referenced
this pull request
Sep 4, 2026
The catalog/data split (#73) changed several duckdb-core behaviors that CI never exercised, because classify-changes does not select the `core` suite on a bare submodule pin bump. Every fix here is facade-safe (verified against serenedb's own sqllogic): - macro dependencies: wire MacroFunction::UnionDependencies, which the split defined but never called, so a CREATE MACRO under enable_macro_dependencies actually records its body's dependencies again -- restoring the self-cycle refusal and the drop-of-referenced-object refusal (test_recursive_macro, test_macro_default_arg_with_dependencies). The same union has to run when overloads are merged: MacroCatalogEntry::MergedWith starts from the superseded version's info, so recompute its top-level dependencies from the merged overload set, or a CREATE OR REPLACE MACRO keeps the old body's subjects and never retires them. serenedb enables this globally and its DDL tests already expect the enforcement. - CREATE OR REPLACE: a definition replace only breaks an *owning* dependent (a sequence its table owns), not a plain one (a view over the replaced function, which rebinds). Refuse just the ownership case in DependencyManager::AlterObject and report it the way a DROP does; SetPermissionsInfo::BreaksDependent stays false so ordinary dependents rebind as before (test_alter_dependency_ownership). - COPY DATABASE: rewrite a copied entry's dependency onto the target database (catalog and oid, not only the outer LogicalDependency), so a copied index resolves against the destination rather than the source (copy_database). - ALTER SCHEMA RENAME: the split left RenameSchemaInfo with no store execution path, so it crashed on an internal assertion. Reject it in the base Catalog::Alter with "Altering schemas is not yet supported" (rename_schema). A catalog subclass that implements schema rename overrides Alter and returns before reaching this, so the rejection is store-only and the transformer stays untouched. Three upstream tests asserted stock-duckdb behavior that serenedb deliberately made PG-compatible -- unknown search_path schemas accepted silently, a single-quoted string kept as one atomic schema name (only unquoted bare names are a list), schema-only search_path display, PG-shaped current_schemas. Update them to the serenedb behavior: test_set_search_path, test_querying_from_detached_catalog, peg_parser/transformer/set_statement.
mkornaukhov
pushed a commit
that referenced
this pull request
Sep 9, 2026
The catalog/data split (#73) changed several duckdb-core behaviors that CI never exercised, because classify-changes does not select the `core` suite on a bare submodule pin bump. Every fix here is facade-safe (verified against serenedb's own sqllogic): - macro dependencies: wire MacroFunction::UnionDependencies, which the split defined but never called, so a CREATE MACRO under enable_macro_dependencies actually records its body's dependencies again -- restoring the self-cycle refusal and the drop-of-referenced-object refusal (test_recursive_macro, test_macro_default_arg_with_dependencies). The same union has to run when overloads are merged: MacroCatalogEntry::MergedWith starts from the superseded version's info, so recompute its top-level dependencies from the merged overload set, or a CREATE OR REPLACE MACRO keeps the old body's subjects and never retires them. serenedb enables this globally and its DDL tests already expect the enforcement. - CREATE OR REPLACE: a definition replace only breaks an *owning* dependent (a sequence its table owns), not a plain one (a view over the replaced function, which rebinds). Refuse just the ownership case in DependencyManager::AlterObject and report it the way a DROP does; SetPermissionsInfo::BreaksDependent stays false so ordinary dependents rebind as before (test_alter_dependency_ownership). - COPY DATABASE: rewrite a copied entry's dependency onto the target database (catalog and oid, not only the outer LogicalDependency), so a copied index resolves against the destination rather than the source (copy_database). - ALTER SCHEMA RENAME: the split left RenameSchemaInfo with no store execution path, so it crashed on an internal assertion. Reject it in the base Catalog::Alter with "Altering schemas is not yet supported" (rename_schema). A catalog subclass that implements schema rename overrides Alter and returns before reaching this, so the rejection is store-only and the transformer stays untouched. Three upstream tests asserted stock-duckdb behavior that serenedb deliberately made PG-compatible -- unknown search_path schemas accepted silently, a single-quoted string kept as one atomic schema name (only unquoted bare names are a list), schema-only search_path display, PG-shaped current_schemas. Update them to the serenedb behavior: test_set_search_path, test_querying_from_detached_catalog, peg_parser/transformer/set_statement.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebased onto
v2026.07.10(supersedes #70, which targetedv2026.07.07).Thirteen feature commits, replayed onto the current version branch. Nothing was merged in from the version branch — the work sits on top of it.
Hooks added, each replacing a hand-rolled copy in the host
ColumnList::TryGetColumn(Identifier)— the nullable sibling of the throwingGetColumnTableCatalogEntry::IsNotNull(constraints, LogicalIndex)CatalogSet::ClearLocalStorage, called fromDropEntry(deliberately notDropEntryInternal, which is also the rename tombstone road where the uncommitted appends must survive)CatalogEntry::CopyPreservingIdentity, used by the fiveCopy()armsDependencyManager::Attachments+ScanDependentsEverywhere/ScanAllEdgesEverywhere— the cross-attachment dependent walk hoisted out ofCheckDropDependencies, taking a catalog filter and a nullable dependent so a counting consumer can ride it. It also skips closed attachments, which the inlined loop did not.LogicalDependency::automatic— lets an index→relation edge be AUTO per edge rather than per object typeFork surface removed (these files are byte-identical to upstream again:
plan_delete.cpp,plan_insert.cpp)default_types.cpp: the namespace-scopeBUILTIN_TYPESarray restored, matching whatdefault_views.cpp/default_functions.cppalready did correctlySchemaCatalogEntry's pure virtuals restored (the throwing bodies were unreachable —DuckSchemaEntryis the only subclass)EntryToString's per-kind arms dropped; the existing fallback renders byte-identical stringsGetStorageTableEntry/GetStorageCatalogdeleted — zero overrides existed anywhere, and the defaults were upstream's own expressions, now inlined at the call sitesCheckTypeIsSupportedBug fixes worth reviewing on their own
DuckIndexEntry::Rollbackignored itsprev_entryand removed the index unconditionally, butCatalogSet::UndocallsRollbackfor every rolled-back version — so rolling back aCOMMENT ON INDEXdropped the table's ART. Now gated onprev_entry.deleted, matchingDuckTableEntry::Rollback.CommitDropandWriteCreateIndextolerate an index whose table has noDataTable(a view index, a search table).AlterObject'sdisallow_alterderives from the dependent's cascade flags instead of a type-keyed exemption list, which never matched for a catalog addressing edges by a stable id.Generated artifacts land in the stacked regen PR.