gorilla-co / odata-query Goto Github PK

Hi there,

Thank you for making this lovely library.

I currently use the AstToSqlAlchemyClauseVisitor which works well with SQLAlchemy ORM, but my application is async so I use sqlalchemy core everywhere apart from where OData is involved. My database does not support async driver so sqlalchemy 2.0 async support won't actually bring an difference to me. With sqlalchemy core I build the query statements and handle all aspects around execution of the statement myself with loop.run_in_executor, which again does not work well with ORM.

Would it possible to add support for sqlalchemy core?

Hi,
I am starting using odata-query for a project (Thanks!) and I noticed that the links in the docs are broken Is it OK if I make a PR with the updated links?
So far, I have noticed just a couple of places:

• PyPi page "contributing guidelines"
• readthedocs "Reporting a bug" link to issues

hi,

odata-query works great for me (using django). except it's not clear to me how to deal with (ugly!) odata attributes. for example I'd like to map Attributes/IntegerAttributes/any(Name eq 'OrbitNumber'??)/{Name, Value} to a literal "OrbitNumber" and a certain db field, respectively. do you have any suggestions on how to proceed..? (an example in the docs would also be great :P)

I'm using the raw lexer (no provided mongodb or other integrations).

Let's look at this example:

from odata_query.grammar import ODataLexer, ODataParser
lexer = ODataLexer()
parser = ODataParser()
odata_filter = parser.parse(lexer.tokenize("foo eq 15"))

I would expect that odata_filter contains a numerical value for foo, instead I get:

Compare(comparator=Eq(), left=Identifier(name='foo'), right=Integer(val='15'))

Hi,

Thanks very much for the helpful library. It got me from zero-to-one with OData very quickly which I really appreciated.

However, I was a bit concerned about some of the warnings I got after using it for parsing OData to core SQLAlchemy constructs, as it seems to override behaviour in a manner that I don't think is correct in all cases. I will explain more below.

For reference the warning that I had a concern over was:

