Skip to content
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

## [Unreleased]

### Fixed

- **`UPDATE … SET … FROM` with same-named columns**: an `UPDATE` was rejected as ambiguous when a table in the `FROM` clause had a column with the same name as the column being assigned. The assignment now always refers to the table being updated, so these statements work and the assigned value gets the target column's type — encrypted or not.

- **Encrypted values as row counts are rejected**: an encrypted column used in `LIMIT`, `OFFSET`, or `FETCH` (for example `LIMIT enc_col`) is now rejected with a type error instead of being forwarded to the database.

- **Statements Proxy cannot type-check fail with a clear error**: a statement Proxy admits for type checking but has no support for is now rejected immediately with an error naming the statement, instead of surfacing later as an opaque resolution error. No currently-supported statement is affected.

## [3.0.0] - 2026-08-05

### Changed
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use eql_mapper_macros::trace_infer;
use sqltk::parser::ast::{
Expr, OrderBy, OrderByKind, Query, Select, SelectItem, SetExpr, Value as SqltkValue,
Expr, Fetch, LimitClause, Offset, OrderBy, OrderByKind, Query, Select, SelectItem, SetExpr,
Value as SqltkValue,
};

use crate::{
inference::{InferType, TypeError},
inference::{unifier::Type, InferType, TypeError},
EqlTrait, TypeInferencer,
};

Expand Down Expand Up @@ -45,7 +46,13 @@ pub(crate) fn resolve_positional_key<'ast>(
#[trace_infer]
impl<'ast> InferType<'ast, Query> for TypeInferencer<'ast> {
fn infer_exit(&mut self, query: &'ast Query) -> Result<(), TypeError> {
let Query { body, order_by, .. } = query;
let Query {
body,
order_by,
limit_clause,
fetch,
..
} = query;

self.unify_nodes(query, &**body)?;

Expand Down Expand Up @@ -78,6 +85,51 @@ impl<'ast> InferType<'ast, Query> for TypeInferencer<'ast> {
}
}

// Row-count expressions in LIMIT/OFFSET/FETCH are evaluated by the
// database as plain integers and can never be encrypted, so pin them
// to `Native`. Without this a placeholder in `LIMIT $1` is left as an
// unconstrained type variable, which later surfaces as an opaque
// "unresolved type variable" error instead of type-checking cleanly.
// `Query::locks` (FOR UPDATE/SHARE) carries no expressions, so there
// is nothing to constrain there.
if let Some(limit_clause) = limit_clause {
match limit_clause {
LimitClause::LimitOffset {
limit,
offset,
limit_by,
} => {
if let Some(limit) = limit {
self.unify_node_with_type(limit, Type::native())?;
}
if let Some(Offset { value, .. }) = offset {
self.unify_node_with_type(value, Type::native())?;
}
// `LIMIT n BY expr, …` (ClickHouse syntax) — the BY
// expressions are per-group keys, not row counts, so
// `Native` would be the wrong constraint, and grouping on
// an encrypted column would need its equality term.
// PostgreSQL rejects the syntax anyway; rejecting it here
// keeps the keys from passing through unconstrained.
if !limit_by.is_empty() {
return Err(TypeError::UnsupportedSqlFeature("LIMIT ... BY".into()));
}
}
LimitClause::OffsetCommaLimit { offset, limit } => {
self.unify_node_with_type(offset, Type::native())?;
self.unify_node_with_type(limit, Type::native())?;
}
}
}

if let Some(Fetch {
quantity: Some(quantity),
..
}) = fetch
{
self.unify_node_with_type(quantity, Type::native())?;
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Ok(())
}
}
60 changes: 54 additions & 6 deletions packages/eql-mapper/src/inference/infer_type_impls/statement.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
use std::sync::Arc;

use eql_mapper_macros::trace_infer;
use sqltk::parser::ast::{AssignmentTarget, ObjectName, ObjectNamePart, Statement};
use sqltk::parser::ast::{AssignmentTarget, ObjectName, ObjectNamePart, Statement, TableFactor};

use crate::{inference::infer_type::InferType, unifier::Type, TypeError, TypeInferencer};
use crate::{
inference::infer_type::InferType,
unifier::{EqlTerm, EqlValue, NativeValue, Type, Value},
ColumnKind, TableColumn, TypeError, TypeInferencer,
};

