Skip to content

fix(mango): correct behavior of fields on _explain#4751

Merged
pgj merged 1 commit intoapache:mainfrom
pgj:fix/mango/correct-explain-fields
Sep 8, 2023
Merged

fix(mango): correct behavior of fields on _explain#4751
pgj merged 1 commit intoapache:mainfrom
pgj:fix/mango/correct-explain-fields

Conversation

@pgj
Copy link
Copy Markdown
Contributor

@pgj pgj commented Sep 7, 2023

The fields attribute on _explain might have the string value "all_fields" when the respective query parameter (fields) is not set by the user. This is to express that no projection of fields would happen on the returned documents.

The current behavior contradicts with the current contract, because by documentation, fields is an array of strings instead to provide information on the projected fields. The discrepancy here makes it hard to formalize this in OpenAPI which leads to problems in the development of SDKs, among others.

Change _explain to return [] for fields when it was not set through the query parameters. This is thought to be semantically equivalent to "all_fields" therefore this would not cause problems while preserving the promised type.

Thanks @ricellis for reporting this problem!

Testing recommendations

make eunit apps=mango

Users who based their solution on utilizing the "all_fields" response explicitly might need to change it to use [] instead. Otherwise they will not be affected by the change.

Checklist

  • Code is written and works correctly
  • Changes are covered by tests
  • Documentation changes were made in the src/docs folder

Copy link
Copy Markdown
Contributor

@nickva nickva left a comment

Choose a reason for hiding this comment

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

+1 (with a few grammar nits)

The main thing I checked was that previously fields: [] was not a thing and it meant all_fields anyway, as opposed some odd "I'd like no fields at all returned" query.

Comment thread src/docs/src/api/database/find.rst Outdated
@pgj pgj force-pushed the fix/mango/correct-explain-fields branch from fe9bf30 to d38ba73 Compare September 7, 2023 15:49
The `fields` attribute on `_explain` might have the string value
`"all_fields"` when the respective query parameter (`fields`) is
not set by the user.  This is to express that no projection of
fields would happen on the returned documents.

The current behavior contradicts with the current contract,
because by documentation, `fields` is an array of strings instead
to provide information on the projected fields.  The discrepancy
here makes it hard to formalize this in OpenAPI which leads to
problems in the development of SDKs, among others.

Change `_explain` to return `[]` for `fields` when it was not set
through the query parameters. This is thought to be semantically
equivalent to `"all_fields"` therefore this would not cause
problems while preserving the promised type.

Thanks @ricellis for reporting this problem!
@pgj pgj force-pushed the fix/mango/correct-explain-fields branch from d38ba73 to 36d847a Compare September 7, 2023 16:07
@pgj pgj merged commit 83e39b6 into apache:main Sep 8, 2023
@pgj pgj deleted the fix/mango/correct-explain-fields branch September 8, 2023 13:22
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.

2 participants