SAWarning: Class lower will not make use of SQL compilation caching as it does not set the 'inherit_cache' attribute to `True`.  This can have significant performance implications including some performance degradations in comparison to prior SQLAlchemy versions.  Set this attribute to True if this object can make use of the cache key generated by the superclass.  Alternatively, this attribute may be set to False which will disable this warning. (Background on this error at: https://sqlalche.me/e/20/cprf)
    conn.execute(qry)

This confused me as after looking into the linked article, it implied that I would be defining the class representing the lower SQL function somewhere, whereas in fact I wasn't. I was however using the function against my PostgresQL database, where I make heavy use of range types, especially datetime ranges.

I then found that in functions_ext you define a lower function handler. Further documentation and code experimentation confirms that this causes it to be hooked into the attribute sqlalchemy.func.lower, which is what I am using to access the lower function I want to use in my non-OData relevant code. Whilst this doesn't currently cause a bug for me, it does scare me slightly as it feels like not the most future-proof solution.

Reading up on the documentation of GenericFunction - the class you subclass to create your lower it says "The primary use case for defining a .GenericFunction class is so that a function of a particular name may be given a fixed return type." But your function clashes with the base function in this sense. In the context that I am using it, lower will return a DateTime.

So I was wondering whether we could:

1. Use the package attribute such that your definitions of these functions wouldn't clash with the sqlalchemy.func.xxx versions as yours would get hooked as sqlalchemy.func.odata.lower leaving sqlalchemy.func.lower for other users. This shouldn't affect the odata code as it just uses the classes representing the functions directly
2. Also set the inheritCache attribute so that even code that uses the OData versions of the functions avoids this warning, and also allows sqlalchemy to cache compilation results of the compiling the functions - which I believe should be safe

I will raise a PR with these suggestions and link it to this ticket.

Thanks,

Byron

Problem Description:
When trying to filter for a column on a join relationship on a legacy sqlalchemy Query object, apply_odata_query does not recognize existing join relationships ending up in a SQL statement that falsely joins the same relationship multiple times:

sqlalchemy.exc.ProgrammingError: (psycopg2.errors.DuplicateAlias) table name "my_relationship" specified more than once

Example code to reproduce:

from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()

class MyRelationship(Base):
    __tablename__ = "my_relationship"
    relationship_id = Column(UUID(as_uuid=True), default=uuid.uuid4, primary_key=True, index=True, nullable=False)
    some_bool = Column(Boolean, nullable=False)
    some_other_bool = Column(Boolean, nullable=False)

class MyModel(Base):
    __tablename__ = "my_model"
    model_id = Column(UUID(as_uuid=True), default=uuid.uuid4, primary_key=True, index=True, nullable=False)
    relationship_id = Column(
        UUID(as_uuid=True),
        ForeignKey(MyRelationship.relationship_id),
        nullable=False,
    )
    my_relationship = relationship(MyRelationship)

from odata_query.sqlalchemy import apply_odata_query

orm_query = session.query(MyModel)
    .select_from(MyModel)
    .join(MyRelationship, MyModel.relationship_id == MyRelationship.relationship_id)
    .filter(MyRelationship.some_bool == false())  # This is a SQLAlchemy Query object with existing join relationship
odata_query = "my_relationship/some_other_bool eq true"  # we want to add a filter on the same relationship

query = apply_odata_query(orm_query, odata_query)
results = query.all()  # this breaks

Is there support for parsing a query, rewriting values in the AST, and generating an OData query string again?

While packaging your library for Fedora (see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2031000), I've noticed that Faker and moto from testing extra are not used. Please consider removing them from the requirements.

Steps to reproduce:

• Try to translate contains(tolower(name), tolower('A')) to SQL using the AstToSqlVisitor
  -> error:

 AttributeError("'Call' object has no attribute 'val'")

Bug spotted:

• AstToSqlVisitor._to_pattern does not take into account that the arg could be an ast.Call (and assumes a literal in the else branch)

Proposed solution:

Change

  if isinstance(arg, ast.Identifier):

Into

  if isinstance(arg, ast.Identifier | ast.Call):

SQLAlchemy offers great support for ENUM fields, mapping them from either a list of options or a Python Enum.
OData-Query doesn't utilize this feature well enough, and currently just crashes if you try to use a non-existing ENUM member.

Right now, it's not possible to check the code which uses the library:

some_file:15: error: Skipping analyzing "odata_query": module is installed, but missing library stubs or py.typed marker
some_file:15: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
some_file: error: Skipping analyzing "odata_query.grammar": module is installed, but missing library stubs or py.typed marker
some_file: error: Skipping analyzing "odata_query.exceptions": module is installed, but missing library stubs or py.typed marker
some_file: error: Skipping analyzing "odata_query.typing": module is installed, but missing library stubs or py.typed marker
some_file: error: Skipping analyzing "odata_query.visitor": module is installed, but missing library stubs or py.typed marker

It would be great to get this ability.

PS: I've implemented my custom visitor, but I'm not sure if it's worth sharing; it's unique to our requirements.

I am Fedora packager of python-odata-query.

Up until v0.8.1 releases were marked with tags. Last two releases are not marked with tags. This prevents from running https://packit.dev/ to automate releases: it picks up v0.8.1 as latest release.

Could you please introduce release tagging back?

Hello again!

May I suggest you add your nice contributor's guide to the read the docs? This can be done without duplicating the documentation's source:

1. Convert (manually) CONTRIBUTING.md to RestructuredText as CONTRIBUTING.rst (Github will still render it just fine as HTML).

2. Add a contributing.rst file to your doc and add it somewhere to the doctree with the following content:

.. _contributing:

.. include:: ../CONTRIBUTING.rst

3. There is no third step!

Problem Description:
Till 0.6.0 (inclusive) we could pass a sqlalchemy.orm.Query object to odata_query.sqlalchemy.apply_odata_query and it worked. With 0.7.0 this changed and apply_odata_query breaks with the following exception:

AttributeError: 'Query' object has no attribute 'columns_clause_froms'

Example code to reproduce:

from odata_query.sqlalchemy import apply_odata_query

orm_query = session.query(MyModel)  # This is a SQLAlchemy Query object
odata_query = "name eq 'test'"  # This will usually come from a query string parameter.

query = apply_odata_query(orm_query, odata_query) # this breaks
results = query.all()

Notes:
The title deliberately contains the word "implicit" as apply_odata_query's query param has type hint sqlalchemy.sql.expression.ClauseElement and sqlalchemy.orm.Query does not inherit from it.

So this issue could also be seen as the question whether it is intended to support SQLAlchemy's Query-API in general as it is kind of deprecated but I guess a lot of folks out there are still using it.

Hi!
Thanks for your great work!
We're currently working on adding an odata API to our Django based application. I.e. we want to access data using Excel.
During our research we found this project.
We currently understand, that your project can be used to translate odata search queries for the Django ORM.
Now we have the following question:
How do you return the results to the client? Are you using the Django REST framework for the communication?
Do you by chance have an example for this that you could share?
Thanks!

• Mark and Freddy

Hello! Thank you for this wonderful contribution to the community! We have been using odata-query with our Django / DRF project to provide OData representations of our 1st-party objects in Salesforce via External Objects, but as we are expanding the usage of OData, we have encountered some difficulty with the parser. For example, when exposing one of our Django models via a filterable OData API for Salesforce, we encountered this error:

TypeException

Cannot apply 'Eq' to 'Call(func=Identifier(name='contains'), args=[Identifier(name='category'), String(val='danielle hutchens')])'

From odata_query/django/django_q.py in visit_Compare at line 194:

node: Compare(comparator=Eq(), left=Call(func=Identifier(name='contains'), args=[Identifier(name='category'), String(val='danielle hutchens')]), right=Boolean(val='true'))
. . .

From odata_query/django/shorthand.py in apply_odata_query at line 24:

odata_query:

"contains(category,'danielle hutchens') eq true or contains(email,'danielle hutchens') eq true or contains(filename,'danielle hutchens') eq true or contains(file_id,'danielle hutchens') eq true or contains(path,'danielle hutchens') eq true or contains(revision,'danielle hutchens') eq true or contains(content_hash,'danielle hutchens') eq true or contains(subcategory,'danielle hutchens') eq true or contains(hyker__sfaccountid,'danielle hutchens') eq true or contains(hyker__sfopportunityid,'danielle hutchen...

Can you confirm whether odata-query supports these kinds of comparisons? Are we (or Salesforce External Objects) doing something wrong?

The AST to SQL transformer still contains some Athena-specific code. Ideally this library contains a pure, generic SQL transformer, and an Athena/Presto dialect can be maintained in a seperate library/extension.

Hi,
I faced a problem that very likely has a simple solution that I do not see. I am using release v0.5.2 to modify sqlalchemy queries.

I tried to use the module to filter a query that selects more than one ORM entity. I faced the problem that AstToSqlAlchemyClauseVisitor expects a single type on entity, but the query gets and filter fields from ORM entities of different types. I guess there is a way of doing this and I do not realize which is it.

As an example, using the models from models.py used in testing, I did not find a proper way to freely filter the following query:

stmt = select(Author, Comment).join(Comment)

Using a similar approach as in shorthand.apply_odata_query I can apply filters on fields of any of the two entities (using AliasRewriter if needed), but since AstToSqlAlchemyClauseVisitor accepts a single entity I do not see a way to filter on both.

Please let me know if there is a proper way to manage this kind of queries. Meanwhile, I made an ugly hack to circumvent this, that it probably breaks many things.

Basic support for SQLAlchemy Core was added in v0.7.0, but this does not include relationship traversal (e.g. author/blogposts) or collection lambda blogposts/any(b: b/title eq 'test') functionality.

To implement this we need a way to find the related table and its target column. In the ORM visitor, this is easy because mapped models usually include a relationship attribute with all necessary details. Core Table objects seem to lack such a linking property.

All integration tests for this are already present, but currently marked xfail.

The OData ABNF doesn't seem to distinguish between a single item list expression and a simple parenthesized literal (e.g. ('draft')). This currently breaks some expressions in this library.
Example failing case:
status in ('draft')

Ref: https://docs.oasis-open.org/odata/odata/v4.01/cs01/abnf/odata-abnf-construction-rules.txt
Grammar rules:

• parenExpr = OPEN BWS commonExpr BWS CLOSE
• listExpr = OPEN BWS commonExpr BWS *( COMMA BWS commonExpr BWS ) CLOSE

Currently, the workaround in odata-query is by forcing a listExpr through a trailing comma: ('draft',). It would be nicer if we could comply with the spec 100%.

Hello,

thank you for this phantastic library, it helps me a lot!

However, I experienced that filtering by uuid columns is not working. Here a minimal example:

import uuid

import sqlalchemy as sa
from sqlalchemy.orm import declarative_base
from sqlalchemy.orm.session import Session
from sqlalchemy_utils import UUIDType
from odata_query.sqlalchemy import apply_odata_query

Base = declarative_base()

class MyTable(Base):
    __tablename__ = "mytable"
    id = sa.Column(UUIDType(binary=False), primary_key=True, default=uuid.uuid4, nullable=False)

DATABASE_URL = "sqlite:///test.db"
engine = sa.create_engine(DATABASE_URL)

Base.metadata.drop_all(bind=engine)
Base.metadata.create_all(bind=engine)

with Session(engine) as db:
    t = MyTable()
    db.add(t)
    db.commit()

    q = db.query(MyTable)
    q = apply_odata_query(q, f"id eq {t.id}")
    print(q.all())

Output: []
The UUIDType(binary=False) generates CHAR(32).

I found out that the following change to ./odata-query/sqlalchemy/sqlalchemy_clause.py makes it work (is the missing 'py_' a typo?):

def visit_GUID(self, node: ast.GUID) -> BindParameter:
        ":meta private:"
        # return literal(node.val)    # Old version
        return literal(node.py_val)  # New version

Output: [<__main__.MyTable object at 0x000001F8295C3DC0>]

Also it seems like the tests in ./odata-query/tests/integration/sqlalchemy/test_querying.py do imply filtering with uuids but only against non-matching:

(Author, "id eq a7af27e6-f5a0-11e9-9649-0a252986adba", 0),
(Author, "id in (a7af27e6-f5a0-11e9-9649-0a252986adba, 800c56e4-354d-11eb-be38-3af9d323e83c)", 0),

Would be nice if this could be fixed.

*Edit:
Just found out that my proposed change leads to exceptions when using the in clause, that are otherwise not present:

...
q = apply_odata_query(q, f"id in ({t.id}, 800c56e4-354d-11eb-be38-3af9d323e83c)")
...

Outputs:

sqlalchemy.exc.InterfaceError: (sqlite3.InterfaceError) Error binding parameter 0 - probably unsupported type.
[SQL: SELECT mytable.id AS mytable_id
FROM mytable
WHERE mytable.id IN (?, ?)]
[parameters: (UUID('0fc434f4-d10d-4725-8d4f-4e00972dea15'), UUID('800c56e4-354d-11eb-be38-3af9d323e83c'))]
(Background on this error at: https://sqlalche.me/e/14/rvf5)

hi!

we are trying to make use of geospatial queries, as described here:

https://www.odata.org/blog/geospatial-properties/

it seems though that the "geography'..'" syntax is not yet accepted, resulting in:

odata_query.exceptions.ParsingException: Failed to parse at: Token(type='STRING', value=String(val=''), lineno=1, index=38)

working around this, calling geo.intersects() in a query results in the following:

odata_query.exceptions.UnknownFunctionException: Unknown function: 'intersects'

(in grammar.py it compares 'intersects' with 'geo.intersects', so no match..?)

working around this as well, it appears there is no conversion to django geo calls as of yet (no djangofunc_intersects?).

is it correct to say that geospatial queries are not really supported at the moment? :)

URL: /api/inventory/access-switches/?$select=description,uuid
odata_select = request.GET.get('$select')
queryset = apply_odata_query(queryset, odata_select)

Raises an issue:
odata_query.exceptions.ParsingException: Failed to parse at: Token(type=':', value=':', lineno=1, index=4)"

But same works for $filter as below
URL: /api/inventory/access-switches/?$filter=(contains(tolower(description), 'httrf'))

odata_filter = request.GET.get('$filter')
queryset = apply_odata_query(queryset, odata_filter)

Hi Oliver,

Are there any plans to add support for aggregations? I'm basically after simple support of cases such as

/api/odata/Orders?$apply=aggregate($count as OrderCount)

Ideally this translates as a sqlalchemy sa.select([sa.func.count(Orders).label('OrderCount')]).

I am happy to help adding support for this feature, but it requires a change to the grammer and I will need guidenace on best way to implement.

Many thanks
Meitham

Hello there! This looks nice, thanks for opensourcing it!

I see you provide visitors for ORMs. Would you accept a contribution for a MongoDB visitor?

Have a nice day!