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

Sorting pivot values numerically when field is numeric. #20918

Open

dennisoelkers wants to merge 35 commits into master from fix/numeric-pivot-sorting

Member

dennisoelkers commented Nov 8, 2024 •

edited

Loading

Note: This requires a backport to 6.0 and 6.1.

Description

Motivation and Context

Fixes Graylog2/graylog-plugin-enterprise#6479.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Refactoring (non-breaking change)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.

dennisoelkers force-pushed the fix/numeric-pivot-sorting branch from b35bc22 to f3ea5b8 Compare

November 8, 2024 08:17

dennisoelkers added 7 commits

November 11, 2024 11:26


          Sorting pivot values numerically when field is numeric.

d4b1a2f


          Improving test code infrastructure.

a9cbff9


          Adding license header.

92599d8


          Adding changelog snippet.

b2ae0db


          Adding mandatory description.

b901868


          Cleaning up tests.

715c0f0


          Completing test case

3f1380f

dennisoelkers force-pushed the fix/numeric-pivot-sorting branch from a7e1cee to 3f1380f Compare

November 11, 2024 10:26

dennisoelkers added 3 commits

November 11, 2024 12:20


          Fixing test.

6d6221a


          Cleaning up.

f82989d


          Using query again to limit search scope.

f81b98b

dennisoelkers marked this pull request as ready for review

November 11, 2024 13:49

dennisoelkers added 3 commits

November 11, 2024 15:15


          Changing order to see error earlier.

91f5bd4


          Merge remote-tracking branch 'origin/master' into fix/numeric-pivot-s…

777231d

…orting


          Removing duplicate assertion.

a38bce9

dennisoelkers requested review from janheise, luk-kaminski and todvora

November 12, 2024 10:51

luk-kaminski reviewed

View reviewed changes

...in/java/org/graylog/storage/opensearch2/views/searchtypes/pivot/buckets/OSValuesHandler.java Outdated Show resolved Hide resolved


          Cleaning up method signatures.

5713da1

luk-kaminski reviewed

View reviewed changes

...in/java/org/graylog/storage/opensearch2/views/searchtypes/pivot/buckets/OSValuesHandler.java Outdated Show resolved Hide resolved

luk-kaminski reviewed

View reviewed changes

...in/java/org/graylog/storage/opensearch2/views/searchtypes/pivot/buckets/OSValuesHandler.java Outdated Show resolved Hide resolved

luk-kaminski reviewed

View reviewed changes

...in/java/org/graylog/storage/opensearch2/views/searchtypes/pivot/buckets/OSValuesHandler.java Outdated Show resolved Hide resolved

luk-kaminski reviewed

View reviewed changes

...in/java/org/graylog/storage/opensearch2/views/searchtypes/pivot/buckets/OSValuesHandler.java Outdated Show resolved Hide resolved

dennisoelkers added 2 commits

November 12, 2024 15:09


          Adding clarifying comments.

64c6a55


          Adding additional tests.

473e747

dennisoelkers force-pushed the fix/numeric-pivot-sorting branch from 37a3716 to 01ce66d Compare

November 13, 2024 09:01


          Generating proper aggregation sort for if multiple pivots.

fcc397a

dennisoelkers force-pushed the fix/numeric-pivot-sorting branch from 01ce66d to fcc397a Compare

November 13, 2024 09:03

dennisoelkers added 2 commits

November 13, 2024 10:12


          Wrapping query with stream, so pivot streams can be defined in tests.

7f6470a


          Normalize streams properly before fetching field type.

ea868f6

dennisoelkers requested a review from luk-kaminski

November 13, 2024 09:17

luk-kaminski reviewed

View reviewed changes

...ava/org/graylog/storage/elasticsearch7/views/searchtypes/pivot/ESPivotBucketSpecHandler.java Outdated Show resolved Hide resolved

luk-kaminski reviewed

View reviewed changes

...ava/org/graylog/storage/elasticsearch7/views/searchtypes/pivot/ESPivotBucketSpecHandler.java Show resolved Hide resolved

luk-kaminski reviewed

View reviewed changes

...ava/org/graylog/storage/elasticsearch7/views/searchtypes/pivot/ESPivotBucketSpecHandler.java Show resolved Hide resolved

dennisoelkers added 16 commits

November 13, 2024 11:54


          Merge remote-tracking branch 'origin/master' into fix/numeric-pivot-s…

…orting


          Improving test robustness.

671fa4b


          Renaming variable to communicate its intent better.

71c1327


          Cleaning up.

ca781c4


          Waiting longer for field types.

01abd53


          Refreshing before executing search.

89fe6c0


          Revert "Refreshing before executing search."

52cd672

This reverts commit 89fe6c0.


          Wait for messages scopes to stream.

c064cd5


          Waiting longer for messages.

55e0bdf


          Merge branch 'master' into fix/numeric-pivot-sorting

1a22d7a


          Increasing wait for messages by a lot for testing.

3c734e1


          Using backend failure config for tests.

17af2d9


          Ingesting each message three times.


          Cleaning up.

fed438e


          Show backend log when waiting for messages fails.

feadd0a


          Merge branch 'master' into fix/numeric-pivot-sorting

d866d3a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet