Support synthetic keys in the (relations ...) CSV loading form#269
Support synthetic keys in the (relations ...) CSV loading form#269hbarthels wants to merge 3 commits into
Conversation
Add a `(keys :synthetic_key)` form as an alternative to the explicit `(keys (column ...) ...)` clause. This tells the loader to synthesize the shared key instead of drawing it from CSV columns. It works with both plain and CDC loading, and any marker other than `:synthetic_key` is a hard error. Source-of-truth changes (all SDK code is regenerated from these): - proto: add `bool synthetic_key = 4` to `TargetRelations`, mutually exclusive with `keys` (additive field, so `buf breaking` passes). - grammar: add the `(keys :synthetic_key)` alternative to `relation_keys`, carry the flag through `construct_relations`, and reject unknown markers. Regenerated the Python, Go, and Julia protobuf bindings, parsers, and pretty printers, and added round-trip fixtures plus parser unit tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| | "(" "keys" ":" SYMBOL ")" | ||
| construct: $$ = construct_synthetic_keys($4) | ||
| deconstruct if $$[1]: | ||
| $4: String = "synthetic_key" |
There was a problem hiding this comment.
@hbarthels why does the name of the key relation be hardcoded? Would (keys :bogus) not be just as unambiguous?
| (paths "s3://bucket/nodes.csv")) | ||
| (csv_config {}) | ||
| (relations | ||
| (keys :synthetic_key) |
There was a problem hiding this comment.
Maybe where we're not yet on the same page about is how the unary relation of synthetic keys will be exposed to the rest of the program.
If its (keys <target_id>) then we should allow any target id, not just :synthetic_keys.
But I guess currently its
(keys :synthetic_key)
(relation :just_the_keys)
Is that right? If so, we need to exercise that in the tests as well. And then (keys :synthetic_key) should just be (keys synthetic) or have a separate (synthetic-key) marker.
In the S-expression syntax, the : is just indicates a human readable label for an internal id.
There was a problem hiding this comment.
Yeah, right, I thought that we wanted to do
(keys :synthetic_key)
(relation :just_the_keys)
(relation :some_value (column ...))
where :synthetic_key is a constant and :just_the_keys is the name of the output relation. I will change :synthetic_key to something without :.
There was a problem hiding this comment.
I changed it to (keys synthetic) and added another test.
Address review feedback: `:` is the ID sigil, and the synthetic-key marker is a constant, not an ID, so it shouldn't use `:`. Switch the keys clause from `(keys :synthetic_key)` to `(keys synthetic)`, where `synthetic` is a soft keyword. Unknown markers remain a hard parse error. The serialized proto is unchanged (the marker only affects surface syntax), so the binary snapshots and generated protobuf bindings are untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cover the case where a synthetic key feeds a single relation that has no value columns — i.e. the relation holds just the (synthetic) key. Adds the `relations_synthetic_key_unary` round-trip fixture and a matching parser unit test asserting synthetic_key is set, keys is empty, and the sole target has no value columns. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Summary
Adds a
(keys synthetic)form to the generalized(relations …)CSV loading construct, as an alternative to the explicit(keys (column …) …)clause:This signals that the loader should synthesize the shared key instead of drawing it from CSV columns. It works with both plain and CDC (
inserts/deletes) loading, and any marker other thansynthetic(e.g.(keys bogus)) is a hard parse error.Changes
Source-of-truth edits (everything else is regenerated):
TargetRelations): addedbool synthetic_key = 4, documented as mutually exclusive withkeys— matching the existingcolumns-vs-relationsconvention inCSVData. Purely additive, sobuf breakingpasses.meta/src/meta/grammar.y): gaverelation_keysa second alternative(keys synthetic)and threaded the flag throughconstruct_relations. Only thesynthetickeyword is accepted; any other token fails to parse.Regenerated via
make: the Python, Go, and Julia protobuf bindings, parsers, and pretty-printers.Test fixtures & unit tests:
tests/lqp/relations_synthetic_key.lqpandrelations_synthetic_key_cdc.lqp(+.bin,pretty/,pretty_debug/snapshots).Design note
The marker is encoded as a
boolalongside the existingrepeated keysfield (mutual exclusion documented in comments) rather than a protooneof, becauseoneofcan't hold arepeatedfield and reworkingkeysinto a wrapper message would be a breaking wire change. A stricteroneofencoding could be scheduled separately as a deliberate breaking change.Verification
🤖 Generated with Claude Code