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

Spike extend compile to handle WhereNode. #29

Closed

Conversation

WaVEV
Copy link
Collaborator

@WaVEV WaVEV commented May 16, 2024

In this PR, I implemented a quick and dirty version of a local compiler. Note that I called it _compile to avoid overwriting the parent method (it should be overwritten, but to make a point, I decided not to).

As we can see, the idea is to take advantage of the Django objects and manage them as a tree. This approach would be easier and more understandable.

Note that each Node has a function as_mql, but we cannot add this method to the Node class. Therefore, I suggest using it as an abstract datatype. (It is a bit unconventional, given that we are using OOD, but it is better than the alternative.)

This implementation passes the same tests as the original one in the basic suite (it is small, but just to prove a concept, I think it is okay).

@WaVEV WaVEV changed the title pike extend compile to handle WhereNode. Spike extend compile to handle WhereNode. May 16, 2024
@WaVEV WaVEV requested review from timgraham and Jibola May 16, 2024 03:57
@timgraham
Copy link
Collaborator

Instead of a long list of isinstance checks, I think we can patch as_mql onto each lookup, etc. as in https://github.com/Snowflake-Labs/django-snowflake/blob/e2962b369e16a38ef718008282578af003d06684/django_snowflake/lookups.py.

Note that I called it _compile to avoid overwriting the parent method (it should be overwritten, but to make a point, I decided not to).

Overriding compile() would break the ability to use MongoDB alongside a SQL database.

@WaVEV
Copy link
Collaborator Author

WaVEV commented May 16, 2024 via email

@WaVEV
Copy link
Collaborator Author

WaVEV commented May 16, 2024

Instead of a long list of isinstance checks, I think we can patch as_mql onto each lookup, etc. as in https://github.com/Snowflake-Labs/django-snowflake/blob/e2962b369e16a38ef718008282578af003d06684/django_snowflake/lookups.py.

Note that I called it _compile to avoid overwriting the parent method (it should be overwritten, but to make a point, I decided not to).

Overriding compile() would break the ability to use MongoDB alongside a SQL database.

Instead of a long list of isinstance checks, I think we can patch as_mql onto each lookup, etc. as in https://github.com/Snowflake-Labs/django-snowflake/blob/e2962b369e16a38ef718008282578af003d06684/django_snowflake/lookups.py.

Note that I called it _compile to avoid overwriting the parent method (it should be overwritten, but to make a point, I decided not to).

Overriding compile() would break the ability to use MongoDB alongside a SQL database.

Any idea where will we put all these stuff? or just create a large file with all lookups?

@timgraham
Copy link
Collaborator

I imagine at least a few files: lookups.py, functions.py, nodes.py.

@timgraham
Copy link
Collaborator

Superseded by #33.

@timgraham timgraham closed this May 20, 2024
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.

None yet

2 participants