Handle presigned links to data blobs#3
Closed
elffjs wants to merge 1 commit into
Closed
Conversation
These are distinguished by a new data_index_key column in ClickHouse. The objects live in a separate bucket, and contain only the data portion of the event. They are served with the appropriate content type.
zer0stars
added a commit
that referenced
this pull request
Jun 20, 2026
#3 events: dedup the decoded-event read path. lakeEventsDeduped collapses at-rest duplicates (QUALIFY ROW_NUMBER over subject,timestamp,name,source — ClickHouse's event ReplacingMergeTree key) so two cloud_event_ids that decode to the same logical event count once, not twice. The materializer's INSERT anti-join keeps them (its key includes cloud_event_id) and the signal read path already deduped; the event read path did not, over-counting events vs CH. #4 fetch: drop the 400-day lookback floor for subject-scoped fetches. It is a DoS guard for subject-less, id-less scans only — ClickHouse imposes no floor when given no `after`, and a subject prunes to one vehicle's files via the (subject, time) sort, so latestCloudEvent / cloudEvents now reach arbitrarily old events. A dormant vehicle whose newest event predates the window no longer wrongly looks empty. The guard stays for genuinely unbounded searches. #8 materializer: hard-error when MATERIALIZER_SHARD_COUNT>1 on the DuckLake path. Sharding is not honored there (the global ingest_progress cursor allows exactly one logical processor); refuse the config rather than silently run every replica over the full stream and roll back the CAS losers. #10 aggregations: implement inPolygon / inCircle location filters in pure SQL (haversine great-circle distance; an even-odd ray cast unrolled over the request's vertices) with no spatial-extension dependency, replacing the "not supported on duckdb backend" rejection. Mirrors ClickHouse geoDistance / pointInPolygon.
zer0stars
added a commit
that referenced
this pull request
Jun 23, 2026
Code-grounded, prioritized analysis of where the DuckLake query layer can exceed the CH-parity implementation (CH limits: pre-materialized MVs, fetch-all-to-Go detector loops, approximate functions). Covers: #0 ignition ongoing-trips (shipped), #1 exact aggregations (shipped+pinned), #2 ASOF trip enrichment (location gap-fill schema-free; distance/avg-speed via the existing Signals channel under no-schema), #3 SQL-native idling/refuel/recharge, #4 spatial (needs LOAD spatial first), #5-7 secondary. Verified by the round-3 E2E reviews (spatial-not-loaded + the schema caveats corrected).
zer0stars
added a commit
that referenced
this pull request
Jun 23, 2026
…ent #3) The idling detector streamed every RPM sample to Go and ran findIdleRpmRanges in-memory (a CH-era necessity — CH can't express run-length on read). The lake backend now computes idle runs in SQL via gaps-and-islands (window functions), returning only the runs instead of every sample: far less data over the wire, exact boundaries, one round-trip. Added the optional segments.IdleRunSource interface (LakeSignalSource.IdleRuns); the idling detector uses it when available and falls back to LevelSamples + the Go scan for ClickHouse — output is identical (the detector applies the SAME clip + minDuration). A run breaks on a non-idle reading or a gap > maxGap, matching the Go logic. Tests: IdleRuns (non-idle break + gap split), plus the existing lake idling parity test stays green.
zer0stars
added a commit
that referenced
this pull request
Jun 23, 2026
Member
|
Closing as superseded — the functionality is already on `main`, reimplemented natively in the DuckLake layer by the `BLOB_BUCKET` refactor (commit b969700).
This PR targets the old ClickHouse `eventrepo` (its go.mod re-adds `clickhouse-go`/`sqlboiler`), which `main` removed — rebasing would resurrect that stack. Functionality verified by #8: `TestLakeEventService_PresignBlobURL` (signing + nil/empty/error cases) and existing `TestLakeEventService_BlobIndexKey` (blob-key routing into the presign branch). Follow-ups noted on the main implementation (from review, optional): `dataUrl` is a bare string so it serializes as `""` rather than `null` on inline events; and `PresignBlobURL` could assert the `BlobKeyPrefix` guard itself (today only the call sites check). |
zer0stars
added a commit
that referenced
this pull request
Jun 25, 2026
Lock in the two behaviors ported from the legacy ClickHouse eventrepo (superseded PRs #3, #4), now served natively from the DuckLake layer: - PresignBlobURL: a blob key becomes a presigned S3 GET URL for the configured blob bucket; nil presigner, empty bucket, and presigner failure all error loudly rather than panic or silently inline. Complements TestLakeEventService_BlobIndexKey (blob-key routing into the resolver's HasPrefix(key, BlobKeyPrefix) presign branch). - Voiding under reverse ingest order: a tombstone whose timestamp precedes the event it voids still hides that event (the anti-join keys on voids_id == id, not on time). Complements TestLakeEventService_VoidingExcludes (tombstone-after).
zer0stars
added a commit
that referenced
this pull request
Jun 25, 2026
…b keys Two issues surfaced reviewing the superseded presign PR (#3), in main's DuckLake-native implementation: - CloudEventWrapper.DataURL was a bare string, so the nullable `dataUrl` GraphQL field serialized as "" (not null) for inline events. MCP tools auto-select dataUrl, so every inline CloudEvent returned `"dataUrl": ""`. Make it *string (nil for inline) and regenerate gqlgen (the field now uses the pointer/nullable marshaller). - LakeEventService.PresignBlobURL now rejects any key not under eventrepo.BlobKeyPrefix. The method is on the exported EventService interface; the prefix check previously lived only at the call sites, so a future caller could have presigned arbitrary objects in the bucket (e.g. raw parquet) for an authenticated user. Extends TestLakeEventService_PresignBlobURL with the non-blob-key rejection case (the presigner must not be invoked).
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
2 participants
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.
These are distinguished by a new data_index_key column in ClickHouse. The objects live in a separate bucket, and contain only the data portion of the event. They are served with the appropriate content type.
Port of DIMO-Network/fetch-api#79.