Add compare_ip operator udfs by ishaoxy · Pull Request #3821 · opensearch-project/sql

ishaoxy · 2025-06-25T10:08:00Z

Description

Introduced a new UDF CompareIpFunction for IP address comparisons.
Since Calcite does not provide a native SQL type for IP addresses, EXPR_IP is mapped to SqlTypeName.OTHER.
Enhanced the registerOperator method to support registering one or multiple operators under a single function name, enabling function overloading based on operand types.
Registered the newly implemented IP comparison operators under the existing comparison function names(e.g., =, !=, <, >, etc.).
Added new and specific type checkers for IP comparison and CIDR functions to ensure accurate operand validation and operator resolution.

Related Issues

Resolves #3776

Check List

New functionality includes testing.
New functionality has been documented.
New functionality has javadoc added.
New functionality has a user manual doc added.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
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: Xinyu Hao <haoxinyu@amazon.com>

qianheng-aws · 2025-06-26T09:08:59Z

I think we should implement implicit conversion before implementing comparison for non-primitive types.

After that, for Ip comparison, we can convert string to ExprIP. And then we can implement some generic comparison functions for types of comparable, via calling their compareTo method.

But current approach is still acceptable if we won't have other UDT in the future.

qianheng-aws

Please add IT for ip comparison.

qianheng-aws · 2025-06-26T09:44:06Z

core/src/main/java/org/opensearch/sql/expression/function/udf/ip/CidrMatchFunction.java

+    // EXPR_IP is mapped to SqlTypeFamily.NULL in
    // UserDefinedFunctionUtils.convertRelDataTypeToSqlTypeName
-    return UDFOperandMetadata.wrap(OperandTypes.STRING_STRING);
+    return UDFOperandMetadata.wrap(


Maybe we should use new PPLTypeChecker here and override its checkOperandTypes method to only allow accept string or ip.

qianheng-aws · 2025-06-26T09:45:46Z

core/src/main/java/org/opensearch/sql/expression/function/PPLTypeChecker.java

              OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.INTEGER));
          case ANY, IGNORE -> List.of(
              OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.ANY));
+            // We borrow SqlTypeFamily.NULL to represent EXPR_IP. This is a workaround


If we stop use CompositeOperandTypeChecker and create a new TypeChecker ourself, maybe we can avoid using SqlTypeFamily.NULL to represent EXPR_IP and check operands on RelDataType level.

@qianheng-aws How about this implementation ishaoxy#1
It doesn't use NULL for IP, but created two other classes for IP type checking.

It just came to me that there's not only compare that checks IP type. CIDR_MATCH also has to validate IP types. If we make a specific IP type checker for compare operators, we may also have to create one for each other functions like cidrmatch and geoip.

penghuo · 2025-06-26T17:05:01Z

