Skip to content

DOCSP-48184: Rework limitations and upcoming #27

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

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

norareidy
Copy link
Collaborator

@norareidy norareidy commented Mar 14, 2025

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-48184

Staging Links

  • index
  • limitations-upcoming
  • Self-Review Checklist

    • Is this free of any warnings or errors in the RST?
    • Did you run a spell-check?
    • Did you run a grammar-check?
    • Are all the links working?
    • Are the facets and meta keywords accurate?

    Copy link

    netlify bot commented Mar 14, 2025

    Deploy Preview for docs-django ready!

    Name Link
    🔨 Latest commit bb9ba2e
    🔍 Latest deploy log https://app.netlify.com/sites/docs-django/deploys/67f82bbc55091100082b0d44
    😎 Deploy Preview https://deploy-preview-27--docs-django.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    @norareidy norareidy marked this pull request as ready for review March 17, 2025 14:44
    not supported. Instead, use ``ObjectIdField``.
    - *Unsupported*.

    * - ``BSONRegExp``
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    This is not a field but a BSON data type.

    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    Moved to the Mongodb "Data Type Support" section


    Performance
    ~~~~~~~~~~~
    * - ``compilemessages``
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    It's strange to list all the management commands as many of them do not interact with the database.

    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    Which commands do you think are relevant to include?

    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    As in other sections, including only the unsupported commands is sufficient.

    Comment on lines 793 to 795
    * - `Django-filter <https://django-filter.readthedocs.io/en/stable/>`__
    - *Unsupported*.
    - ✓
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    As far as I know, it is supported because @aclark4life didn't identify any needed changes.

    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    Following up with him!

    Comment on lines 766 to 768
    * - Making migrations
    - ✓
    - ✓
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    It is strange to list "Making migrations, Recording schema history, Storing migration files". These are the basics of the migrations system, and we wouldn't have any support without them.

    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'll remove

    Comment on lines +333 to +334
    - *Full Support Not Planned*. However, we plan to address the
    ``Q`` object and ``None`` key filtering limitations in a post-GA release.
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    I'm not sure they can be. \cc @WaVEV

    Copy link

    Choose a reason for hiding this comment

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

    It is hard to implement; all operators must consider the null factor and handle it separately, given the nature of null in SQL. Null values behave as either false or true in filtering. Sometimes a filter filters for when a condition is met, and other times for when it is not. As a result, having a condition C and its negation ~C can yield overlapping results between both queries. This behavior occurs in SQL but not in MongoDB.

    Comment on lines 371 to 373
    * - ``as_manager()``
    - ✓
    - ✓
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    This isn't a method that interacts with the database. Do we need to list every supported method? You didn't list every supported model field, so there seems to be a slight inconsistency.

    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    Since there's already a list of all supported model fields in the Models guide, I linked out to it instead of repeating them. But I agree that this table is pretty long; I can just list the unsupported/partially supported, then link to the Django QuerySet docs and say that we support all other methods not mentioned?

    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    Sounds good to me.

    Comment on lines 55 to 58
    * - Atlas Search and Atlas Vector Search indexes
    - *Partially Supported*. You cannot use the Django
    Indexes API to create these indexes, but you can use
    the PyMongo Driver by :ref:`exposing your MongoClient <django-client-operations>`.
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    By this definition (partial support = can do it in pymongo), I'd think nearly everything that's unsupported would be considered partially supported.

    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 changed to "Unsupported" but kept the information about exposing the MongoClient

    Comment on lines 367 to 369
    * - ``distinct()``
    - *Unsupported*.
    - *Unsupported*.
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    Correction: distinct() is supported.

    Copy link

    @rustagir rustagir left a comment

    Choose a reason for hiding this comment

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

    overall looks great, just left a comment to format differently for readability.


    - ``DurationField``
    .. list-table::

    Choose a reason for hiding this comment

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

    S: suggest adding a stub column to all tables to separate the feature name from the support columns

    @norareidy norareidy requested a review from R-shubham April 10, 2025 20:39
    Copy link
    Collaborator

    @R-shubham R-shubham left a comment

    Choose a reason for hiding this comment

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

    Can we follow the same template on the limitation page as Laravel?

    One of the things I noticed from users who read this page was - the way we had a big list of not supported feature..some user walked away with the impression that there is a lot that need to be done vs..a great deal of feature is supported & here is things that is in works, or has limitations.

    For example, I’m reading indexes section - may be we can say all index type is supported except these “3”. Kind of baking in a full picture. We may not need to go in details of every supported features and can use blanket statement like, all of Django's queryset function is known to be supported except a,b,c.

    @norareidy
    Copy link
    Collaborator Author

    Can we follow the same template on the limitation page as Laravel?

    One of the things I noticed from users who read this page was - the way we had a big list of not supported feature..some user walked away with the impression that there is a lot that need to be done vs..a great deal of feature is supported & here is things that is in works, or has limitations.

    For example, I’m reading indexes section - may be we can say all index type is supported except these “3”. Kind of baking in a full picture. We may not need to go in details of every supported features and can use blanket statement like, all of Django's queryset function is known to be supported except a,b,c.

    Hi Shubham! Can you clarify what you mean by same template - the same two-column table format? After the GA release the tables will have two columns (feature and availability), but for now I think it makes more sense to separate Public Preview and GA availability. See this thread for a discussion about that. Or do you mean the same subheaders? I reused the subheaders that applied (Query, Database, Migration support) but most of the Laravel sections are Laravel-specific (Eloquent Relationships, Mutator, Model Factory, etc). But let me know if there's something I'm missing from the Laravel page.

    And yes, I can rework each section to just include what we don’t support with a statement like “everything is supported except the following”, similar to what I have in Data Type, Field Type, etc.

    Copy link

    @rachel-mack rachel-mack left a comment

    Choose a reason for hiding this comment

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

    Adding this to close #31

    - *Unsupported*. The ``BigAutoField`` and ``SmallAutoField`` types are also
    not supported. Instead, use ``ObjectIdField``.
    - *Unsupported*.

    Choose a reason for hiding this comment

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

    Add this here in favour of #31

    Suggested change
    * - ``CompositePrimaryKey``
    - *Unsupported*.
    - *Unsupported*.

    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    Updates for Django 5.2 could be deferred until after this PR, but changes for Django 5.2 shouldn't be mixed in with this refactor. For one, mixing concerns like that will make backporting more difficult.

    Comment on lines +361 to +371
    * - ``bulk_update()``
    - *Unsupported*.
    - *Unsupported*.

    * - ``dates()``
    - *Unsupported*.
    - *Unsupported*.

    * - ``datetimes()``
    - *Unsupported*.
    - *Unsupported*.
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    bulk_update/dates/datetimes are all supported.

    Comment on lines +382 to +385
    * - ``JOIN`` operations
    - *Partially Supported*. You can use MongoDB's :manual:`$lookup </reference/operator/aggregation/lookup/>`
    pipeline stage to perform joins.
    - *Full Support Not Planned*.
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    The backend automatically uses $lookup. There is nothing for the user to do. I'm not sure why this is considered "partial support".

    Comment on lines +211 to +213
    * - Cached data storage
    - *Unsupported*.
    - ✓
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    Supported as per 39d9ffe.

    Comment on lines +514 to +516
    * - ``createcachetable``
    - *Unsupported*.
    - *Unsupported*.
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    Add note per 39d9ffe.

    Comment on lines +463 to +465
    - *Partially Supported*. The ``tzinfo`` parameter doesn't work
    properly because MongoDB converts the result back to UTC.
    - *Full Support Not Planned*.
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    Update per 39d9ffe.

    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.

    6 participants