Repository navigation
feat(bigquery): support fetching bigtable shard boundaries - #71
matt-codecov wants to merge 1 commit into
Conversation
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| ) | ||
| assert ( | ||
| placeholder in query_dict["query"] | ||
| ), f"sharded query must contain the {placeholder!r} placeholder" |
There was a problem hiding this comment.
Empty placeholder corrupts query
High Severity
If bigtable_shard.placeholder is an empty string, validation still passes because "" in query is always true, but query.replace("", clause) inserts the shard predicate between every character and produces invalid SQL for BigQuery.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ad2234c. Configure here.
| ) | ||
| assert ( | ||
| placeholder in query_dict["query"] | ||
| ), f"sharded query must contain the {placeholder!r} placeholder" |
There was a problem hiding this comment.
Placeholder without shard config
Medium Severity
A query may contain {shard_predicate} while bigtable_shard is omitted; parsing succeeds, but main runs the non-sharded path and sends the literal placeholder to BigQuery instead of injecting row-key predicates.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ad2234c. Configure here.
ad2234c to
1d896f2
Compare
1d896f2 to
16cb0c8
Compare
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
2 issues from previous reviews remain unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 16cb0c8. Configure here.
|
got some pushback on the approach so putting this in draft until we align |
|
not pursuing this approach |
1 participant
Allows users to specify some Bigtable parameters in their BigQuery query configuration. These will allow the BigQuery fetcher to call the
SampleRowKeysAPI to compute shard boundaries to split large queries up. It will then run the sharded query and aggregate results before emitting them to Kafka.I don't love teaching the BigQuery fetcher to look past BigQuery to the underlying storage backend but the alternatives all involve a ton more orchestration. I at least put the Bigtable junk behind an interface so we can pretend that this'll one day be extended for a whole array of BigQuery data sources.
Ref FS-210