-
Notifications
You must be signed in to change notification settings - Fork 124
Ordered compaction and inlining with full catalog integration #642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Ordered compaction and inlining with full catalog integration #642
Conversation
|
The failing test did not reproduce for me locally, so I made a commit with just a comment change to force a CI re-run in the hopes that it was a flaky test. The test is not an area of the code that I think I should have edited, but, there are reasons that we have tests... Failing test was |
|
Hmm, this is a different failing test and does not appear related. Please let me know if you would like for me to investigate, otherwise it seems random... |
|
Thinking ahead to adding the insert capability, is the right spot to add that in ducklake_insert.cpp in |
There are a couple of spurious tests failing that I still have to track down, the ones I have seen here are unrelated to your PR. |
That sounds about right! |
Hi folks!
This is a new PR meant to solve the same use case as #593, but addressing the PR feedback! Thank you for the guidance - it was super helpful. I am still open to any changes you recommend!
This adds a new table to the DuckLake spec:
ducklake_sort_key. This keeps a version history of the sort settings for tables over time. (The prior PR used an option for this, but that has been removed based on the feedback).Also, would you prefer the name
ducklake_sort_infoto align withducklake_partition_infoas the spec table name?There are still a few limitations with this PR:
expression, so the intention was to make the spec itself forwards-compatible with expression-oriented sorting.I believe that I made this fully compatible with the batching code, but I was testing locally on a DuckDB catalog and not on Postgres. Any extra eyes on that side would be great!
If this looks good, I can also do any docs PRs that you recommend - happy to help there.
Thanks folks! CC @philippmd as well as an FYI