You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
spiceai/spiceai#14783 applies the inverse of a dataset's retention policy to the on_zero_results: use_source fallback query, so a row that retention_sql or retention_period evicted from the acceleration is no longer read back from the source. When the inverse cannot be planned against the source schema, the dataset now fails to load. Before this change, the docs listed retention policies as a reason to enable use_source without saying that the fallback would return soft-deleted and expired rows, and the "Accelerating application logs" example implied every log older than the refresh window falls back to the source.
Pages changed
website/docs/features/data-acceleration/data-refresh.md: new "Fallback with a Retention Policy" subsection under Behavior on Zero Results (three-valued inverse, NULLtime_column kept, applies even with retention_check_enabled: false, load-time refusal and its error text, fix); a pointer sentence in the zero-results intro; item 5 of the application-logs example now notes the 7-day retention cutoff.
website/docs/reference/spicepod/datasets.md: acceleration.on_zero_results notes the retention inverse and the load failure, linking to the new subsection.
Versioned docs: vNext only. git tag --contains d2922159ca returns no tags, so no released version has this behavior and website/versioned_docs/ is unchanged.
Verification
Review round 2 (both items confirmed on trunk, code inspection only): with a scheduled retention check on a non-Cayenne engine, the fallback inverse uses the partition-AND cutoff that includes time_partition_column; Cayenne, or no scheduled check, uses time_column alone — let apply_time_column_keep = !declared_retention_runs || acceleration_settings.engine == Engine::Cayenne; at crates/runtime/src/datafusion/mod.rs:4177. In caching mode no fallback keep is built — && refresh_mode != RefreshMode::Caching at crates/runtime-table/src/accelerated/mod.rs:1318.
Reproduced: no release contains the change — git merge-base --is-ancestor <merge of #14783> v2.4.0-rc.1 returns false, and v2.4.0-rc.1 is the newest tag (https://github.com/spiceai/spiceai/tags), so the page names "Spice v2.4.0-rc.1 and earlier releases".
Reproduced: load-time error text: "Failed to register dataset '{dataset_name}' ({connector}): \on_zero_results: use_source` cannot be combined with this retention configuration, so the dataset was not loaded. ..."` (crates/runtime/src/datafusion/mod.rs:289)
Reproduced: suggested fix text: "Remove \on_zero_results: use_source`, or use a `retention_sql` / `retention_period` predicate that can be evaluated as a filter on the source columns."` (crates/runtime/src/datafusion/mod.rs:289)
Reproduced: unsupported time-column cause: "time-based retention compares \{time_column}` against the cutoff, but that column cannot be used as a source filter (unsupported type)"` (crates/runtime-table/src/accelerated/fallback_retention.rs:39)
Reproduced: inverse is skipped in caching mode: ) && !matches!(refresh_mode, RefreshMode::Caching) (crates/runtime/src/datafusion/mod.rs:4166)
Reproduced: time inverse applied when no scheduled worker runs (or on Cayenne): !declared_retention_runs || acceleration_settings.engine == Engine::Cayenne; (crates/runtime/src/datafusion/mod.rs:4178)
Unverified, code inspection only: the retention_sql inverse applies when retention_check_enabled is false (from_configured takes retention_delete_expr, which is parsed whenever retention_sql is set).
Unverified, code inspection only: the application-logs example returns no row for a log older than 7 days.
cd website && npm run build at 01936ca: [SUCCESS] Generated static files in "build". exit=0 (no broken links or anchors)
The retention_period description omits time_partition_column. When a scheduled retention worker runs on a non-Cayenne engine, the delete predicate uses both time columns, so its inverse can keep a row with an expired time_column when the partition value is recent or NULL. Cayenne and unscheduled policies use only time_column; document this distinction so readers can predict fallback results.
Scope on_zero_results behavior to supported use_source modes
on_zero_results is ignored in refresh_mode: caching, and the implementation skips both retention-inverse validation and fallback filtering there. This sentence currently states the behavior for any dataset with retention; scope it to use_source in the supported modes so caching users do not expect origin reads to honor this filter.
Addressed both items from the latest review in c660c47: the retention_period paragraph now covers time_partition_column (partition-AND inverse on non-Cayenne engines with a scheduled check; time_column alone on Cayenne or without a scheduled check, per crates/runtime/src/datafusion/mod.rs:4177), and the datasets.md note is scoped to on_zero_results: use_source and says caching mode ignores it (crates/runtime-table/src/accelerated/mod.rs:1316-1318).
This still implies that every log older than the one-day refresh window falls back. In append mode, refresh_data_window limits what each refresh reads but does not evict rows already accelerated; a log loaded while recent can remain local until the seven-day retention cutoff. Qualify the fallback as applying only when the requested log is absent from the acceleration.
Addressed the previously-missed item in 8bee069: item 5 now falls back only for a log that is not in the acceleration, and notes that a log loaded within the last day stays until the retention check removes it.
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
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.
spiceai/spiceai#14783 applies the inverse of a dataset's retention policy to the
on_zero_results: use_sourcefallback query, so a row thatretention_sqlorretention_periodevicted from the acceleration is no longer read back from the source. When the inverse cannot be planned against the source schema, the dataset now fails to load. Before this change, the docs listed retention policies as a reason to enableuse_sourcewithout saying that the fallback would return soft-deleted and expired rows, and the "Accelerating application logs" example implied every log older than the refresh window falls back to the source.Pages changed
website/docs/features/data-acceleration/data-refresh.md: new "Fallback with a Retention Policy" subsection under Behavior on Zero Results (three-valued inverse,NULLtime_columnkept, applies even withretention_check_enabled: false, load-time refusal and its error text, fix); a pointer sentence in the zero-results intro; item 5 of the application-logs example now notes the 7-day retention cutoff.website/docs/reference/spicepod/datasets.md:acceleration.on_zero_resultsnotes the retention inverse and the load failure, linking to the new subsection.Versioned docs: vNext only.
git tag --contains d2922159careturns no tags, so no released version has this behavior andwebsite/versioned_docs/is unchanged.Verification
Review round 2 (both items confirmed on trunk, code inspection only): with a scheduled retention check on a non-Cayenne engine, the fallback inverse uses the partition-AND cutoff that includes
time_partition_column; Cayenne, or no scheduled check, usestime_columnalone —let apply_time_column_keep = !declared_retention_runs || acceleration_settings.engine == Engine::Cayenne;at crates/runtime/src/datafusion/mod.rs:4177. In caching mode no fallback keep is built —&& refresh_mode != RefreshMode::Cachingat crates/runtime-table/src/accelerated/mod.rs:1318.Reproduced: no release contains the change —
git merge-base --is-ancestor <merge of #14783> v2.4.0-rc.1returns false, andv2.4.0-rc.1is the newest tag (https://github.com/spiceai/spiceai/tags), so the page names "Spice v2.4.0-rc.1 and earlier releases".Trunk SHA:
f23a6aa686ddff43ae5a73effa36087a3b8425f9."Failed to register dataset '{dataset_name}' ({connector}): \on_zero_results: use_source` cannot be combined with this retention configuration, so the dataset was not loaded. ..."` (crates/runtime/src/datafusion/mod.rs:289)"Remove \on_zero_results: use_source`, or use a `retention_sql` / `retention_period` predicate that can be evaluated as a filter on the source columns."` (crates/runtime/src/datafusion/mod.rs:289)"time-based retention compares \{time_column}` against the cutoff, but that column cannot be used as a source filter (unsupported type)"` (crates/runtime-table/src/accelerated/fallback_retention.rs:39)) && !matches!(refresh_mode, RefreshMode::Caching)(crates/runtime/src/datafusion/mod.rs:4166)!declared_retention_runs || acceleration_settings.engine == Engine::Cayenne;(crates/runtime/src/datafusion/mod.rs:4178)retention_sql, worker off), and Cayenne (worker on and off), and a never-loaded recent row still falls back: source PR integration tests incrates/runtime/tests/acceleration/on_zero_results_retention.rs, fix(acceleration): keep retention-evicted rows out of the on_zero_results source fallback spiceai#14783keep_exprthree-valued logic andNULLtimestamps kept: source PR unit tests, fix(acceleration): keep retention-evicted rows out of the on_zero_results source fallback spiceai#14783retention_sqlinverse applies whenretention_check_enabledisfalse(from_configuredtakesretention_delete_expr, which is parsed wheneverretention_sqlis set).cd website && npm run buildat 01936ca:[SUCCESS] Generated static files in "build". exit=0(no broken links or anchors)