ClassAlias Reference is compiled to wrong sql

Bug #682989 reported by Jeroen T. Vermeulen
28
This bug affects 5 people
Affects Status Importance Assigned to Milestone
Storm
New
High
Unassigned

Bug Description

We've got an important query in Launchpad doing a more extensive version of:

Foo = ClassAlias(Table, "Foo")
Bar = ClassAlias(Table, "Bar")
bar_ids = [1, 2, 3]
selection = Select(
    Foo.id,
    tables=[Foo, Bar],
    where=And(
        Foo.x == Bar.x,
        Bar.id.is_in(bar_ids),
        Foo.y == None,
        Bar.y == None
    ))
store.find(Table, Table.id.is_in(selection)).set(attribute=value)

This results in SQL of the form:

UPDATE Table
SET attribute = value
WHERE Table.id IN (
    SELECT Foo.id
    FROM Table AS Foo, Table AS Bar
    WHERE
        Foo.x = Bar.x AND
        Bar.id IN (1, 2, 3) AND
        Table.y IS NULL AND
        Table.y IS NULL
);

The conditions "Foo.y == None" and "Bar.y == None" are both translated as "Table.y IS NULL"—which refers to the row being updated, not the rows referenced in the subquery!

For Foo.y this is not a problem in this case, since id is a key and so the match on "Table.id IN (SELECT Foo.id)" guarantees that Table and Foo are actually the same row. But x is not a key (actually we have a more complex join condition here) and so I can't say the same for Bar.y!

Revision history for this message
Thomas Herve (therve) wrote :

It seems a little bit more subtle than that. The translation of the example given in a test case worked for me: http://pastebin.ubuntu.com/538211/

Changed in storm:
importance: Undecided → High
status: New → Incomplete
assignee: nobody → Thomas Herve (therve)
milestone: none → 0.19
description: updated
Revision history for this message
Thomas Herve (therve) wrote :

It's a problem with Reference compilation. I'm looking at it.

Changed in storm:
status: Incomplete → New
Revision history for this message
Thomas Herve (therve) wrote :

Here is a simple test case:

    def test_alias_reference(self):
        c = StringIO()
        debug(True, c)
        Bar1 = ClassAlias(Bar, "bar1")
        self.store.find(Bar1, Bar1.foo == None).one()
        data = c.getvalue()
        self.assertNotIn("bar.foo_id IS NULL", data)

Unfortunately it's more complicated than I thought. I don't know how to fix that for now.

Changed in storm:
assignee: Thomas Herve (therve) → nobody
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

I may try to have a look at it.. can you please outline what you found and why it feels tricky?

Revision history for this message
Thomas Herve (therve) wrote :

Sure. The problem I think is that the Reference doesn't know if it's called on the original class or the alias. _local_key is not updated, and thus the relation built has no idea that the class is not the original class. There may be something wrong with _find_descriptor_class too, because it can't return the alias class as it browses the MRO. I *think* if Relation._build_relation is fixed so that self._local_key.table ends up pointing at the alias class, it should work.

summary: - set() ignores ClassAlias, updates wrong rows
+ ClassAlias Reference is compiled to wrong sql
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Jeroen,

You have probably already found this work around, but I will go ahead and document it here in the bug.
With this table structure:
        class Other(object):
            __storm_table__ = "other_table"
            prop1 = Property("column1", primary=True)
        class Class(object):
            __storm_table__ = "table"
            prop1 = Property("column1", primary=True)
            other_ref_id = Property("column1_ref_id")
            other_ref = Reference(other_ref_id, Other.prop1)
        ClassAlias = ClassAlias(self.Class, "alias")

store.find(ClassAlias.prop1, ClassAlias.other_ref == 1)
will generate WHERE "table".column1_ref_id = 1;
But, store.find(ClassAlias.prop1, ClassAlias.other_ref_id == 1)
will generate WHERE alias.column1_ref_id = 1;

Barry Warsaw (barry)
Changed in storm:
milestone: 0.19 → 0.20
Changed in storm:
milestone: 0.20 → 0.21
Colin Watson (cjwatson)
Changed in storm:
milestone: 0.21 → 0.22
Colin Watson (cjwatson)
Changed in storm:
milestone: 0.22 → 0.23
Colin Watson (cjwatson)
Changed in storm:
milestone: 0.23 → none
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.