Coercing values during chaining #33

Closed
uranusjr opened this Issue Dec 17, 2013 · 1 comment

Comments

Projects
None yet
2 participants
@uranusjr
Contributor

uranusjr commented Dec 17, 2013

This works

>>> print select('person', where={'id in': ('foo', 'bar')})
SELECT * FROM "person" WHERE "id" IN ('foo', 'bar')

But this doesn't

>>> print select('person', where={'id in': (1, 2, 3)})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "mosql/util.py", line 819, in __call__
    return self.stringify(*positional_values, **clause_args)
  File "mosql/util.py", line 815, in stringify
    return self.format(clause_args)
  File "mosql/util.py", line 801, in format
    return self.statement.format(clause_args)
  File "mosql/util.py", line 752, in format
    pieces.append(clause.format(arg))
  File "mosql/util.py", line 679, in format
    x = formatter(x)
  File "mosql/util.py", line 359, in joiner_wrapper
    return f(x)
  File "mosql/util.py", line 519, in build_where
    return _build_condition(x, identifier, value)
  File "mosql/util.py", line 464, in _build_condition
    v = paren(concat_by_comma(v))
  File "mosql/util.py", line 359, in joiner_wrapper
    return f(x)
  File "mosql/util.py", line 383, in concat_by_comma
    return ', '.join(i)
TypeError: sequence item 0: expected string, int found

because join works only when the sequence contains str (and unicode) instances.

It seems pretty reasonable to coerce before concatenating values:

@joiner
def concat_by_comma(i):
    '''A joiner function which concats the iterable by ``,`` (comma).'''
    return ', '.join(str(v) for v in i)

concat_by_space, concat_by_or and concat_by_and are all similar, I believe. Maybe extract the logic into a decorator?

There may be some encoding issues, too. I would rather use unicode instead of str, but you will need to add some version detection if so.

@moskytw

This comment has been minimized.

Show comment
Hide comment
@moskytw

moskytw Dec 17, 2013

Owner

It is a regression. I'm checking.

Owner

moskytw commented Dec 17, 2013

It is a regression. I'm checking.

@moskytw moskytw closed this in 20ba8fc Dec 17, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment