Pegase745 / sqlalchemy-datatables

SQLAlchemy integration of jQuery DataTables >= 1.10.x (Pyramid and Flask examples)
MIT License
159 stars 67 forks source link

Updated datatables.__init__.py to use column length in SQLAlchemy cast expression. #46

Closed obenshaindw closed 7 years ago

obenshaindw commented 8 years ago

Updated datatables.init.py to use column length in SQLAlchemy cast expressions, required for Oracle dialect where cast to VARCHAR2 requires a length. Resolves issue #31

Pegase745 commented 8 years ago

@obenshaindw could you possibly take a look on the failing tests please?

obenshaindw commented 8 years ago

@Pegase745 the Travis-CI build fails because I naively assumed the SQLAlchemy objects to have a length attribute, which for some types they do not. We're always casting to a String type in datatables.init.py, so for the oracle dialect we always need to provide a length to get valid DDL if we're casting to a string.

We could implement a try with except on AttributeError and assign an arbitrary size (i.e., maximum length for types that don't have a class with length attribute).

try:
    conditions.append(cast(
        get_attr(sqla_obj, column_name), String(get_attr(sqla_obj, column_name).type.length))
        .ilike('%%%s%%' % searchValue))

except AttributeError:
    conditions.append(cast(
        get_attr(sqla_obj, column_name), String(22))
        .ilike('%%%s%%' % searchValue))

What are your thoughts?

Pegase745 commented 8 years ago

isn't it cleaner to tweak this way only if we're using Oracle rather than generalizing it for every db ?

obenshaindw commented 8 years ago

Yes, I was thinking along the same lines. So we would now have an IF/ELSE block to test if using oracle dialect, and use the Try/Except if true, else we default to what you already have.

Pegase745 commented 8 years ago

totally agree, because honestly I have no idea how to test this quickly in order to deliver the fix

tdamsma commented 7 years ago

Is this still an issue after 1.0.0? Closing this until further notice