Skip to content

Update to hotdata SDK 0.2.0 and follow truncated query results#155

Merged
zfarrell merged 3 commits into
mainfrom
worktree-synthetic-churning-blanket
Jun 16, 2026
Merged

Update to hotdata SDK 0.2.0 and follow truncated query results#155
zfarrell merged 3 commits into
mainfrom
worktree-synthetic-churning-blanket

Conversation

@zfarrell

Copy link
Copy Markdown
Contributor

Bumps the hotdata Rust SDK to 0.2.0 and handles its new truncation contract: a fast-completing but large query returns an inline 200 with truncated = true and only a preview, so the inline branch now follows result_id to the full result via the same Arrow path the async branch uses (falling back to the preview with a warning when no result_id is present).

Comment thread src/query.rs Outdated
return query_response_from_sdk(resp);
}
match resp.result_id.clone().flatten() {
Some(result_id) => fetch_arrow_result(api, &result_id),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: following a truncated inline response to Arrow silently drops the inline response's warning (and execution_time_ms). fetch_arrow_resultarrow_result_to_query_response hardcodes warning: None / execution_time_ms: None, so a query-level warning that the server attached to a truncated-but-persisted result won't reach the user. If that warning is meaningful here, consider carrying resp.warning.flatten() through to the fetched result. (not blocking)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — fixed in 579135b. The Some(result_id) arm now carries resp.warning and execution_time_ms onto the fetched Arrow result, with a test asserting both survive the follow.

claude[bot]
claude Bot previously approved these changes Jun 16, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Clean handling of the new truncation contract. The resolve_inline branching (non-truncated → as-is, truncated+result_id → Arrow follow, truncated+no result_id → preview with warning) is sound, and the tests cover all three paths plus warning preservation. One non-blocking nit left inline.

@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.02913% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/query.rs 99.02% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Prior nit resolved (warning + execution_time_ms now carried through the Arrow follow, with a test asserting both survive). Logic is correct across all three branches, conversions are consistent with the existing inline path, and the fallback warning appends rather than clobbers. Tests are meaningful. LGTM.

@zfarrell zfarrell merged commit 098e45a into main Jun 16, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant