Skip to content

INTPYTHON-348 Update raw_query tests for raw_mql #10

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

Closed
wants to merge 28 commits into from
Closed

INTPYTHON-348 Update raw_query tests for raw_mql #10

wants to merge 28 commits into from

Conversation

aclark4life
Copy link
Collaborator

@aclark4life aclark4life commented Oct 28, 2024

@aclark4life aclark4life requested a review from timgraham October 28, 2024 17:56
github-actions[bot]

This comment was marked as off-topic.

@timgraham
Copy link
Collaborator

Looks to be on the right track.

@aclark4life aclark4life marked this pull request as draft October 28, 2024 19:37
@timgraham timgraham force-pushed the mongodb-5.0.x branch 3 times, most recently from ba72390 to 7fcf5f0 Compare November 1, 2024 20:56


class Author(models.Model):
id = ObjectIdAutoField(primary_key=True)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to add an explicit primary key to all these methods. Put DEFAULT_AUTO_FIELD = "django_mongodb.fields.ObjectIdAutoField" in your test settings file.

@timgraham timgraham force-pushed the mongodb-5.0.x branch 2 times, most recently from 8a3dabc to afbecef Compare November 6, 2024 14:07
@@ -247,46 +248,46 @@ def test_query_representation(self):
"""
Test representation of raw query with parameters
"""
query = "SELECT * FROM raw_query_author WHERE last_name = %(last)s"
qset = Author.objects.raw(query, {"last": "foo"})
query = [{"$match": {"last_name": "%(last)s" % {"last": "foo"}}}]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test seems useless if there is no need for a params argument to raw_mql(). Using string interpolation to construct the query is not adding any value, so you might just skip test like these if they aren't applicable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand why that would be needed with MQL or SQL. There is another test that includes params kwarg (are we planning to support this?) This one seems to be testing string interpolation for no reason that I understand …

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Query parameters are passed separately in SQL-land to protect against SQL injection attacks.

@aclark4life
Copy link
Collaborator Author

Superseded by #12

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.

3 participants