Skip to content
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

Minor cleanup and formatting fixes #445

Merged
merged 1 commit into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ homepage = "https://github.com/partiql/partiql-lang-rust"
repository = "https://github.com/partiql/partiql-lang-rust"
version = "0.6.0"
edition = "2021"
resolver = 2

[workspace]

Expand Down
19 changes: 7 additions & 12 deletions extension/partiql-extension-visualize/src/ast_to_dot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl<'d, 'w> ScopeExt<'d, 'w> for Scope<'d, 'w> {

fn cluster_auto_labelled(&mut self, lbl: &str) -> Scope<'_, 'w> {
let mut cluster = self.cluster();
cluster.set("label", lbl, lbl.contains(" "));
cluster.set("label", lbl, lbl.contains(' '));
cluster
}

Expand All @@ -49,22 +49,17 @@ trait ChildEdgeExt {
impl ChildEdgeExt for Targets {
fn edges(self, out: &mut Scope, from: &NodeId, lbl: &str) -> Targets {
for target in &self {
out.edge(&from, &target).attributes().set_label(lbl);
out.edge(from, target).attributes().set_label(lbl);
}
self
}
}

type Targets = Vec<NodeId>;

#[derive(Default)]
pub struct AstToDot {}

impl Default for AstToDot {
fn default() -> Self {
AstToDot {}
}
}

impl<T> ToDotGraph<T> for AstToDot
where
AstToDot: ToDot<T>,
Expand Down Expand Up @@ -96,7 +91,7 @@ where
self.to_dot(&mut digraph, ast);
}

return String::from_utf8(output_bytes).expect("invalid utf8");
String::from_utf8(output_bytes).expect("invalid utf8")
}
}

Expand All @@ -120,7 +115,7 @@ where
fn to_dot(&mut self, out: &mut Scope, asts: &Vec<T>) -> Targets {
let mut res = Vec::with_capacity(asts.len());
for ast in asts {
res.extend(self.to_dot(out, &ast));
res.extend(self.to_dot(out, ast));
}
res
}
Expand All @@ -133,7 +128,7 @@ where
fn to_dot(&mut self, out: &mut Scope, ast: &Option<T>) -> Targets {
match ast {
None => vec![],
Some(ast) => self.to_dot(out, &ast),
Some(ast) => self.to_dot(out, ast),
}
}
}
Expand Down Expand Up @@ -455,7 +450,7 @@ fn symbol_primitive_to_label(sym: &ast::SymbolPrimitive) -> String {
use ast::CaseSensitivity;
match &sym.case {
CaseSensitivity::CaseSensitive => format!("'{}'", sym.value),
CaseSensitivity::CaseInsensitive => format!("{}", sym.value),
CaseSensitivity::CaseInsensitive => sym.value.to_string(),
}
}

Expand Down
2 changes: 1 addition & 1 deletion extension/partiql-extension-visualize/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ pub trait ToDotGraph<T> {
}

#[cfg(feature = "visualize-dot")]
pub(crate) const FG_COLOR: &'static str = "\"#839496\"";
pub(crate) const FG_COLOR: &str = "\"#839496\"";
29 changes: 9 additions & 20 deletions extension/partiql-extension-visualize/src/plan_to_dot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,9 @@ use std::collections::HashMap;

use crate::common::{ToDotGraph, FG_COLOR};

#[derive(Default)]
pub struct PlanToDot {}

impl Default for PlanToDot {
fn default() -> Self {
PlanToDot {}
}
}

impl PlanToDot {
pub(crate) fn to_dot(&self, scope: &mut Scope, plan: &LogicalPlan<BindingsOp>) {
let mut graph_nodes = HashMap::new();
Expand Down Expand Up @@ -71,10 +66,10 @@ impl PlanToDot {
let clauses = [
lo.limit
.as_ref()
.map(|e| format!("limit {}", expr_to_str(&e))),
.map(|e| format!("limit {}", expr_to_str(e))),
lo.offset
.as_ref()
.map(|e| format!("offset {}", expr_to_str(&e))),
.map(|e| format!("offset {}", expr_to_str(e))),
]
.iter()
.filter_map(|o| o.as_ref())
Expand All @@ -92,10 +87,7 @@ impl PlanToDot {
format!(
"{{ {} join | {} }}",
kind,
join.on
.as_ref()
.map(|e| expr_to_str(e))
.unwrap_or("".to_string())
join.on.as_ref().map(expr_to_str).unwrap_or("".to_string())
)
}
BindingsOp::BagOp(_) => "bag op (TODO)".to_string(),
Expand All @@ -108,9 +100,7 @@ impl PlanToDot {
.join(" | "),
)
}
BindingsOp::ProjectAll => {
format!("{{project * }}")
}
BindingsOp::ProjectAll => "{project * }".to_string(),
BindingsOp::ProjectValue(pv) => {
format!("{{project value | {} }}", expr_to_str(&pv.expr))
}
Expand All @@ -135,8 +125,7 @@ impl PlanToDot {
}
BindingsOp::Sink => "sink".to_string(),
};
node.set_shape(Shape::Mrecord)
.set_label(&format!("{label}"));
node.set_shape(Shape::Mrecord).set_label(&label.to_string());

node.id()
}
Expand All @@ -163,8 +152,8 @@ fn expr_to_str(expr: &ValueExpr) -> String {
let expr = expr.replace('{', "\\{");
let expr = expr.replace('}', "\\}");
let expr = expr.replace('<', "\\<");
let expr = expr.replace('>', "\\>");
expr

expr.replace('>', "\\>")
}
}
}
Expand Down Expand Up @@ -206,6 +195,6 @@ impl ToDotGraph<LogicalPlan<BindingsOp>> for PlanToDot {
self.to_dot(&mut digraph, plan);
}

return String::from_utf8(output_bytes).expect("invalid utf8");
String::from_utf8(output_bytes).expect("invalid utf8")
}
}
22 changes: 8 additions & 14 deletions partiql-eval/src/eval/expr/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,22 +118,16 @@ impl EvalPathComponent {
impl EvalExpr for EvalPath {
fn evaluate<'a>(&'a self, bindings: &'a Tuple, ctx: &'a dyn EvalContext) -> Cow<'a, Value> {
let value = self.expr.evaluate(bindings, ctx);
let mut path_componenents = self.components.iter();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a clippy warning that looks unrelated to this PR's changes on L3:

unused import: BorrowMut

Copy link
Contributor Author

@jpschorr jpschorr Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

match value {
Cow::Borrowed(borrowed) => self
.components
.iter()
.fold(Some(borrowed), |v, path| {
v.and_then(|v| path.get_val(v, bindings, ctx))
})
.map_or_else(|| Cow::Owned(Value::Missing), Cow::Borrowed),
Cow::Owned(owned) => self
.components
.iter()
.fold(Some(owned), |v, path| {
v.and_then(|v| path.take_val(v, bindings, ctx))
})
.map_or_else(|| Cow::Owned(Value::Missing), Cow::Owned),
Cow::Borrowed(borrowed) => path_componenents
.try_fold(borrowed, |v, path| path.get_val(v, bindings, ctx))
.map(Cow::Borrowed),
Cow::Owned(owned) => path_componenents
.try_fold(owned, |v, path| path.take_val(v, bindings, ctx))
.map(Cow::Owned),
}
.unwrap_or_else(|| Cow::Owned(Value::Missing))
}
}

Expand Down
Loading