Skip to content
Draft
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
8 changes: 7 additions & 1 deletion .github/workflows/labeler.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
name: 'Pull Request Labeler'
on:
- pull_request_target
# SAFETY: pull_request_target runs from the base branch with full token
# permissions. This is REQUIRED so the labeler can write labels on forked
# PRs. The only step is actions/labeler@v5, which reads the PR metadata and
# applies labels — it does NOT checkout code. DO NOT add actions/checkout
# or any other step that processes PR-supplied files, as that would allow
# PR authors to exfiltrate repository secrets.
pull_request_target

jobs:
labeler:
Expand Down
4 changes: 4 additions & 0 deletions core/wren-core-base/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ repository = "https://github.com/Canner/WrenAI"
keywords = ["sql", "semantic-layer", "mdl", "manifest", "wren"]
categories = ["database"]

[lints.clippy]
unwrap_used = "deny"
expect_used = "deny"

[features]
python-binding = ["dep:pyo3"]
default = []
Expand Down
9 changes: 9 additions & 0 deletions core/wren-core-wasm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,15 @@ impl WrenEngine {
use arrow::json::writer::JsonArray;
use arrow::json::WriterBuilder;

// Require MDL to be loaded — without it, the DataFusion context has no
// Wren analyzer rules and SQL would bypass column-level ACL, row-level
// security, and model rewrites entirely.
if self.analyzed_mdl.is_none() {
return Err(JsError::new(
"No MDL loaded. Call loadMDL() first, or use cubeQuery() for structured queries.",
));
}

self.runtime.block_on(async {
let df = self
.ctx
Expand Down
4 changes: 4 additions & 0 deletions core/wren-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ repository = "https://github.com/Canner/WrenAI"
rust-version = "1.78"
version = "0.3.0"

[workspace.lints.clippy]
unwrap_used = "deny"
expect_used = "deny"

[workspace.dependencies]
async-trait = "0.1.89"
datafusion = { version = "53", default-features = false, features = [
Expand Down
3 changes: 3 additions & 0 deletions core/wren-core/benchmarks/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ repository.workspace = true
rust-version.workspace = true
version.workspace = true

[lints]
workspace = true

[lib]
name = "wren_benchmarks"
path = "src/lib.rs"
Expand Down
3 changes: 3 additions & 0 deletions core/wren-core/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ repository = { workspace = true }
license = { workspace = true }
authors = { workspace = true }

[lints]
workspace = true

[lib]
name = "wren_core"
path = "src/lib.rs"
Expand Down
16 changes: 10 additions & 6 deletions core/wren-core/core/src/logical_plan/analyze/model_generation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::DataFusionError;
use datafusion::common::alias::AliasGenerator;
use datafusion::common::config::ConfigOptions;
use datafusion::common::tree_node::{Transformed, TransformedResult};
use datafusion::common::{plan_err, Result};
use datafusion::common::{internal_datafusion_err, plan_datafusion_err, plan_err, Result};
use datafusion::logical_expr::{col, ident, Extension, UserDefinedLogicalNodeCore};
use datafusion::logical_expr::{Expr, LogicalPlan, LogicalPlanBuilder};
use datafusion::optimizer::analyzer::AnalyzerRule;
Expand Down Expand Up @@ -69,8 +69,10 @@ impl ModelGenerationRule {
model_plan
.required_exprs
.iter()
.map(|expr| rebase_column(expr, &alias).unwrap())
.collect()
.map(|expr| rebase_column(expr, &alias).map_err(|e| {
internal_datafusion_err!("failed to rebase column: {e}")
}))
.collect::<Result<Vec<_>>>()?
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} else {
model_plan.required_exprs.clone()
};
Expand Down Expand Up @@ -114,7 +116,9 @@ impl ModelGenerationRule {
.analyzed_wren_mdl
.wren_mdl()
.get_model(&model_plan.model_name)
.expect("Model not found"),
.ok_or_else(|| {
plan_datafusion_err!("Model not found: {}", model_plan.model_name)
})?,
);
let mut required_exprs = model_plan.required_exprs.clone();
required_exprs.iter_mut().try_for_each(|expr| {
Expand All @@ -124,7 +128,7 @@ impl ModelGenerationRule {
let table_scan = match &model_plan.original_table_scan {
Some(LogicalPlan::TableScan(original_scan)) => {
let table_ref_name = model.table_reference()
.expect("TableScan-based model must have a table_reference");
.ok_or_else(|| plan_datafusion_err!("TableScan-based model must have a table_reference"))?;
LogicalPlanBuilder::scan_with_filters(
TableReference::from(table_ref_name),
create_remote_table_source(
Expand Down Expand Up @@ -157,7 +161,7 @@ impl ModelGenerationRule {
}
wren_core_base::mdl::ModelSource::TableReference => {
let table_ref_name = model.table_reference()
.expect("table_reference model must have a table_reference");
.ok_or_else(|| plan_datafusion_err!("table_reference model must have a table_reference"))?;
LogicalPlanBuilder::scan(
TableReference::from(table_ref_name),
create_remote_table_source(
Expand Down
26 changes: 18 additions & 8 deletions core/wren-core/core/src/logical_plan/analyze/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ impl ModelPlanNodeBuilder {
self.fields.iter().cloned().collect(),
HashMap::new(),
)
.expect("create schema failed"),
.map_err(|e| internal_datafusion_err!("create schema failed: {e}"))?,
);

let mut iter = self.directed_graph.node_indices();
Expand Down Expand Up @@ -604,12 +604,16 @@ impl ModelPlanNodeBuilder {

let mut iter = column_graph.node_indices();

let start = iter.next().unwrap();
let start = iter.next().ok_or_else(|| {
internal_datafusion_err!("column graph has no nodes")
})?;
let source_required_fields = partial_model_required_fields
.get(&model_ref)
.map(|c| c.iter().cloned().map(|c| c.expr).collect())
.unwrap_or_default();
let source = column_graph.node_weight(start).unwrap();
let source = column_graph.node_weight(start).ok_or_else(|| {
internal_datafusion_err!("node not found in column graph")
})?;

let source_chain = RelationChain::source(
source,
Expand Down Expand Up @@ -684,7 +688,9 @@ fn collect_partial_model_plan_for_calculation(
if column.is_calculated {
let expr = create_wren_expr_for_model(
&c.name,
dataset.try_as_model().unwrap(),
dataset.try_as_model().ok_or_else(|| {
internal_datafusion_err!("expected dataset to be a model")
})?,
Arc::clone(&session_state_ref),
)?;
required_fields
Expand Down Expand Up @@ -941,8 +947,12 @@ fn merge_graph(
let Some((source, target)) = new_graph.edge_endpoints(edge) else {
return internal_err!("Edge not found");
};
let source = node_map.get(&source).unwrap();
let target = node_map.get(&target).unwrap();
let source = node_map.get(&source).ok_or_else(|| {
internal_datafusion_err!("source node not found in merged graph")
})?;
let target = node_map.get(&target).ok_or_else(|| {
internal_datafusion_err!("target node not found in merged graph")
})?;
// Skip duplicate edges between the same pair of nodes: the same
// relationship may appear in multiple calc-col sub-graphs (e.g. two
// calc cols traversing the same relationship), and adding parallel
Expand Down Expand Up @@ -1154,7 +1164,7 @@ impl ModelSourceNode {
let fields = fields_buffer.into_iter().collect::<Vec<_>>();
let schema_ref = DFSchemaRef::new(
DFSchema::new_with_metadata(fields, HashMap::new())
.expect("create schema failed"),
.map_err(|e| internal_datafusion_err!("create schema failed: {e}"))?,
);
let required_exprs = required_exprs_buffer
.into_iter()
Expand Down Expand Up @@ -1271,7 +1281,7 @@ impl CalculationPlanNode {
.collect::<Result<Vec<_>>>()?;
let schema_ref = DFSchemaRef::new(
DFSchema::new_with_metadata(output_field, HashMap::new())
.expect("create schema failed"),
.map_err(|e| internal_datafusion_err!("create schema failed: {e}"))?,
);
Ok(Self {
calculation,
Expand Down
10 changes: 7 additions & 3 deletions core/wren-core/core/src/logical_plan/analyze/relation_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::mdl::Dataset;
use crate::mdl::{AnalyzedWrenMDL, SessionStateRef};
use crate::DataFusionError;
use datafusion::common::alias::AliasGenerator;
use datafusion::common::{internal_err, plan_err, DFSchema, DFSchemaRef, Result};
use datafusion::common::{internal_datafusion_err, internal_err, plan_err, DFSchema, DFSchemaRef, Result};
use datafusion::common::{plan_datafusion_err, TableReference};
use datafusion::logical_expr::{
col, Expr, Extension, LogicalPlan, LogicalPlanBuilder, SubqueryAlias,
Expand Down Expand Up @@ -81,14 +81,18 @@ impl RelationChain {
let mut prev = start;

for next in iter {
let target = directed_graph.node_weight(next).unwrap();
let target = directed_graph.node_weight(next).ok_or_else(|| {
internal_datafusion_err!("node not found in relation chain graph")
})?;
let link_index = directed_graph
.find_edge(prev, next)
.or_else(|| directed_graph.find_edge(start, next));
let Some(link_index) = link_index else {
break;
};
let link = directed_graph.edge_weight(link_index).unwrap();
let link = directed_graph.edge_weight(link_index).ok_or_else(|| {
internal_datafusion_err!("edge not found in relation chain graph")
})?;
let target_ref = TableReference::full(
analyzed_wren_mdl.wren_mdl().catalog(),
analyzed_wren_mdl.wren_mdl().schema(),
Expand Down
6 changes: 5 additions & 1 deletion core/wren-core/core/src/logical_plan/unparser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ impl UserDefinedLogicalNodeUnparser for SqlReferenceNodeUnparser {
)));
}

let statement = statements.into_iter().next().expect("checked length == 1");
let statement = statements.into_iter().next().ok_or_else(|| {
datafusion::error::DataFusionError::Internal(
"expected exactly one statement after length check".to_string(),
)
})?;
let Statement::Query(parsed_query) = statement else {
return Err(datafusion::error::DataFusionError::Plan(format!(
"ref_sql for model '{}' must be a SELECT statement",
Expand Down
24 changes: 23 additions & 1 deletion core/wren-core/core/src/mdl/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,29 @@ pub async fn apply_wren_on_ctx(
// SessionStates within the call share that snapshot, isolating in-flight
// catalog registration from the base context and concurrent calls. See
// `clone_catalog_list` for the exact sharing contract.
let private_catalog_list = clone_catalog_list(ctx.state().catalog_list());
let private_catalog_list: Arc<dyn CatalogProviderList> = if matches!(mode, Mode::LocalRuntime) {
// Strip non-MDL catalogs in LocalRuntime mode to prevent direct
// physical table access that bypasses the MDL semantic layer
// (column-level ACL, row-level security, model rewrites). Only the
// MDL's own catalog and information_schema survive the filter.
// ModelGenerationRule uses create_remote_table_source (catalog-free)
// so MDL model expansion is unaffected.
let state = ctx.state();
let original = state.catalog_list();
let wren_mdl = analyzed_mdl.wren_mdl();
let mdl_catalog = wren_mdl.catalog().to_string();
let filtered = MemoryCatalogProviderList::new();
for name in original.catalog_names() {
if name == mdl_catalog || name == "information_schema" {
if let Some(catalog) = original.catalog(&name) {
filtered.register_catalog(name, catalog);
}
}
}
Arc::new(filtered)
} else {
clone_catalog_list(ctx.state().catalog_list())
};
let reset_default_catalog_schema = Arc::new(RwLock::new(
SessionStateBuilder::new_from_existing(ctx.state())
.with_config(config.clone())
Expand Down
5 changes: 2 additions & 3 deletions core/wren-core/core/src/mdl/dataset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,8 @@ impl Dataset {
.unwrap_or_else(|| quoted(model.name()));

let schema = register_tables
.map(|rt| rt.get(&qualifier))
.filter(|rt| rt.is_some())
.map(|rt| rt.unwrap().schema());
.and_then(|rt| rt.get(&qualifier))
.map(|rt| rt.schema());

if let Some(schema) = schema {
DFSchema::try_from_qualified_schema(qualifier.as_str(), &schema)
Expand Down
12 changes: 10 additions & 2 deletions core/wren-core/core/src/mdl/dialect/inner_dialect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,10 @@ pub struct OracleDialect {}
impl InnerDialect for OracleDialect {
fn identifier_quote_style(&self, identifier: &str) -> Option<char> {
// Oracle defaults to upper case for identifiers
let identifier_regex = Regex::new(r"^[a-zA-Z_][a-zA-Z0-9_]*$").unwrap();
static IDENTIFIER_REGEX: std::sync::OnceLock<Regex> = std::sync::OnceLock::new();
let identifier_regex = IDENTIFIER_REGEX.get_or_init(|| {
Regex::new(r"^[a-zA-Z_][a-zA-Z0-9_]*$").expect("built-in regex is valid")
});
if ALL_KEYWORDS.contains(&identifier.to_uppercase().as_str())
|| !identifier_regex.is_match(identifier)
|| non_uppercase(identifier)
Expand Down Expand Up @@ -502,7 +505,12 @@ impl InnerDialect for ClickHouseDialect {
"toDayOfWeek",
&[args[1].clone()],
)?
.expect("clickhouse_function_to_sql always returns Some");
.ok_or_else(|| {
datafusion::error::DataFusionError::Plan(
"clickhouse_function_to_sql should return Some for toDayOfWeek"
.to_string(),
)
})?;
return Ok(Some(ast::Expr::BinaryOp {
left: Box::new(inner_expr),
op: ast::BinaryOperator::Modulo,
Expand Down
5 changes: 4 additions & 1 deletion core/wren-core/core/src/mdl/dialect/wren_dialect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ impl Dialect for WrenDialect {
return Some(quote);
}

let identifier_regex = Regex::new(r"^[a-zA-Z_][a-zA-Z0-9_]*$").unwrap();
static IDENTIFIER_REGEX: std::sync::OnceLock<Regex> = std::sync::OnceLock::new();
let identifier_regex = IDENTIFIER_REGEX.get_or_init(|| {
Regex::new(r"^[a-zA-Z_][a-zA-Z0-9_]*$").expect("built-in regex is valid")
});
if ALL_KEYWORDS.contains(&identifier.to_uppercase().as_str())
|| !identifier_regex.is_match(identifier)
|| non_lowercase(identifier)
Expand Down
25 changes: 14 additions & 11 deletions core/wren-core/core/src/mdl/function/remote_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ impl RemoteFunction {
if coercions.iter().any(|r| r.is_err()) {
signatures.push(TypeSignature::Exact(types.clone()));
} else {
let coercions = coercions.into_iter().map(|r| r.unwrap()).collect();
let coercions = coercions
.into_iter()
.filter_map(|r| r.ok())
.collect::<Vec<_>>();
signatures.push(TypeSignature::Coercible(coercions));
}
}
Expand Down Expand Up @@ -274,14 +277,14 @@ fn build_document(func: &RemoteFunction) -> Documentation {
"",
);
if let Some(param_names) = func.param_names.as_ref() {
let param_types = func.param_types.as_deref().unwrap_or(&[]);
for (i, name) in param_names.iter().enumerate() {
let description = func
.param_types
.as_ref()
.map(|types| types[i].clone().unwrap_or("".to_string()))
.unwrap_or("".to_string());
let description = param_types
.get(i)
.and_then(|t| t.clone())
.unwrap_or_default();
builder = builder
.with_argument(name.clone().unwrap_or("".to_string()), description);
.with_argument(name.clone().unwrap_or_default(), description);
}
}
builder.build()
Expand Down Expand Up @@ -376,8 +379,8 @@ impl ByPassAggregateUDF {

impl From<RemoteFunction> for ByPassAggregateUDF {
fn from(func: RemoteFunction) -> Self {
// just panic if the return type is not valid to avoid we input invalid type
let return_type = ReturnType::from_str(&func.return_type).unwrap();
let return_type = ReturnType::from_str(&func.return_type)
.unwrap_or(ReturnType::Specific(DataType::Utf8));
ByPassAggregateUDF {
return_type,
signature: func.get_signature(),
Expand Down Expand Up @@ -458,8 +461,8 @@ impl ByPassWindowFunction {

impl From<RemoteFunction> for ByPassWindowFunction {
fn from(func: RemoteFunction) -> Self {
// just panic if the return type is not valid to avoid we input invalid type
let return_type = ReturnType::from_str(&func.return_type).unwrap();
let return_type = ReturnType::from_str(&func.return_type)
.unwrap_or(ReturnType::Specific(DataType::Utf8));
ByPassWindowFunction {
return_type,
signature: func.get_signature(),
Expand Down
Loading
Loading