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

Make CIText inherit types.Concatenable #6

Merged
merged 2 commits into from
Jan 8, 2015
Merged

Make CIText inherit types.Concatenable #6

merged 2 commits into from
Jan 8, 2015

Conversation

brmzkw
Copy link
Contributor

@brmzkw brmzkw commented Jan 7, 2015

This code:

>> from sqlalchemy import Column
>> from citext import CIText
>> print Column(CIText, name="field") + "whatever"

Without the inheritance, generates the invalid syntax:

field + :field_1

With the inheritance, generates a valid, working syntax:

field || :field_1

It is also problematic if you need to use "startswith":

>>> print Column(CIText, name="myfield").startswith("ABC")
myfield LIKE :myfield_1 + '%%'

Note you can still transform the filter to a literal as a workaround.

>>> print Column(CIText, name="myfield").startswith(literal("ABC"))
myfield LIKE :myfield_1 || '%%'

This code:

>> from sqlalchemy import Column
>> from citext import CIText
>> print Column(CIText, name="field") + "whatever"

Without the inheritance, generates the invalid syntax:

field + :field_1

With the inheritance, generates a valid, working syntax:

field || :field_1
@mahmoudimus
Copy link
Owner

@brmzkw this is sweet! I'll merge this when I can but first I think it's time I write a test suite for this package. Want to start it off? If not, I can do it either today or tomorrow.

@brmzkw
Copy link
Contributor Author

brmzkw commented Jan 7, 2015

I'll try to write some tests tomorrow, and will create another pull request :)

@brmzkw
Copy link
Contributor Author

brmzkw commented Jan 8, 2015

Done

mahmoudimus added a commit that referenced this pull request Jan 8, 2015
Make CIText inherit types.Concatenable
@mahmoudimus mahmoudimus merged commit c4c49b2 into mahmoudimus:master Jan 8, 2015
@brmzkw
Copy link
Contributor Author

brmzkw commented Jan 8, 2015

Is it possible to push a release on pypi? :)
It would be cool to have a Changelog, too!

@mahmoudimus
Copy link
Owner

Yep, I'll push a release :)

@mahmoudimus
Copy link
Owner

Agree re: changelog, will get started on one. Want to create a follow up github issue?

@brmzkw
Copy link
Contributor Author

brmzkw commented Jan 8, 2015

#7

@mahmoudimus
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants