Add max/min eval functions by ritvibhatt · Pull Request #4333 · opensearch-project/sql

ritvibhatt · 2025-09-19T00:20:54Z

Description

Add support for min/max statistical eval functions, allowing users to find maximum and minimum values among multiple arguments within a single row.

Updated parser to add max and min syntax for eval command
Added MaxFunction and MinFunction classes with Calcite UDF implementation
Updated PPLBuiltinOperators and PPLFuncImpTable to register the new functions

Usage Examples

-- Returns the larger value between age field and 30 for each row
source=accounts | eval max_age = MAX(age, 30) | fields age, max_age

-- Returns either 'John' or value in firstname depending on what is larger lexicographically
source=accounts | eval result = MAX(age, 'John', firstname) | fields age, firstname, result

-- Returns either the value in the age field or 35
source=accounts | eval result = MIN(age, 35, firstname) | fields age, firstname, result

Related Issues

Resolves #4341

Check List

New functionality includes testing.
New functionality has been documented.
New functionality has javadoc added.
New functionality has a user manual doc added.
New PPL command checklist all confirmed.
API changes companion pull request created.
Commits are signed per the DCO using --signoff or -s.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

dai-chen

A high level QQ: is supporting max(int, string) a hard requirement for now?

I'm just thinking otherwise:

We probably can reuse the existing array_min/max function;
The implicit conversion (string -> numeric) can be supported in #4349. cc: @penghuo

penghuo · 2025-09-23T22:24:22Z

docs/user/ppl/functions/statistical.rst

+   :local:
+   :depth: 1
+
+.. versionadded:: 3.3.0


the format looks not correct. https://github.com/ritvibhatt/sql/blob/7203266ac0d716e116e5ea0a9af1ed8902dda008/docs/user/ppl/functions/statistical.rst

Updated the doc

penghuo · 2025-09-23T22:33:41Z

core/src/main/java/org/opensearch/sql/expression/function/udf/math/MaxFunction.java

