chore: bump erpl-bics and erpl-odp for the tuning API and test hardening - #131
Merged
Conversation
erpl-odp (#8): - Guards a null local state that crashed sap_odp_read_full at threads := 8, when DuckDB opens more workers than there are packages. - Gives erpl_odp its first settings: erpl_odp_fetch_size and erpl_odp_max_threads, matching erpl_rfc's vocabulary. I_MAXPACKAGESIZE was compiled in at 2 MiB and unreachable, so nobody could trade round-trips against per-packet memory. - Replaces 17 assertions of the form `count(*) >= 0 ---- true` -- true of any result, including an empty one. Three delta tests passed end-to-end if read_delta always returned nothing. erpl-bics (#11): - sap_bics_begin's rows / columns / filters were declared and never read; they now place characteristics at open time, which also removes three full session round-trips from building a query. - Models the fixed cost of a fetch, which was assumed to be zero, and exposes both cost terms as settings. Budget defaults deliberately unchanged. - The overflow message now states what a refused query will really cost: memory scales with result ROWS, not data cells, so the cell-derived figure it used to quote understated a 46,755-row result by 19x. Suites green at these commits: bics 45/45, odp 24/24 on both backends.
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
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.
Pointer bump only — the submodules have no CI of their own, so this is where their
changes get built across the matrix.
erpl-odp (DataZooDE/erpl-odp#8)
OdpSessionLocalStatereturnsnullptrwhen the queue drained beforea worker started — which DuckDB does whenever it opens more local states than there
are packages — and
OdpSessionScancast it unconditionally. Reproducible withthreads := 8.erpl_odp_fetch_sizeanderpl_odp_max_threads, matchingerpl_rfc's vocabulary.I_MAXPACKAGESIZEwascompiled in at 2 MiB and unreachable.
if
read_deltaalways returned nothing.Measured while doing it: at the default 2 MiB the ODP test source yields 3 packages,
so the threads sweep was mostly exercising the drained-queue path, not concurrent
hand-off. At 16 KiB it yields 195, and 20 runs at
threads=16came back identical.erpl-bics (DataZooDE/erpl-bics#11)
sap_bics_begin'srows/columns/filterswere declared and never read. Theynow place characteristics at open time, which also removes three full session
round-trips from building a query.
cost terms as settings. Budget defaults deliberately unchanged; correcting them
would cut permitted sizes ~10x, which is a product decision, not a refactor.
with result rows, not data cells, so the cell-derived figure understated a
46,755-row result by 19x (34 MB quoted, 666 MB actual). The new estimate predicts
657 MB — 1.3% out.
Verification at these commits
BICS 45/45, ODP 24/24 on both backends,
[odp_xml]31 assertions (was 17).Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.