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.

Sign up for GitHub

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

Jump to bottom

[BUG] fix median.ts to work for scripted field #1302

Merged
merged 1 commit into from
Mar 8, 2022
Merged

[BUG] fix median.ts to work for scripted field #1302

merged 1 commit into from
Mar 8, 2022

Conversation

ananzh
Copy link
Member

@ananzh ananzh commented Mar 1, 2022 •
edited by kavilla
Loading

Issue Resolved: #1296

Results

Create a script using sample eCommerce data:

if (doc.containsKey('base_price') && !doc['base_price'].empty) {
    if (doc['base_price'].value <30) {
    return -10;
}
}
return 10;

Previously, the result is:
Screen Shot 2022-03-01 at 11 48 44 AM

After fix, now we could see the correct median:
Screen Shot 2022-03-01 at 11 43 25 AM

Check List

  • New functionality includes testing.
    • All tests pass
      • yarn test:jest
      • yarn test:jest_integration
      • yarn test:ftr
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff

@ananzh ananzh requested a review from a team as a code owner March 1, 2022 20:09
@ananzh ananzh requested review from kavilla and tmarkley March 1, 2022 20:10
@ananzh ananzh self-assigned this Mar 1, 2022
@ananzh ananzh added the bug Something isn't working label Mar 1, 2022
@kavilla kavilla added the v1.3.0 label Mar 4, 2022
@ananzh ananzh force-pushed the median_fix branch 2 times, most recently from 5288e94 to 4f31220 Compare March 7, 2022 17:37
@ananzh ananzh requested a review from ashwin-pc March 7, 2022 19:01
@ananzh
[BUG] fix median.ts to work for scripted field
5ad2d48 
Fix PR comment to replace unused agg to _
remove write function

Issue Resolved: opensearch-project#1296

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
ashwin-pc
ashwin-pc approved these changes Mar 8, 2022
View reviewed changes
Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

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

Nice!

kavilla
kavilla approved these changes Mar 8, 2022
View reviewed changes
Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@kavilla kavilla merged commit 48eec83 into opensearch-project:main Mar 8, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 8, 2022
@ananzh @github-actions
[BUG] fix median.ts to work for scripted field (#1302)
79a2106 
Fix PR comment to replace unused agg to _
remove write function

Issue Resolved: #1296

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
(cherry picked from commit 48eec83)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Mar 8, 2022
Merged
kavilla pushed a commit that referenced this pull request Mar 8, 2022
@ananzh @kavilla
[BUG] fix median.ts to work for scripted field (#1302)
6f763c6 
Fix PR comment to replace unused agg to _
remove write function

Issue Resolved: #1296

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
Signed-off-by: Anan <79961084+ananzh@users.noreply.github.com>
(cherry picked from commit 48eec83)
ananzh added a commit that referenced this pull request Mar 8, 2022
@opensearch-trigger-bot @ananzh
[BUG] fix median.ts to work for scripted field (#1302) (#1325)
e02179b 
Fix PR comment to replace unused agg to _
remove write function

Issue Resolved: #1296

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
Signed-off-by: Anan <79961084+ananzh@users.noreply.github.com>
(cherry picked from commit 48eec83)

Co-authored-by: Anan <79961084+ananzh@users.noreply.github.com>
@ananzh ananzh deleted the median_fix branch June 1, 2022 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kavilla kavilla kavilla approved these changes

@ashwin-pc ashwin-pc ashwin-pc approved these changes

@tmarkley tmarkley Awaiting requested review from tmarkley

Assignees

@ananzh ananzh

Labels
bug Something isn't working v1.3.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Median metric does not work for scripted fields
3 participants
@ananzh @kavilla @ashwin-pc