-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/UUID timestamp extraction #1070
Changes from all commits
a73218b
d1cd92c
bda349b
93f8855
b079a73
5dba0b6
c498499
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
use std::iter; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: avoid extra space here. Have a single block of |
||
use super::{dbutil, do_validate, parse_validate, Config, Lambda, Param, Transform}; | ||
use anyhow::Context; | ||
use futures::channel::mpsc; | ||
|
@@ -213,7 +215,12 @@ fn parse_open( | |
} = transform; | ||
|
||
let source = source.unwrap(); | ||
let params: Vec<_> = source.projections.iter().map(Param::new).collect(); | ||
let params: Vec<_> = source | ||
.projections | ||
.iter() | ||
.zip(iter::repeat(&source)) | ||
.map(|(p, c)| Param::new(p, c)) | ||
.collect(); | ||
|
||
let block: String = serde_json::from_str(&lambda_config_json).with_context(|| { | ||
format!("failed to parse SQLite lambda block: {lambda_config_json}") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -141,13 +141,16 @@ fn bind_parameter<N: doc::AsNode>( | |
is_content_encoding_base64, | ||
is_format_integer, | ||
is_format_number, | ||
collection, | ||
.. | ||
}: &Param, | ||
document: &N, | ||
) -> rusqlite::Result<()> { | ||
use derive::PointerExt; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i don't love the introduced dependence on |
||
use doc::Node; | ||
let uuid_ptr_parsed = doc::Pointer::maybe_parse(collection.uuid_ptr.as_str()); | ||
|
||
match ptr.query(document).map(doc::AsNode::as_node) { | ||
ptr.query_and_resolve_virtuals(uuid_ptr_parsed, document, |doc| match doc { | ||
None | Some(Node::Null) => return stmt.raw_bind_parameter(index + 1, None::<bool>), | ||
Some(Node::Bool(b)) => return stmt.raw_bind_parameter(index + 1, b), | ||
|
||
|
@@ -179,7 +182,7 @@ fn bind_parameter<N: doc::AsNode>( | |
Some(n @ Node::Object(_)) => { | ||
stmt.raw_bind_parameter(index + 1, &serde_json::to_string(&n).unwrap()) | ||
} | ||
} | ||
}) | ||
} | ||
|
||
fn row_to_json( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,8 @@ use proto_flow::flow; | |
pub struct Param { | ||
// Projection which is being related as a SQLite parameter. | ||
pub projection: flow::Projection, | ||
// Collection which contains the projection that is being related as a SQLite parameter. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most of the reason for I'm thinking we need to introduce a That allows resolution of all this stuff from only the For this crate specifically,
block, you also test for |
||
pub collection: flow::CollectionSpec, | ||
// Canonical SQLite parameter encoding for this field. | ||
pub canonical_encoding: String, | ||
// JSON pointer location of this field within documents. | ||
|
@@ -20,9 +22,10 @@ pub struct Param { | |
} | ||
|
||
impl Param { | ||
pub fn new(p: &flow::Projection) -> Self { | ||
pub fn new(p: &flow::Projection, c: &flow::CollectionSpec) -> Self { | ||
Self { | ||
projection: p.clone(), | ||
collection: c.clone(), | ||
canonical_encoding: canonical_param_encoding(&p.field), | ||
ptr: doc::Pointer::from_str(&p.ptr), | ||
is_format_integer: matches!(&p.inference, Some(flow::Inference{string: Some(str), ..}) if str.format == "integer"), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not thrilled with adding new dependency on
derive
. That crate is slatted to be taken out back to the woodshed...