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

Jsonb support #94

Closed
MSusik opened this issue Oct 23, 2014 · 14 comments
Closed

Jsonb support #94

MSusik opened this issue Oct 23, 2014 · 14 comments

Comments

@MSusik
Copy link

MSusik commented Oct 23, 2014

Hello!

Lately PostgreSQL introduced new field type: jsonb. It is designed to allow querying specific fields inside json dictionaries. http://www.postgresql.org/docs/9.4/static/functions-json.html . It is a significant performance improvement over the old implementation

The support for the field is already implemented in SQLAlchemy. Shouldn't sqlalchemy-utils support it as a part of JSONType?

JSONType could take an optional argument which would define which field type is requested if the Postgres is being used.

@MSusik
Copy link
Author

MSusik commented Feb 2, 2015

As zzzeek/sqlalchemy#145 has been merged, sth can be done in order to solve this issue. If you want, I can create a PR, just tell me how would you like to have it solved.

@mehcode
Copy link
Collaborator

mehcode commented Feb 2, 2015

For reference: http://stackoverflow.com/questions/22654170/explanation-of-jsonb-introduced-by-postgresql


Unless you're hand-writing SQL I see no advantage to jsonb in the context of sqlalchemy. I'm not sure an impl keyword argument makes sense from an api perspective.

Unless @kvesteri has a better idea, I'd recommend (in your own codebase):

class JSONBType(JSONType):
    def load_dialect_impl(self, dialect):
        return JSONB()

However if you could write queries to keys inside a jsonb type with just sqlalchemy (and no hand-written SQL) then I would be of a completely different mindset and probably recommend a queryable parameter to the JSONType.

@MSusik
Copy link
Author

MSusik commented Feb 3, 2015

However if you could write queries to keys inside a jsonb type with just sqlalchemy (and no hand-written SQL) then I would be of a completely different mindset and probably recommend a queryable parameter to the JSONType.

Please check zzzeek/sqlalchemy#101 . The operators have been already implemented in SQLAlchemy. The contains operator still requires some raw syntax, but it is more abstract then writing directly SQL syntax.

@mehcode
Copy link
Collaborator

mehcode commented Feb 3, 2015

Hmm.. Well, then. Thank you for pointing that out to me.


I'd recommend something describing what using jsonb allows you to do as an argument to toggle between json and jsonb then. Perhaps queryable.

@MSusik
Copy link
Author

MSusik commented Feb 4, 2015

Hmm.. Well, then. Thank you for pointing that out to me.

Your welcome. It was easy to overlook, I believe this feature wasn't advertised at all.

I'm not sure if queryable describes well what is the merit of the new implementation. I'd suggest rather simply using binary. Anyway, the final decision obviously belongs to the authors of the library.

@mehcode
Copy link
Collaborator

mehcode commented Feb 5, 2015

Anyway, the final decision obviously belongs to the authors of the library.

And one of them is me. 🍔


Whatever we do, this, makes me want to make jsonb the default.

After taking some time to read that document from postgresql.org I agree with binary. I would like to make it default though because of the many advantages; however, that wouldn't be advisable in a library scenario so we'll need to keep it set to False by default. Remember that we need a ConfigurationError or similar to be raised for any other dialects with binary set to True.

@MSusik
Copy link
Author

MSusik commented Feb 5, 2015

What about a quiet fallback? For example JSONType returns text field is the dialect is not postgres. The same thing can be implemented for JSONB case.

@mehcode
Copy link
Collaborator

mehcode commented Feb 5, 2015

It wouldn't make sense to fallback to text when we just passed in binary=True. Perhaps an optimized BSON encoding for other dialects would be an appropriate fallback but we can add that in time.

@MSusik
Copy link
Author

MSusik commented Feb 5, 2015

I just have one doubt. When sb uses this library and types implemented here, he usually has a software which is not database engine dependant. When such developer has an SQLAlchemy model:

class MyModel(declarative_base()):

    my_column = Column(JSON(binary=True), primary_key=True)

It will result in weird, exploding behaviour on engines different than postgres. To be honest, I thought that that was the reason why JSON type had a fallback.

@mehcode
Copy link
Collaborator

mehcode commented Feb 6, 2015

I completely agree with you there which is why I'm suggesting the BSON fallback. I just don't think its needed for a first iteration of the feature. Just throw a ConfigurationError or similar for now. Unless you want to tackle the fallback as well.

@kvesteri
Copy link
Owner

kvesteri commented Feb 6, 2015

Wouldn't it be better to:

  1. Have JSONType accept impl attribute by which you can override the default implementation.
  2. Have JSONB as the default type for PostgreSQL (IF it exists). This breaks BC but I think it makes sense.

@MSusik
Copy link
Author

MSusik commented Feb 6, 2015

Continuing discussion in #125

@kvesteri
Copy link
Owner

SQLAlchemy will have native JSON type as of 1.1. I'll deprecate and remove JSONType from SA-Utils at some point.

@python273
Copy link

The problem with native JSON type, it doesn't have fallback to text

CompileError: (in table '...', column '...'): Compiler <sqlalchemy.dialects.sqlite.base.SQLiteTypeCompiler object at 0x10a6f9f10> can't render element of type <class 'sqlalchemy.sql.sqltypes.JSON'>

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

No branches or pull requests

4 participants