#[trace_infer]
impl<'ast> InferType<'ast, Statement> for TypeInferencer<'ast> {
Expand All @@ -20,19 +26,48 @@ impl<'ast> InferType<'ast, Statement> for TypeInferencer<'ast> {
}

Statement::Update {
// FIXME: use table to resolve the assignments (instead of looking up the columns names in the scope).
table: _,
table,
assignments,
returning,
..
} => {
// Assignment targets belong to the table being updated, so
// resolve them against `table` directly. Resolving through the
// lexical scope would also see every `FROM`-joined relation,
// letting a same-named column there shadow the target column
// (or make it spuriously ambiguous).
let target_table = match &table.relation {
TableFactor::Table { name, .. } if table.joins.is_empty() => name,
_ => {
return Err(TypeError::UnsupportedSqlFeature(
"UPDATE target that is not a plain table".into(),
))
}
};

for assignment in assignments.iter() {
match &assignment.target {
AssignmentTarget::ColumnName(ObjectName(parts)) if parts.len() == 1 => {
let ObjectNamePart::Identifier(ident) = parts.last().unwrap();
let stc = self
.table_resolver
.resolve_table_column(target_table, ident)?;

let tc = TableColumn {
table: stc.table.clone(),
column: stc.column.clone(),
};

let value_ty = match &stc.kind {
ColumnKind::Native => Value::Native(NativeValue(Some(tc))),
ColumnKind::Eql(features, identity) => Value::Eql(EqlTerm::Full(
EqlValue(tc, identity.clone(), *features),
)),
};

self.unify_node_with_type(
&assignment.value,
self.resolve_ident(ident)?,
Arc::new(Type::Value(value_ty)),
)?;
}

Expand Down Expand Up @@ -88,7 +123,20 @@ impl<'ast> InferType<'ast, Statement> for TypeInferencer<'ast> {
// EXPLAIN itself returns metadata, not the query results - give it empty projection
self.unify_node_with_type(statement, Type::empty_projection())?;
}
_ => {}

// Invariant: every statement variant admitted by
// `requires_type_check` (see `eql_mapper.rs`) has an explicit arm
// above that constrains the statement's top-level type. This arm
// fails closed so that widening `requires_type_check` without
// adding a matching arm becomes a loud error instead of a
// silently-unconstrained statement.
unhandled => {
return Err(TypeError::InternalError(format!(
"type inference has no rule for statement `{unhandled}`; \
`requires_type_check` admits a statement variant that \
`InferType<'_, Statement>` does not handle"
)))
}
};

Ok(())
Expand Down
198 changes: 198 additions & 0 deletions packages/eql-mapper/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1303,6 +1303,204 @@ mod test {
);
}

/// In `UPDATE t1 SET x = ... FROM t2` the assignment target must resolve
/// against the table being updated, not through the lexical scope. The
/// scope also contains the `FROM` relations, so a same-named column there
/// used to make the target spuriously ambiguous (and could shadow it).
/// Here both tables have an `email` column; the assignment must get
/// `users.email` — the encrypted one.
#[test]
fn update_assignment_resolves_against_target_table_not_from_relation() {
let schema = resolver(schema! {
tables: {
users: {
id,
email (EQL: Eq),
}
aux: {
id,
email,
}
}
});

let statement = parse("UPDATE users SET email = $1 FROM aux WHERE users.id = aux.id");

let typed = match type_check(schema, &statement) {
Ok(typed) => typed,
Err(err) => panic!("type check failed: {err}"),
};

let target = Value::Eql(EqlTerm::Full(EqlValue::with_canonical_identity(
TableColumn {
table: id("users"),
column: id("email"),
},
EqlTraits::from(EqlTrait::Eq),
)));

assert_eq!(typed.params, vec![(Param(1), target)]);
assert_eq!(typed.projection, Projection(vec![]));
}

