Coalesce doesn't infer result type, breaks with "can't adapt."

Bug #420364 reported by Jeroen T. Vermeulen
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Storm
Confirmed
Medium
Unassigned

Bug Description

I'm trying to write a query condition Coalesce(x, y, z) == foo. The resulting query bombs out of psycopg with the error "can't adapt." Here x, y, and z are DBEnum values.

Edited notes from IRC:

<jamesh> The DBItem instance would be the culprit.
It is not getting translated.

<jtv> Yup. Wonder why it fails only when I use Coalesce.

<jamesh> Well, the Column.__eq__() method casts the other argument to the same Variable type that the column uses
and the Variable type for DBEnumerations knows to convert DBItems to integers before passing them down to the database.
Storm doesn't know what the Variable type for Coalesce()'s return value
so isn't doing the conversion itself.

<jtv> I thought it'd be able to infer that from its inputs... Is there an explicit cast I can apply to make this go away?

<jamesh> You could do Coalesce(x, y) == Enum.foo.value

[That worked].

<jamesh> you are right that it should be possible
but at the moment there isn't any special compilation code to do so -- it is treating Coalesce like a generic function call
and the generic function call compilation process can't determine the return type without knowing something about the function
(we could definitely improve this though)

<jamesh> it should be pretty easy to add return type awareness to things like Coalesce
and a variable_factory @property that returns args[0].variable_factory

Tags: tech-debt
Revision history for this message
James Henstridge (jamesh) wrote :

So my suggestion here was to do something like:

    class Coalesce(NamedFunc):
        ...
        @property
        def variable_factory(self):
            return self.args[0].variable_factory

The Comparable.__eq__() method tries to get self.variable_factory and cast the "other" value to that variable type, so the above would propagate the type info from the function arguments to the function return.

The same could be done for some other common functions (e.g. set Count.variable_factory to IntVariable).

Curtis Hovey (sinzui)
tags: added: tech-debt
Revision history for this message
James Henstridge (jamesh) wrote :

Tim Penhey reported a similar issue related to Max() on IRC:

  09:52 <thumper> jamesh: we have UtcDateTimeCol fields for our storm classes
  09:52 <jamesh> yep. I think I wrote the initial version of that ...
  09:52 <thumper> jamesh: which need a timezone aware datetime
  09:52 <thumper> however in a query I'm using
  09:52 <thumper> I have Max(Table.date_col) is returning a timezone unaware date time
  09:52 <thumper> why?
  09:53 <thumper> in the db, the column is "timestamp without timezone)
  09:53 <jamesh> probably because Max() doesn't know that its return value should be the same type as its argument
  09:53 <thumper> it is making my test a PITA
  09:54 <jamesh> so is just returning what psycopg gives it

Changed in storm:
importance: Undecided → Medium
status: New → Confirmed
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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