Skip to content

Conversation

Tishj
Copy link
Contributor

@Tishj Tishj commented Oct 2, 2024

This PR fixes https://github.com/duckdblabs/duckdb-internal/issues/3050
This PR is a revival of #10677

We use GetNaiveExportOrder as we already do, but now we use DependencyManager::ReorderEntries to make sure a dependent entry gets exported first if the dependency manager is aware that there is a dependency on the entry.

Tishj added 30 commits August 17, 2023 16:24
…inders CatalogEntryRetriever, add tests with created types and dependencies created by them
… dependencies of the generated columns are registered as dependencies of the table
…, don't make any cross-catalog dependencies, detect and properly error when CREATE OR REPLACE statements depend on itself
@duckdb-draftbot duckdb-draftbot marked this pull request as draft October 4, 2024 12:21
@Tishj Tishj marked this pull request as ready for review October 4, 2024 12:24
@Mytherin Mytherin changed the base branch from main to feature October 7, 2024 07:07
@Mytherin Mytherin changed the base branch from feature to main October 7, 2024 07:08
Copy link
Collaborator

@Mytherin Mytherin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Looks great - can you just have a look at the failing CI run?

…es, they need to be synced | only with serialization version >= 1.1.0 do we serialize dependencies, to make this test pass we need to enable serializing of the dependencies in order to properly reorder the exported entries
@duckdb-draftbot duckdb-draftbot marked this pull request as draft October 7, 2024 10:34
@Tishj Tishj marked this pull request as ready for review October 7, 2024 10:45
@Mytherin Mytherin changed the base branch from main to feature October 8, 2024 12:04
@duckdb-draftbot duckdb-draftbot marked this pull request as draft October 8, 2024 12:06
@Mytherin Mytherin marked this pull request as ready for review October 8, 2024 12:06
@Mytherin
Copy link
Collaborator

Mytherin commented Oct 8, 2024

Retagging this to feature after all as I think the new dependencies have some risk of introducing new bugs/differences in behavior. Otherwise this is good to go if the CI passes.

@Mytherin Mytherin merged commit 37f8ce3 into duckdb:feature Oct 11, 2024
42 of 44 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants