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

Bug fix for predicate order for queries with a function/lambda select clause #126

Merged
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
11 changes: 7 additions & 4 deletions clorm/orm/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -3415,7 +3415,7 @@ def make_query(qp, factsets, factindexes):
# ------------------------------------------------------------------------------


def make_outputter(insig, outsig):
def make_outputter(queryroots, insig, outsig):
def make_simple_outputter():
af = make_input_alignment_functor(insig, outsig)
return lambda intuple, af=af: af(intuple)
Expand All @@ -3432,7 +3432,8 @@ def make_complex_outputter():
tmp = make_input_alignment_functor(insig, out.paths)
metasig.append(lambda x, af=tmp, f=out.functor: f(*af(x)))
elif callable(out):
metasig.append(lambda x, f=out: f(*x))
tmp = make_input_alignment_functor(insig, queryroots)
metasig.append(lambda x, af=tmp, f=out: f(*af(x)))
else:
metasign.append(lambda x, out=out: out)

Expand Down Expand Up @@ -4232,10 +4233,11 @@ def all(self) -> Generator[Any, None, None]:
(self._qplan, self._query) = self._make_plan_and_query()

outsig = self._qspec.select
roots = self._qspec.roots
if outsig is None or not outsig:
outsig = self._qspec.roots

self._outputter = make_outputter(self._qplan.output_signature, outsig)
self._outputter = make_outputter(roots, self._qplan.output_signature, outsig)
self._unwrap = not self._qspec.tuple and len(outsig) == 1
self._distinct = self._qspec.distinct

Expand Down Expand Up @@ -4387,9 +4389,10 @@ def execute_fn(facts):
)

outsig = self._qspec.select
roots = self._qspec.roots
if outsig is None or not outsig:
outsig = self._qspec.roots
self._outputter = make_outputter(self._qplan.output_signature, outsig)
self._outputter = make_outputter(roots, self._qplan.output_signature, outsig)
self._unwrap = False
self._distinct = self._qspec.distinct

Expand Down
55 changes: 54 additions & 1 deletion tests/test_orm_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -2593,9 +2593,11 @@ def f_in_factmaps(f, factmaps):
return f in fm.factset


def factmaps_dict(facts, indexes=[]):
def factmaps_dict(facts, indexes=None):
from itertools import groupby

if indexes is None:
indexes = []
indexes = sorted([hashable_path(p) for p in indexes])
predicate2indexes = {}
for k, g in groupby(indexes, lambda p: path(p).meta.predicate):
Expand Down Expand Up @@ -3431,6 +3433,57 @@ def replace(subroots, fn):
self.assertTrue(f_in_factmaps(F(35, "a"), factmaps))
self.assertTrue(f_in_factmaps(F(35, "foo"), factmaps))

# ----------------------------------------------------------------------------------------
# Test the output order of the predicates for a complex join matches the input order. This
# is to track down a bug that is not re-aligning the order when the query execution plan
# internally changes the order.
#
# NOTE: the original bug is to do with passing a function/lambda to the "select" clause.
# Internally since no input signature is associated with the function/lambda it should be
# given the signature of the query roots. This wasn't happening and was instead being
# passed the inputs as they were generated by the query plan. The fix is to use the query
# root as the input signature for a function/lambda.
# ----------------------------------------------------------------------------------------
def test_api_QueryExecutor_output_alignment(self):
class F(Predicate):
aint = IntegerField

class G(Predicate):
astr = StringField

class H(Predicate):
aint = IntegerField
astr = StringField

pw = process_where
pj = process_join
roots = [F, H, G]

factmaps = factmaps_dict(
[F(1), F(2), G("a"), G("b"), H(1, "a"), H(2, "b")], [G.astr, F.aint, H.astr]
)

qspec = QuerySpec(
roots=roots,
join=pj([F.aint == H.aint, G.astr == H.astr], roots),
where=pw(F.aint == 1, roots),
select=(lambda f, h, g: (f, h, g),),
)

qe = QueryExecutor(factmaps, qspec)
plan, _ = qe._make_plan_and_query()
plan_roots = [x.root.meta.predicate for x in plan]

# To check that the bug is fixed and the order of the output is the same as the input
# we need to make sure the order of the input roots is different to the plan roots.
self.assertNotEqual(roots, plan_roots)
output = list(qe.all())
self.assertEqual(len(output), 1)
f, h, g = output[0]
self.assertEqual(type(f), F)
self.assertEqual(type(g), G)
self.assertEqual(type(h), H)


# ------------------------------------------------------------------------------
# main
Expand Down
Loading