Skip to content

Commit 98e8893

Browse files
committed
Fix postgres filter continuation and island entry correctness
1 parent 1024ba5 commit 98e8893

File tree

1 file changed

+88
-8
lines changed
  • libs/@local/hashql/eval/src/postgres/filter

1 file changed

+88
-8
lines changed

libs/@local/hashql/eval/src/postgres/filter/mod.rs

Lines changed: 88 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,24 @@ impl From<Continuation> for Expression {
9595
// (filter, block, locals, values)
9696
let row = match continuation {
9797
Continuation::Return { filter } => {
98+
// Normalize SQL three-valued logic to two-valued filter semantics:
99+
// `NULL` from predicate evaluation should behave like `FALSE`, not
100+
// like continuation passthrough.
101+
let filter = filter.grouped().cast(PostgresType::Boolean);
102+
let filter_is_not_false = Self::Unary(UnaryExpression {
103+
op: UnaryOperator::IsNotFalse,
104+
expr: Box::new(filter.clone()),
105+
});
106+
let filter_is_not_null = Self::Unary(UnaryExpression {
107+
op: UnaryOperator::Not,
108+
expr: Box::new(Self::Unary(UnaryExpression {
109+
op: UnaryOperator::IsNull,
110+
expr: Box::new(filter),
111+
})),
112+
});
113+
98114
vec![
99-
filter.grouped().cast(PostgresType::Boolean),
115+
Self::all(vec![filter_is_not_false, filter_is_not_null]),
100116
null.clone(),
101117
null.clone(),
102118
null,
@@ -177,11 +193,18 @@ fn finish_switch_int<A: Allocator>(
177193

178194
debug_assert_eq!(branch_results.len(), targets.values().len());
179195

180-
// SwitchInt compares the discriminant against integer values. If the
181-
// discriminant is a boolean expression (e.g. `IS NOT NULL`), PostgreSQL
182-
// rejects `boolean = integer`. Casting to `::int` is safe for all types
183-
// and a no-op when the discriminant is already integral.
184-
let discriminant = Box::new(discriminant.grouped().cast(PostgresType::Int));
196+
// Preserve the existing boolean-switch behavior (`::int`) for 0/1 cases,
197+
// but avoid 32-bit narrowing for wider SwitchInt values.
198+
let cast = if targets
199+
.values()
200+
.iter()
201+
.all(|&value| i32::try_from(value).is_ok())
202+
{
203+
PostgresType::Int
204+
} else {
205+
PostgresType::Numeric
206+
};
207+
let discriminant = Box::new(discriminant.grouped().cast(cast.clone()));
185208

186209
let mut discriminant = Some(discriminant);
187210
let mut conditions = Vec::with_capacity(targets.values().len());
@@ -197,7 +220,7 @@ fn finish_switch_int<A: Allocator>(
197220
let when = Expression::Binary(BinaryExpression {
198221
op: BinaryOperator::Equal,
199222
left: discriminant,
200-
right: Box::new(Expression::Constant(query::Constant::U128(value))),
223+
right: Box::new(Expression::Constant(query::Constant::U128(value)).cast(cast.clone())),
201224
});
202225

203226
conditions.push((when, then));
@@ -605,6 +628,48 @@ impl<'ctx, 'heap, A: Allocator, S: Allocator> GraphReadFilterCompiler<'ctx, 'hea
605628
unreachable!("The postgres island always has an entry block (BasicBlockId::START)")
606629
}
607630

631+
fn find_external_entry_target(
632+
&self,
633+
island: &IslandNode,
634+
entry_block: BasicBlockId,
635+
) -> Option<(BasicBlockId, Target<'heap>)> {
636+
let mut incoming = None;
637+
638+
for predecessor in self.body.basic_blocks.predecessors(entry_block) {
639+
if island.contains(predecessor) {
640+
continue;
641+
}
642+
643+
let terminator = &self.body.basic_blocks[predecessor].terminator.kind;
644+
645+
if let TerminatorKind::GraphRead(read) = terminator
646+
&& read.target == entry_block
647+
{
648+
let target = Target::block(entry_block);
649+
650+
if let Some((_, existing)) = incoming {
651+
debug_assert_eq!(existing, target);
652+
} else {
653+
incoming = Some((predecessor, target));
654+
}
655+
}
656+
657+
for &target in terminator.successor_targets() {
658+
if target.block != entry_block {
659+
continue;
660+
}
661+
662+
if let Some((_, existing)) = incoming {
663+
debug_assert_eq!(existing, target);
664+
} else {
665+
incoming = Some((predecessor, target));
666+
}
667+
}
668+
}
669+
670+
incoming
671+
}
672+
608673
fn compile_island_exit(
609674
&mut self,
610675
db: &mut DatabaseContext<'heap, A>,
@@ -631,6 +696,12 @@ impl<'ctx, 'heap, A: Allocator, S: Allocator> GraphReadFilterCompiler<'ctx, 'hea
631696
}
632697

633698
for local in live_out {
699+
// The environment local is immutable and available independently from
700+
// the local expression map, so it does not need to be serialized here.
701+
if local == Local::ENV {
702+
continue;
703+
}
704+
634705
let value = self
635706
.locals
636707
.lookup(local)
@@ -739,8 +810,17 @@ impl<'ctx, 'heap, A: Allocator, S: Allocator> GraphReadFilterCompiler<'ctx, 'hea
739810
{
740811
debug_assert_eq!(island.target(), TargetId::Postgres);
741812

813+
let entry_block = self.find_entry_block(island);
814+
815+
// Non-start islands may receive block arguments from predecessors outside
816+
// the island. Seed entry parameters once before starting compilation.
817+
if let Some((from, target)) = self.find_external_entry_target(island, entry_block) {
818+
let span = self.body.basic_blocks[from].terminator.span;
819+
self.assign_params(db, span, &target);
820+
}
821+
742822
let mut stack = Vec::new_in(self.scratch.clone());
743-
stack.push(Frame::Compile(self.find_entry_block(island)));
823+
stack.push(Frame::Compile(entry_block));
744824

745825
let mut results = Vec::new_in(self.scratch.clone());
746826

0 commit comments

Comments
 (0)