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

MONGOID-5161 - [Documentation Only] Clarify the policy for working with . and $ named fields which were introduced in MongoDB 5.0 #5051

Merged

Conversation

johnnyshields
Copy link
Contributor

@johnnyshields johnnyshields commented Aug 21, 2021

HELP: I cannot create tickets in MongoDB JIRA. When I open "Create Ticket" then press submit, the JIRA server returns 200 but no ticket is made. (I've tried in multiple browsers and with multiple JIRA accounts.) In order to follow ticket process as best as possible, until the issue is resolved I will:

  • mark Github PRs with MONGOID-????
  • include this message in each affected PR

Documentation only.

In summary:

  • $ and . fields (introduced in MongoDB 5.0) are special "hidden fields" that can only be accessed with special "shadow" operators.
  • Mongoid query operators / field names will behave as normal post-MongoDB 5.0
  • The field function will not allow . and $ in field names; Ruby doesn't support such method names anyway.

@johnnyshields johnnyshields changed the title MONGOID-???? - This clarifies the policy for working with . and $ named fields which were introduced in MongoDB 5.0 MONGOID-???? - Clarify the policy for working with . and $ named fields which were introduced in MongoDB 5.0 Aug 21, 2021
@johnnyshields johnnyshields changed the title MONGOID-???? - Clarify the policy for working with . and $ named fields which were introduced in MongoDB 5.0 MONGOID-???? - [Documentation Only] Clarify the policy for working with . and $ named fields which were introduced in MongoDB 5.0 Aug 21, 2021
@p-mongo
Copy link
Contributor

p-mongo commented Aug 23, 2021

@johnnyshields can you try creating an issue from the form at https://jira.mongodb.org/secure/CreateIssue!default.jspa ? I escalated this internally as well.

@p-mongo p-mongo changed the title MONGOID-???? - [Documentation Only] Clarify the policy for working with . and $ named fields which were introduced in MongoDB 5.0 MONGOID-5161 - [Documentation Only] Clarify the policy for working with . and $ named fields which were introduced in MongoDB 5.0 Aug 25, 2021
Copy link
Contributor

@p-mongo p-mongo left a comment

Choose a reason for hiding this comment

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

I responded to the core idea of this proposal in https://jira.mongodb.org/browse/MONGOID-5161. The inline comments are reactions to specific changes that are proposed here.

docs/reference/fields.txt Outdated Show resolved Hide resolved
* Mongoid does not support queries or operations which reference specially-named fields.
* If a document existing in the database contains specially-named fields,
Mongoid will load the document normally **except** for its specially-named fields,
which will not be retrieved.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not true, attributes will show up in attributes hash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you test this on MongoDB 5.0? I do not believe the DB will return them on standard queries.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe I tested on 5.0 but if you wish I can test again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, please provide evidence of this. The MongoDB documentation seems to indicate such fields won't be returned.

("specially-named fields"):

* Mongoid's :ref:`field <field-types>` macro does not support defining specially-named fields.
* Mongoid does not support getting or setting the values of specially-named fields.
Copy link
Contributor

Choose a reason for hiding this comment

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

I expect this to be possible with e.g. an explicit $set.

Copy link
Contributor Author

@johnnyshields johnnyshields Aug 29, 2021

Choose a reason for hiding this comment

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

It should not be possible with $set, because MongoDB's $set does not allow it. Note that set interprets . as path traversal.

You must use the $setField operator instead.

Please, please do not hack $set in Mongoid so that it magically calls $setField under the hood. Instead, if required, make $setField it's own special atomic operator.

when working with fields whose names are prefixed with ``$`` or contain ``.``
("specially-named fields"):

* Mongoid's :ref:`field <field-types>` macro does not support defining specially-named fields.
Copy link
Contributor

Choose a reason for hiding this comment

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

I tested it and it seems to work. The utiilty of such fields is limited presently but I can define one:

class Foo
  include Mongoid::Document
  
  field 'a.b'
end

Copy link
Contributor Author

@johnnyshields johnnyshields Aug 29, 2021

Choose a reason for hiding this comment

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

Yes, but it doesn't support persisting the field to the database today, does it?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a driver limitation. Once it is lifted in the driver, I expect all published Mongoid versions will be able to persist fields like this.

Copy link
Contributor Author

@johnnyshields johnnyshields Sep 10, 2021

Choose a reason for hiding this comment

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

@p-mongo from your comments on this issue it appears you are fundamentally misunderstanding how these fields work in MongoDB, and furthermore you have a mistaken conception that they can be treated like regular fields in Mongoid (to provide a supposedly "seamless" experience to the end-user.)

Although such an approach may be possible to some extent, it will require so many hacks and caveats that it will not be practical. The fields should be handled specially and only accessed via specialized getSpecialField(s)/setSpecialField(s) functions.


* Mongoid's :ref:`field <field-types>` macro does not support defining specially-named fields.
* Mongoid does not support getting or setting the values of specially-named fields.
* Mongoid does not support queries or operations which reference specially-named fields.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see how this is true. If you are using 5.0 server you should be able to query these fields using the respective server operators.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does Mongoid support this today?

Copy link
Contributor

Choose a reason for hiding this comment

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

What I meant was that you should be able to use the 5.0 server operators via the MQL syntax. Mongoid supports this (I would say) because it passes through MQL syntax to the driver and ultimately the server.

Mongoid doesn't presently provide helpers for the new 5.0 query operators, but a user utilizing Mongoid should be able to perform these queries hence I'd rather not state that something is not possible when there is a way to do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've clarified the language "Mongoid does not provide helper functions or idiomatic support..."


Lastly, please note that method names containing ``$`` or ``.``
are `not permitted in Ruby language
<https://docs.ruby-lang.org/en/3.0.0/doc/syntax/methods_rdoc.html>`_.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also not true:

define_method('foo.bar') { 42 }
send('foo.bar')
# => 42

We actually had a ticket filed by someone a while ago who generated method names with spaces in them based on MongoDB field names.

Copy link
Contributor Author

@johnnyshields johnnyshields Aug 29, 2021

Choose a reason for hiding this comment

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

That is hideous. And no, that doesn't count as a standard method because I cannot define it with def and call it as object.foo.bar. I've updated the doc to say are not standard syntax.

I pray that you are not considering supporting . / $ in this manner... we should absolutely not define special methods which can only be accessed with send. Instead, we should require using read_attribute to access them.

Copy link
Contributor

Choose a reason for hiding this comment

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

At this point I am not aware of any plans for dot or dollar changes in Mongoid. In any event I would prefer to not break compatibility, hence I think we are largely on the same page with respect to Mongoid's direction but again I am unaware of any specific changes that are under consideration for Mongoid right now.

@p-mongo
Copy link
Contributor

p-mongo commented Aug 30, 2021

The purpose of my examples was to demonstrate the various ways that someone can already use to work with data which happens to include dots and dollars. It is a known fact that dots and dollars are treated specially by various MongoDB-related software and therefore they require care, and aren't easy to use in general in field names. However I don't see how stating that X is "not supported" or "not permitted" when there are ways of achieving it in some capacity is helpful.

Especially given that the direction of MongoDB is to make dots and dollars in field names more usable, adding documentation stating this sort of thing is not supported/not permitted isn't aligned with the direction in which MongoDB is heading.

@johnnyshields
Copy link
Contributor Author

johnnyshields commented Aug 30, 2021

the direction of MongoDB is to make dots and dollars in field names more usable

A better way to say this is "MongoDB is making dots and dollars possible to use, with extreme caveats, drawbacks, and limitations." Such fields will not be treated as "first-class citizens"; you cannot index them, you must use special operators to get/set them, etc. This is clear in the MongoDB documentation. They are only being supported for edge-cases of working with legacy data-sources which for whatever reason can't be mapped.

Mongoid/Ruby should take a similar approach: In the future, it should be possible to use/access such fields if you have no other alternative, but it should be discouraged. These fields should not be treated as "standard", and path traversal/operator functionality should not be sacrificed in order to support them.

(For avoidance of doubt: if you are under the impression that MongoDB is deprecating path traversal/operators in order to support these fields, I recommend to speak with your Product team.)

@p-mongo
Copy link
Contributor

p-mongo commented Sep 10, 2021

I intend to propose some language here and I'll put this PR into my todo list to remind me to do so.

@johnnyshields would you prefer me to add commits to this PR with changes, make my own PR or something else?

@johnnyshields
Copy link
Contributor Author

@p-mongo it's fine to commit the changes to this branch.

@johnnyshields
Copy link
Contributor Author

@p-mongo any update on this?

@p-mongo p-mongo force-pushed the clarify-policy-for-dot-dollar-fields branch from ccc8515 to f56184e Compare January 3, 2022 17:42
@p-mongo p-mongo requested a review from comandeo January 3, 2022 17:42
@p-mongo p-mongo merged commit 7ad439b into mongodb:master Jan 5, 2022
p-mongo pushed a commit that referenced this pull request Jan 7, 2022
…th . and $ named fields which were introduced in MongoDB 5.0 (#5051)

* MONGOID-???? - Documentation only. This clarifies the policy for working with . and $ named fields which were introduced in MongoDB 5.0

* Add reference

* Update fields.txt

* Update fields.txt

* Update fields.txt

* add sphinx note

* rewrite the docs

Co-authored-by: shields <[email protected]>
Co-authored-by: Oleg Pudeyev <[email protected]>
p-mongo pushed a commit to johnnyshields/mongoid that referenced this pull request Jan 21, 2022
* master:
  MONGOID-5098 Bug fix: Range.mongoize should mongoize its members (mongodb#5108)
  MONGOID-5212 Support for Decimal128 type (mongodb#5125)
  MONGOID-5213 Document changes to BigDecimal type and addition of global flag (mongodb#5126)
  MONGOID-5209 Document "Custom Field Options" functionality + various docs improvements (mongodb#5120)
  MONGOID-5193 rails 7 support  (mongodb#5122)
  Fix default topology name - should be standalone (mongodb#5130)
  Update MRSS (mongodb#5129)
  MONGOID-5207 Test Ruby 3.1 on Evergreen (mongodb#5123)
  MONGOID-5186 .with_scope should restore previous scope (mongodb#5127)
  MONGOID-5161 - [Documentation Only] Clarify the policy for working with . and $ named fields which were introduced in MongoDB 5.0 (mongodb#5051)
p-mongo pushed a commit to johnnyshields/mongoid that referenced this pull request Jan 21, 2022
* master: (48 commits)
  MONGOID-5098 Bug fix: Range.mongoize should mongoize its members (mongodb#5108)
  MONGOID-5212 Support for Decimal128 type (mongodb#5125)
  MONGOID-5213 Document changes to BigDecimal type and addition of global flag (mongodb#5126)
  MONGOID-5209 Document "Custom Field Options" functionality + various docs improvements (mongodb#5120)
  MONGOID-5193 rails 7 support  (mongodb#5122)
  Fix default topology name - should be standalone (mongodb#5130)
  Update MRSS (mongodb#5129)
  MONGOID-5207 Test Ruby 3.1 on Evergreen (mongodb#5123)
  MONGOID-5186 .with_scope should restore previous scope (mongodb#5127)
  MONGOID-5161 - [Documentation Only] Clarify the policy for working with . and $ named fields which were introduced in MongoDB 5.0 (mongodb#5051)
  MONGOID-5173 Specs should use bang (!) methods (without describe/context change) (mongodb#5109)
  Fix doc syntax
  RUBY-2783 Require bson 4-stable for Mongoid as bson master is 5.0 and not compatible with driver (mongodb#5113)
  MONGOID-5208 fix error on reloading nil embedded document (mongodb#5116)
  MONGOID-5206 fix bug where embedded document is not re-embedded (mongodb#5115)
  MONGOID-5207 Add Ruby 3.1 to GH Actions (mongodb#5114)
  MONGOID-5207 Use YAML.safe_load in specs (mongodb#5112)
  MONGOID-5199 Reloadable#reload_embedded_document doesn't respect session (mongodb#5105)
  Fix MONGOID-5198 Calling children_changed? on a deep cyclical data structure will cause semi-infinite looping (mongodb#5104)
  MONGOID-5203 Add all available auth_mech options to Configuration documentation (mongodb#5103)
  ...
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.

4 participants