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

Fix issue with unicode json path keys #221

Closed
wants to merge 1 commit into from
Closed

Conversation

@jjjacksn
Copy link

@jjjacksn jjjacksn commented Jan 14, 2017

This fixes an issue with unicode keys in JSON path. I observed the bug on Python 2.7.11.

The previous implementation of SQLBuilder.eval_json_path() did not consider unicode objects valid keys.

Here is some sample code which should reproduce the issue.

from pony.orm import *

db = Database()

class MyEntity(db.Entity):
    id = PrimaryKey(int, auto=True)
    json = Optional(Json

...

def query_json():
    with db_session:
        my_entities = select(e for e in MyEntity if e.json[u'key'] == 'value')[:]

    return my_entities

I ran into this issue because I am developing a Python 2/3 codebase using the unicode literals feature from future. The workaround I am using for now is as follows:

from __future__ import unicode_literals

import sys
PY2 = sys.version_info[0] == 2

...

def _json_key(key):
    return bytes(key) if PY2 else key

def query_json():
    with db_session:
        my_entities = select(e for e in MyEntity if e.json[_json_key('key')] == 'value')[:]

    return my_entities
@kozlovsky kozlovsky closed this in 10e45c5 Mar 21, 2017
@kozlovsky
Copy link
Member

@kozlovsky kozlovsky commented Mar 21, 2017

Thanks for reporting! I fixed it in a slightly different way, and covered it with a test

@kozlovsky kozlovsky self-assigned this Mar 21, 2017
@kozlovsky kozlovsky added the bug label Mar 21, 2017
@kozlovsky kozlovsky added this to the 0.7.2 milestone Mar 21, 2017
@jjjacksn
Copy link
Author

@jjjacksn jjjacksn commented Mar 21, 2017

Great thanks!

kozlovsky added a commit that referenced this pull request Jul 17, 2017
# New features

* All arguments of db.bind() can be specified as keyword arguments. Previously Pony required the first positional argument which specified the database provider. Now you can pass all the database parameters using the dict: db.bind(**db_params). See https://docs.ponyorm.com/api_reference.html#Database.bind
* The `optimistic` attribute option is added https://docs.ponyorm.com/api_reference.html#cmdoption-arg-optimistic

# Bugfixes

* Fixes #219: when a database driver raises an error, sometimes this error was masked by the 'RollbackException: InterfaceError: connection already closed' exception. This happened because on error, Pony tried to rollback transaction, but the connection to the database was already closed and it masked the initial error. Now Pony displays the original error which helps to understand the cause of the problem.
* Fixes #276: Memory leak
* Fixes the __all__ declaration. Previously IDEs, such as PyCharm, could not understand what is going to be imported by 'from pony.orm import *'. Now it works fine.
* Fixes #232: negate check for numeric expressions now checks if value is zero or NULL
* Fixes #238, fixes #133: raise TransactionIntegrityError exception instead of AssertionError if obj.collection.create(**kwargs) creates a duplicate object
* Fixes #221: issue with unicode json path keys
* Fixes bug when discriminator column is used as a part of a primary key
* Handle situation when SQLite blob column contains non-binary value
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants