-
Notifications
You must be signed in to change notification settings - Fork 111
feat: initial implementation of route to pipeline stage
#1786
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
Changes from 13 commits
236138b
991fef3
eb4c00f
f651f7f
bb91da5
6424a52
5d1214c
735eed2
3a44015
be1237a
58396cd
9267112
d2368a9
8f3aac8
3aea290
16446e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,9 @@ pub enum DataExpression { | |
|
|
||
| /// Conditional data expression. | ||
| Conditional(ConditionalDataExpression), | ||
|
|
||
| /// Output data expression | ||
| Output(OutputDataExpression), | ||
| } | ||
|
|
||
| impl DataExpression { | ||
|
|
@@ -28,6 +31,7 @@ impl DataExpression { | |
| DataExpression::Summary(s) => s.try_fold(scope), | ||
| DataExpression::Transform(t) => t.try_fold(scope), | ||
| DataExpression::Conditional(c) => c.try_fold(scope), | ||
| DataExpression::Output(o) => o.try_fold(scope), | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -39,6 +43,7 @@ impl Expression for DataExpression { | |
| DataExpression::Summary(s) => s.get_query_location(), | ||
| DataExpression::Transform(t) => t.get_query_location(), | ||
| DataExpression::Conditional(c) => c.get_query_location(), | ||
| DataExpression::Output(o) => o.get_query_location(), | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -48,6 +53,7 @@ impl Expression for DataExpression { | |
| DataExpression::Summary(_) => "DataExpression(Summary)", | ||
| DataExpression::Transform(_) => "DataExpression(Transform)", | ||
| DataExpression::Conditional(_) => "DataExpression(Conditional)", | ||
| DataExpression::Output(_) => "DataExpression(Output)", | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -57,6 +63,7 @@ impl Expression for DataExpression { | |
| DataExpression::Summary(s) => s.fmt_with_indent(f, indent), | ||
| DataExpression::Transform(t) => t.fmt_with_indent(f, indent), | ||
| DataExpression::Conditional(c) => c.fmt_with_indent(f, indent), | ||
| DataExpression::Output(o) => o.fmt_with_indent(f, indent), | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -295,3 +302,89 @@ impl ConditionalDataExpressionBranch { | |
| &self.expressions | ||
| } | ||
| } | ||
|
|
||
| /// Data expression representing an operation that emits data to a sink. | ||
| #[derive(Debug, Clone, PartialEq)] | ||
| pub struct OutputDataExpression { | ||
| query_location: QueryLocation, | ||
| output: OutputExpression, | ||
| } | ||
|
|
||
| impl OutputDataExpression { | ||
| pub fn new(query_location: QueryLocation, output: OutputExpression) -> Self { | ||
| Self { | ||
| query_location, | ||
| output, | ||
| } | ||
| } | ||
|
|
||
| pub fn get_output(&self) -> &OutputExpression { | ||
| &self.output | ||
| } | ||
|
|
||
| pub fn try_fold(&mut self, _scope: &PipelineResolutionScope) -> Result<(), ExpressionError> { | ||
| // No folding currently supported for output expressions. | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| impl Expression for OutputDataExpression { | ||
| fn get_query_location(&self) -> &QueryLocation { | ||
| &self.query_location | ||
| } | ||
|
|
||
| fn get_name(&self) -> &'static str { | ||
| "OutputDataExpression" | ||
| } | ||
|
|
||
| fn fmt_with_indent(&self, f: &mut std::fmt::Formatter<'_>, indent: &str) -> std::fmt::Result { | ||
| writeln!(f, "Output:")?; | ||
| write!(f, "{indent}└── ")?; | ||
| match &self.output { | ||
| OutputExpression::NamedSink(expr) => { | ||
| expr.fmt_with_indent(f, format!("{indent} ").as_str()) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Expression representing an operation that emits data to a sink. | ||
| #[derive(Debug, Clone, PartialEq)] | ||
| pub enum OutputExpression { | ||
| /// Output data to a sink identified by name. | ||
| // Currently this contains a static string because it's the only way we handle identifying | ||
| // where to output the data. In the future we could support dynamic sink identified by a | ||
| // variable, result of a function call, or other some expression, at which point we can change | ||
| // this to contain the more general `StaticExpression`. | ||
| NamedSink(StringScalarExpression), | ||
|
Comment on lines
+354
to
+359
Member
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. @albertlockett I just noticed this. Sorry for the late feedback but here it is...
Member
Author
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. re point 1: imo "sink" is kind of a synonym for to "destination". I don't see how one is any more coupled to the collector than the other, but I'm fine to change it to destination if you think it's a better name. I'm not personally hung up on the naming one way or the other. re point 2: the intention of the output expression was simply that the data would be emitted to some destination, and that the variants of the enum would control how the destination is defined. We could change the design to have the expression variants control how data is routed in the pipeline, but that could also be achieved through other types of data expressions. For example, we could imagine a fork with multiple branches as child expressions, which could achieve the "copy the data and continue on" scenario in your example.
Member
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. Sounds like we need a design discussion 😄
Member
Author
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. Yeah that'd be great! We can discuss during wednesday's SIG meeting |
||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod test { | ||
| use super::*; | ||
| use std::fmt; | ||
|
|
||
| // Helper struct to test fmt_with_indent by implementing Display | ||
| struct DisplayWrapper<'a, T: Expression>(&'a T, &'a str); | ||
|
|
||
| impl<'a, T: Expression> fmt::Display for DisplayWrapper<'a, T> { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| self.0.fmt_with_indent(f, self.1) | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_output_expression_fmt_with_indent() { | ||
| let string_expr = StringScalarExpression::new(QueryLocation::new_fake(), "sink_name"); | ||
| let output_expr = OutputExpression::NamedSink(string_expr.clone()); | ||
| let output_data_expr = OutputDataExpression::new(QueryLocation::new_fake(), output_expr); | ||
| let output = format!("{}", DisplayWrapper(&output_data_expr, "")); | ||
| assert_eq!( | ||
| output, | ||
| format!( | ||
| "Output:\n\ | ||
| └── {string_expr:?}\n" | ||
| ) | ||
| ); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.