Skip to content

fix(mango): align opts.fields with fields on _explain#4756

Merged
pgj merged 1 commit intoapache:mainfrom
pgj:fix/mango/explain-opts_fields
Sep 12, 2023
Merged

fix(mango): align opts.fields with fields on _explain#4756
pgj merged 1 commit intoapache:mainfrom
pgj:fix/mango/explain-opts_fields

Conversation

@pgj
Copy link
Copy Markdown
Contributor

@pgj pgj commented Sep 11, 2023

This is a follow-up to 83e39b6 where the all_fields value was replaced for [] to preserve the JSON array type. The same has to be done for fields in the opts nesting object as well — these two attributes should behave alike.

Testing recommendations

make eunit apps=mango

Similarly to #4751, users who based their solution on utilizing the "all_fields" response explicitly might need to change it to use [] instead. Otherwise they are not affected by the change.

Checklist

  • Code is written and works correctly
  • Changes are covered by tests

@pgj pgj force-pushed the fix/mango/explain-opts_fields branch from 226534e to ee47a38 Compare September 11, 2023 17:23
This is a follow-up to 83e39b6 where the `all_fields` value was
replaced for `[]` to preserve the JSON array type.  The same has to
be done for `fields` in the `opts` nesting object as well -- these
two attributes should behave alike.
@pgj pgj force-pushed the fix/mango/explain-opts_fields branch from ee47a38 to c478955 Compare September 11, 2023 18:16
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 good catch, we might as well stay consistent.

all_fields is not an explicit option users pass in to be worth preserving "as-is" for debugging purposes.

@pgj pgj merged commit 86df356 into apache:main Sep 12, 2023
@pgj pgj deleted the fix/mango/explain-opts_fields branch September 12, 2023 11:14
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