core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java


    void populate() {
+      // register operators for IP comparing
+      registerOperator(NOTEQUAL, PPLBuiltinOperators.NOT_EQUALS_IP);


Questions, Do we need to support IP as UDT in PPL engine?

In OpenSearch PPL, IP handling depends on index field type. CIDR-based filtering should use ip_range queries for ip fields, script pushdown / in-memory processing for keyword, text, and runtime string fields.

Field Type Use Case Expectation

IP Field search index=log ip="192.168.0.0/16" Rewrite as term query

search index | where cidrmatch("192.168.0.0/16", ip) Rewrite as term query

Keyword Field search index=log ip="192.168.0.0/16" Rewrite as term query, extactally keyword match

search index | where cidrmatch("192.168.0.0/16", ip) Script pushdown — ip field is a string, not rewrite as term query.

Text Field search index=log ip="192.168.0.0/16" Rewrite as query_string query, full text search

search index | where cidrmatch("192.168.0.0/16", ip) Script pushdown — ip field is a string, not rewrite as term query.

Runtime Field search index=log | parse ip=regex(...)| where ip="192.168.0.0/16" Script pushdown — ip field is a string, it is a string comparsion query

search index=log | parse ip=regex(...)| where cidrmatch("192.168.0.0/16", ip) Script pushdown — ip field is a string, not rewrite as term query.

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

qianheng-aws · 2025-06-30T03:30:58Z

@qianheng-aws How about this implementation ishaoxy#1
It doesn't use NULL for IP, but created two other classes for IP type checking.

Yes, I think that's what we need. In the future when we have type conversion, that IPTypeChecker should only accept type of IP, and type conversion should convert String to ExprIpType

It just came to me that there's not only compare that checks IP type. CIDR_MATCH also has to validate IP types. If we make a specific IP type checker for compare operators, we may also have to create one for each other functions like cidrmatch and geoip.

Yes since we treat ExprIPType as a specific type instead of String

@yuancu

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>

Make IpComparisonOperators an inner enum of CompareIPFunction

LantaoJin · 2025-07-01T03:24:06Z

core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java

+      registerOperator(EQUAL, PPLBuiltinOperators.EQUALS_IP);
+      registerOperator(GREATER, PPLBuiltinOperators.GREATER_IP);
+      registerOperator(GTE, PPLBuiltinOperators.GTE_IP);
+      registerOperator(LESS, PPLBuiltinOperators.LESS_IP);
+      registerOperator(LTE, PPLBuiltinOperators.LTE_IP);


SqlStdOperatorTable.EQUALS` is a very basic std operator which widely used in calcite internal, I am worry that it may introduce potential bugs and performance regression for pushdown.

But Calcite's built-in operators like SqlStdOperatorTable.EQUALS does not handle our UDT like IP. These new operators are only effective to IP comparison, controlled via type checkers. Comparison between other types will still falls to Calcite's built-in comparison operators.

There was two solutions:

one is to convert IP UDT to a type that is comparable by calcite's comparators.

another is to add new operators exclusively for IP comparision

We opted the latter.

In order to keep the comparison logic the same as v2, I chose to add these specific operator udfs for IP comparison instead of converting it to string type that calcite can compare.

These new operators are only effective to IP comparison, controlled via type checkers. Comparison between other types will still falls to Calcite's built-in comparison operators.

Can we move above logic to a specific method registerOverrideOperator, it quite confused me.

Or modify to

registerOperator(EQUAL, check(PPLBuiltinOperators.EQUALS_IP，SqlStdOperatorTable.EQUALS));

and leverage check() to manage the specific register logic via typeChecker.

check() can name to case() or set() or queue()...

Or modify to

registerOperator(EQUAL, check(PPLBuiltinOperators.EQUALS_IP，SqlStdOperatorTable.EQUALS));

and leverage check() to manage the specific register logic via typeChecker.

check() can name to case() or set() or queue()...

Modified, improving codes readability.

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

add type checker for cidr

LantaoJin · 2025-07-04T09:01:22Z

core/src/main/java/org/opensearch/sql/calcite/utils/UserDefinedFunctionUtils.java

-        case EXPR_IP -> SqlTypeName.VARCHAR;
+          // EXPR_IP is mapped to SqlTypeName.OTHER since there is no
+          // corresponding SqlTypeName in Calcite.
+        case EXPR_IP -> SqlTypeName.OTHER;


please update the PR description to align changes.

Thanks for reminding me.

LantaoJin · 2025-07-04T09:03:28Z

core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java

-      } else {
-        typeChecker = operator.getOperandTypeChecker();
-      }
+    public void registerOperator(BuiltinFunctionName functionName, SqlOperator... operators) {


please add a javadoc for this method to explain what are the operators for

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

qianheng-aws · 2025-07-07T03:29:45Z

core/src/main/java/org/opensearch/sql/expression/function/udf/ip/CompareIpFunction.java

+        IPAddress addr1 = IPUtils.toAddress(ip1);
+        IPAddress addr2 = IPUtils.toAddress(ip2);
+        int result = IPUtils.compare(addr1, addr2);
+        return switch (comparisonType) {


For efficiency consideration, we'd better move this switch case to the implement method

qianheng-aws · 2025-07-07T03:30:56Z

core/src/main/java/org/opensearch/sql/expression/function/udf/ip/CompareIpFunction.java

+
+    public static boolean compare(Object obj1, Object obj2, ComparisonType comparisonType) {
+      try {
+        String ip1 = extractIpString(obj1);


Why not leverage ExprIpValue::compare? Then we don't need to do extra transformation for ExprIPValue

Sorry for only focusing on IPUtils before and ignoring the functions encapsulated in ExprIpValue. Now fixed.

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

opensearch-trigger-bot · 2025-07-11T09:27:26Z

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-3821-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 6c3efa1475f815fc9025a2e3c533b7b21d9ea8cf
# Push it to GitHub
git push --set-upstream origin backport/backport-3821-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-3821-to-2.19-dev.

LantaoJin · 2025-07-11T09:28:42Z

@ishaoxy please follow the instructions to backport manually

* ip_compare operator added Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * only type checker issue left Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * fix by modifying ip.sqlTypeName from OTHER to NULL in type checker Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * fix less Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * modify the CalcitePPLFunctionTypeTest text Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * allow CalciteIPComparisonIT in CalciteNoPushdownIT Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * Modify the signature description in udf Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * fix some typing errors Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * modify the udfs for better style Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * Make IpComparisonOperators an inner enum of CompareIPFunction Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * modify registerOperator Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * modify registerOperator Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * add type checker for cidr Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * add javadoc Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * move switch case to the implement method Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> --------- Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> Co-authored-by: Yuanchun Shen <yuanchu@amazon.com> (cherry picked from commit 6c3efa1)

* Add compare_ip operator udfs (#3821) * ip_compare operator added Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * only type checker issue left Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * fix by modifying ip.sqlTypeName from OTHER to NULL in type checker Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * fix less Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * modify the CalcitePPLFunctionTypeTest text Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * allow CalciteIPComparisonIT in CalciteNoPushdownIT Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * Modify the signature description in udf Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * fix some typing errors Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * modify the udfs for better style Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * Make IpComparisonOperators an inner enum of CompareIPFunction Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * modify registerOperator Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * modify registerOperator Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * add type checker for cidr Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * add javadoc Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * move switch case to the implement method Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> --------- Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> Co-authored-by: Yuanchun Shen <yuanchu@amazon.com> (cherry picked from commit 6c3efa1) * backport to java11 Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * backport to java11 Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * backport to java11 Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * backport to java11 Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * backport to java11 Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> * backport to java11 Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> --------- Signed-off-by: Xinyu Hao <haoxinyu@amazon.com> Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> Co-authored-by: Yuanchun Shen <yuanchu@amazon.com>

opensearch-trigger-bot · 2025-07-22T03:53:16Z

The backport to manually 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-manually manually
# Navigate to the new working tree
pushd ../.worktrees/sql/backport-manually
# Create a new branch
git switch --create backport/backport-3821-to-manually
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 6c3efa1475f815fc9025a2e3c533b7b21d9ea8cf
# Push it to GitHub
git push --set-upstream origin backport/backport-3821-to-manually
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/sql/backport-manually

Then, create a pull request where the base branch is manually and the compare/head branch is backport/backport-3821-to-manually.

ishaoxy added 3 commits June 25, 2025 15:13

ip_compare operator added

5d8c62c

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

only type checker issue left

3008d75

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

fix by modifying ip.sqlTypeName from OTHER to NULL in type checker

851f40e

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

ishaoxy requested review from GumpacG, LantaoJin, MaxKsyunz, Swiddis, YANG-DB, Yury-Fridlyand, acarbonetto, anirudha, dai-chen, derek-ho, forestmvey, joshuali925, kavithacm, mengweieric, noCharger, penghuo, ps48, qianheng-aws, seankao-az and ykmr1224 as code owners June 25, 2025 10:08

ishaoxy added 2 commits June 26, 2025 10:53

fix less

e4f619e

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

modify the CalcitePPLFunctionTypeTest text

73bec7b

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

qianheng-aws reviewed Jun 26, 2025

View reviewed changes

penghuo reviewed Jun 26, 2025

View reviewed changes

Swiddis added the enhancement New feature or request label Jun 26, 2025

allow CalciteIPComparisonIT in CalciteNoPushdownIT

2f9c5b4

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

ishaoxy and others added 4 commits June 30, 2025 11:35

fix some typing errors

a4916b2

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

modify the udfs for better style

f755494

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

Make IpComparisonOperators an inner enum of CompareIPFunction

0acc8f7

Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>

Merge pull request #2 from yuancu/refactor-ip-compare

e587271

Make IpComparisonOperators an inner enum of CompareIPFunction

LantaoJin reviewed Jul 1, 2025

View reviewed changes

ishaoxy and others added 4 commits July 3, 2025 15:53

modify registerOperator

fb8e969

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

modify registerOperator

999fdf7

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

add type checker for cidr

5151350

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

Merge pull request #3 from ishaoxy/add-typechecker-cidr

ea1414c

add type checker for cidr

LantaoJin reviewed Jul 4, 2025

View reviewed changes

add javadoc

e2a124f

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

qianheng-aws reviewed Jul 7, 2025

View reviewed changes

move switch case to the implement method

62a6084

Signed-off-by: Xinyu Hao <haoxinyu@amazon.com>

qianheng-aws previously approved these changes Jul 7, 2025

View reviewed changes

Merge remote-tracking branch 'upstream/main' into compare-ip-operator

eedd8c9

ishaoxy dismissed qianheng-aws’s stale review via eedd8c9 July 11, 2025 08:05

LantaoJin approved these changes Jul 11, 2025

View reviewed changes

LantaoJin added the backport 2.19-dev label Jul 11, 2025

qianheng-aws approved these changes Jul 11, 2025

View reviewed changes

qianheng-aws merged commit 6c3efa1 into opensearch-project:main Jul 11, 2025
25 checks passed

opensearch-trigger-bot bot added the backport-failed label Jul 11, 2025

LantaoJin added the backport-manually Filed a PR to backport manually. label Jul 22, 2025

This was referenced Aug 12, 2025

Fix PPL eval command string concatenation with + operator #4020

Merged

[GitHub Request] Add yuancu as maintainer of OpenSearch SQL opensearch-project/.github#374

Closed

Field Type	Use Case	Expectation
IP Field	search index=log ip="192.168.0.0/16"	Rewrite as term query
	search index \| where cidrmatch("192.168.0.0/16", ip)	Rewrite as term query
Keyword Field	search index=log ip="192.168.0.0/16"	Rewrite as term query, extactally keyword match
	search index \| where cidrmatch("192.168.0.0/16", ip)	Script pushdown — ip field is a string, not rewrite as term query.
Text Field	search index=log ip="192.168.0.0/16"	Rewrite as query_string query, full text search
	search index \| where cidrmatch("192.168.0.0/16", ip)	Script pushdown — ip field is a string, not rewrite as term query.
Runtime Field	search index=log \| parse ip=regex(...)\| where ip="192.168.0.0/16"	Script pushdown — ip field is a string, it is a string comparsion query
	search index=log \| parse ip=regex(...)\| where cidrmatch("192.168.0.0/16", ip)	Script pushdown — ip field is a string, not rewrite as term query.

Conversation

ishaoxy commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

qianheng-aws commented Jun 26, 2025

Uh oh!

qianheng-aws 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qianheng-aws commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuancu Jul 1, 2025 • 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!

LantaoJin Jul 1, 2025 • 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!

LantaoJin Jul 1, 2025 • 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!

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 Jul 11, 2025

Uh oh!

LantaoJin commented Jul 11, 2025

Uh oh!

opensearch-trigger-bot bot commented Jul 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ishaoxy commented Jun 25, 2025 •

edited

Loading

qianheng-aws commented Jun 30, 2025 •

edited

Loading

yuancu Jul 1, 2025 •

edited

Loading

LantaoJin Jul 1, 2025 •

edited

Loading

LantaoJin Jul 1, 2025 •

edited

Loading