/// Proxy loads its schema from the database with *quoted* column idents
/// (`Ident::with_quote('"', ..)`) behind an editable resolver, while SQL
/// usually spells the same columns unquoted. A type identity derived from
/// an assignment target must still unify with one derived from the scope,
/// so the resolver has to return the schema's canonical idents rather than
/// echo the caller's spelling. With the caller's spelling,
/// `UPDATE t SET c = $1 WHERE c = $1` pinned the same param to
/// `EQL(t."c")` and `EQL(t.c)` and failed with "cannot unify EQL terms".
#[test]
fn update_reused_param_unifies_against_quoted_schema_idents() {
let eq = EqlTraits::from(EqlTrait::Eq);

let mut schema = Schema::new("public");
let mut table = crate::model::Table::new(Ident::new("encrypted"));
table.add_column(Arc::new(crate::model::Column::native(Ident::with_quote(
'"', "id",
))));
table.add_column(Arc::new(crate::model::Column::eql(
Ident::with_quote('"', "encrypted_text"),
eq,
crate::unifier::DomainIdentity::canonical(crate::unifier::TokenType::Text, eq),
)));
schema.add_table(table);

// The editable resolver is the one Proxy uses at runtime; it resolves
// through `SchemaDelta`, not `Schema`.
let resolver = Arc::new(TableResolver::new_editable(Arc::new(schema)));

let statement = parse("UPDATE encrypted SET encrypted_text = $1 WHERE encrypted_text = $1");

let typed = match type_check(resolver, &statement) {
Ok(typed) => typed,
Err(err) => panic!("type check failed: {err}"),
};

// The param's identity is the canonical (quoted) schema spelling.
let target = Value::Eql(EqlTerm::Full(EqlValue::with_canonical_identity(
TableColumn {
table: id("encrypted"),
column: Ident::with_quote('"', "encrypted_text"),
},
eq,
)));

assert_eq!(typed.params, vec![(Param(1), target)]);
}

/// The row-count expressions in `LIMIT`/`OFFSET` can never be encrypted,
/// so placeholders there must be pinned to `Native` at inference time.
/// Previously they were left as unconstrained type variables and only
/// resolved to `Native` by the late unresolved-value fallback in
/// `Unifier::resolve_unresolved_value_nodes` — this pins the guarantee
/// where the clause is inferred instead of relying on that fallback.
#[test]
fn limit_and_offset_placeholders_infer_native() {
let schema = resolver(schema! {
tables: {
users: {
id,
email (EQL: Eq),
}
}
});

let statement = parse("SELECT id FROM users LIMIT $1 OFFSET $2");

let typed = match type_check(schema, &statement) {
Ok(typed) => typed,
Err(err) => panic!("type check failed: {err}"),
};

assert_eq!(
typed.params,
vec![
(Param(1), Value::Native(NativeValue(None))),
(Param(2), Value::Native(NativeValue(None))),
]
);
}

/// Same as `limit_and_offset_placeholders_infer_native`, but for the
/// quantity in a `FETCH FIRST n ROWS ONLY` clause.
#[test]
fn fetch_first_placeholder_infers_native() {
let schema = resolver(schema! {
tables: {
users: {
id,
email (EQL: Eq),
}
}
});

let statement = parse("SELECT id FROM users FETCH FIRST $1 ROWS ONLY");

let typed = match type_check(schema, &statement) {
Ok(typed) => typed,
Err(err) => panic!("type check failed: {err}"),
};

assert_eq!(
typed.params,
vec![(Param(1), Value::Native(NativeValue(None)))]
);
}

/// Because `LIMIT` is pinned to `Native` at inference time, an encrypted
/// value can no longer flow into it silently — the mapper refuses the
/// statement instead of forwarding SQL that the database would reject
/// (or worse, that would leak a ciphertext into a row count).
#[test]
fn encrypted_column_in_limit_is_rejected() {
let schema = resolver(schema! {
tables: {
users: {
id,
email (EQL: Eq),
}
}
});

let statement = parse("SELECT id FROM users LIMIT email");

type_check(schema, &statement)
.expect_err("an encrypted column must not type check as a LIMIT row count");
}

/// A statement variant with no inference rule must fail closed with an
/// error stating the invariant, not traverse without constraining the
/// statement's top-level type. (`requires_type_check` never admits
/// `TRUNCATE`, so this can only be reached by calling `type_check`
/// directly — but if `requires_type_check` is ever widened without a
/// matching inference rule, this is the error that makes it loud.)
#[test]
fn statement_without_inference_rule_fails_closed() {
let schema = resolver(schema! {
tables: {
users: {
id,
}
}
});

let statement = parse("TRUNCATE TABLE users");

match type_check(schema, &statement) {
Ok(_) => panic!("expected type check to fail"),
Err(err) => assert_eq!(
err.to_string(),
format!(
"type inference has no rule for statement `{statement}`; \
`requires_type_check` admits a statement variant that \
`InferType<'_, Statement>` does not handle"
)
),
}
}

#[test]
fn delete() {
// init_tracing();
Expand Down
Loading
Loading