+      boolean aIsNumeric = isNumeric(a);
+      boolean bIsNumeric = isNumeric(b);
+
+      if (aIsNumeric != bIsNumeric) {


what is expectataion is max(4, "2")? should be 4, right?

Max(4, "2") should result in "2" since strings are always considered larger than numeric values

penghuo · 2025-09-23T22:49:58Z

A high level QQ: is supporting max(int, string) a hard requirement for now?

I'm just thinking otherwise:

We probably can reuse the existing array_min/max function;

The implicit conversion (string -> numeric) can be supported in [RFC] Support permissive mode in PPL #4349. cc: @penghuo

array_min/max required all arguments have same type.

max/min required speical handling, implicit cast may not work, e.g. the expactation is

max(20, "4")    should return 20
max(20, "4a")   should return 4a

convert fields to int does not work

array_max(20, 4)      should return 20
array_max(20, null)   should return 20

convert fields to string does not work

array_max("20", "4")    should return "4"
array_max("20", "4a")   should return "4a"

dai-chen · 2025-09-24T16:20:02Z

@ritvibhatt I synced with @penghuo offline. For Q2, it doesn’t seem to be a type conversion issue as I originally thought, but rather a data sorting one. For example, we could define a custom comparator in Java to handle the new sorting rule for numeric and string values, and then apply it in min/max/sort APIs. Perhaps we can do something similar within the Calcite accumulator?

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

Signed-off-by: ritvibhatt <53196324+ritvibhatt@users.noreply.github.com>

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

dai-chen · 2025-09-29T22:26:10Z

core/src/main/java/org/opensearch/sql/data/utils/MaxTypeComparator.java

+import java.util.Comparator;
+
+/** Comparator for MAX operations where strings have higher precedence than numbers. */
+public class MaxTypeComparator implements Comparator<Object> {


Is MinTypeComparator simply the inverse of MaxTypeComparator? I'm thinking of only one comparator and use it in Min/MaxFunction below by Java Stream's min/max(comparator) API.

Also this may worth adding dedicated UT to show its behavior for num-num, num-string, string-string etc.

Updated and added tests thank you!

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

dai-chen

Thanks for the changes!

dai-chen · 2025-09-30T16:12:25Z

core/src/main/java/org/opensearch/sql/data/utils/MixedTypeComparator.java

+      if (aIsNumeric) {
+        return Double.compare(((Number) a).doubleValue(), ((Number) b).doubleValue());
+      } else {
+        return Integer.compare(a.toString().compareTo(b.toString()), 0);


what does this Integer.compare with 0 meaning?

It will normalize it so if the string comparison returns a negative it will make it -1 and if it returns a positive it will be 1. Don't think that is necessary, can remove and just leave the string comparison

dai-chen · 2025-09-30T16:13:31Z

core/src/main/java/org/opensearch/sql/data/utils/MixedTypeComparator.java

+  }
+
+  private static boolean isNumeric(Object obj) {
+    return obj instanceof Number;


Could you confirm whether comparison between "1" and "2" also considered as numerical comparison?

No they will be compared as strings so max("9", "21") will return "9"

opensearch-trigger-bot · 2025-09-30T18:30:31Z

The backport to 2.19-dev failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/sql/backport-2.19-dev 2.19-dev
# Navigate to the new working tree
pushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-4333-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 fae06873a057f3bcdea1a7a113c74932fc801deb
# Push it to GitHub
git push --set-upstream origin backport/backport-4333-to-2.19-dev
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/sql/backport-2.19-dev

Then, create a pull request where the base branch is 2.19-dev and the compare/head branch is backport/backport-4333-to-2.19-dev.

(cherry picked from commit fae0687)

(cherry picked from commit fae0687) Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

* main-apple: (218 commits) Add ignorePrometheus Flag for integTest and docTest (opensearch-project#4442) Create fab-radar.yml PPL `fillnull` command enhancement (opensearch-project#4421) reverting to _doc + _id (opensearch-project#4435) Support `multisearch` command in calcite (opensearch-project#4332) Add 3.3 release notes (opensearch-project#4422) (opensearch-project#4423) [SQL/PPL] Fix the `count(*)` and `dc(field)` to be capped at MAX_INTEGER opensearch-project#4416 (opensearch-project#4418) Change the default search sort tiebreaker to `_shard_doc` for PIT search (opensearch-project#4378) [Enhancement] Add error handling for known limitation of sql `JOIN` (opensearch-project#4344) Bugfix: SQL type mapping for legacy JDBC output (opensearch-project#3613) Version bump: 3.3 (opensearch-project#4417) Add max/min eval functions (opensearch-project#4333) Support time modifiers in search command (opensearch-project#4224) Fix numbered token bug and make it optional output in patterns command (opensearch-project#4402) refactor span (opensearch-project#4334) Move release notes categories (opensearch-project#3818) [Doc] Enable doctest with Calcite (opensearch-project#4379) Mod function should return decimal instead of float when handle the operands are decimal literal (opensearch-project#4407) Scale of decimal literal should always be positive in Calcite (opensearch-project#4401) Enable Calcite by default and implicit fallback the unsupported commands (opensearch-project#4372) ...

ritvibhatt added 7 commits September 18, 2025 17:19

add eval max/min functions

831e361

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

enable calcite doctest

5f7b2ed

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

update docs

e753052

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

fix docs

5afd678

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

remove max/min from math docs

5049d20

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

fix formatting

d116833

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

fix typo

c076751

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

ritvibhatt marked this pull request as ready for review September 19, 2025 23:49

RyanL1997 added calcite calcite migration releated PPL Piped processing language backport 2.19-dev feature labels Sep 22, 2025

ritvibhatt added 4 commits September 22, 2025 17:03

fix null handling

07fc0b6

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

add tests for null

7858fd5

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

fix failure

f776fb1

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

fix

7203266

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

dai-chen reviewed Sep 23, 2025

View reviewed changes

penghuo reviewed Sep 23, 2025

View reviewed changes

ritvibhatt and others added 3 commits September 29, 2025 11:27

move min/max comparison logic

0d583f9

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

Merge branch 'main' into eval-max-min

a4ab8bd

Signed-off-by: ritvibhatt <53196324+ritvibhatt@users.noreply.github.com>

fix formatting

160042d

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

dai-chen reviewed Sep 29, 2025

View reviewed changes

ritvibhatt added 2 commits September 29, 2025 15:27

update docs

cecaf57

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

add one comparator for min/max

43f5480

Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

dai-chen approved these changes Sep 30, 2025

View reviewed changes

Swiddis approved these changes Sep 30, 2025

View reviewed changes

Swiddis merged commit fae0687 into opensearch-project:main Sep 30, 2025
33 checks passed

opensearch-trigger-bot bot added the backport-failed label Sep 30, 2025

ritvibhatt added a commit to ritvibhatt/sql that referenced this pull request Sep 30, 2025

Add max/min eval functions (opensearch-project#4333)

0e6bf82

(cherry picked from commit fae0687)

penghuo mentioned this pull request Oct 7, 2025

[Backport 2.19-dev] [Doc] Enable doctest with Calcite (#4379) #4448

Closed

8 tasks

ritvibhatt added a commit to ritvibhatt/sql that referenced this pull request Oct 7, 2025

Add max/min eval functions (opensearch-project#4333)

021e233

(cherry picked from commit fae0687) Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

penghuo pushed a commit that referenced this pull request Oct 7, 2025

Add max/min eval functions (#4333) (#4425)

25033f6

(cherry picked from commit fae0687) Signed-off-by: Ritvi Bhatt <ribhatt@amazon.com>

LantaoJin added the backport-manually Filed a PR to backport manually. label Oct 9, 2025

dai-chen mentioned this pull request Nov 10, 2025

[BUG] Scalar and aggregate MIN/MAX conflict when translating PPL to SQL #4774

Closed

Conversation

ritvibhatt commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

dai-chen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

penghuo commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dai-chen commented Sep 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dai-chen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

opensearch-trigger-bot bot commented Sep 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ritvibhatt commented Sep 19, 2025 •

edited

Loading

dai-chen left a comment •

edited

Loading

penghuo commented Sep 23, 2025 •

edited

Loading