Skip to content

Conversation

@yuancu
Copy link
Collaborator

@yuancu yuancu commented Oct 21, 2025

Description

Previously, GEOIP function supports only string as its input type for IPs. However, there exists IP type in PPL, and it's counter-intuitive to not supporting it as a valid input type.

This PR makes GEOIP function accept IP input types. Please note that this correction is only valid when Calcite is enabled. V2's behavior remains unchanged.

Related Issues

Resolves #4468

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.

Comment on lines +40 to +41
* <li>(STRING, IP) -> MAP
* <li>(STRING, IP, STRING) -> MAP
Copy link
Member

Choose a reason for hiding this comment

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

should be following ? Seems not a breaking change from the ip.rst, but why the description here is different.

 *.  <li>(STRING, STRING) -> MAP
 *   <li>(STRING, STRING, STRING) -> MAP
 *   <li>(STRING, IP) -> MAP
 *   <li>(STRING, IP, STRING) -> MAP

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now I modified the underlying function to accept IP only.

It still accepts strings as inputs with implicit type coercion.

If it gets string as input, it will try to cast it to IP first. This guarantees that the input is a valid IP address.

@yuancu yuancu added the bug Something isn't working label Oct 21, 2025
@yuancu yuancu merged commit bdd42bc into opensearch-project:main Oct 21, 2025
67 of 70 checks passed
@yuancu yuancu deleted the issues/4468 branch October 21, 2025 13:39
@opensearch-trigger-bot
Copy link
Contributor

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

yuancu added a commit to yuancu/sql-plugin that referenced this pull request Oct 22, 2025
LantaoJin pushed a commit that referenced this pull request Oct 23, 2025
(cherry picked from commit bdd42bc)

Signed-off-by: Yuanchun Shen <[email protected]>
@LantaoJin LantaoJin added the backport-manually Filed a PR to backport manually. label Oct 23, 2025
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
  Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
  Make nested alias type support referring to outer context (opensearch-project#4673)
  Update big5 ppl queries and check plans (opensearch-project#4668)
  Support push down sort after limit (opensearch-project#4657)
  Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
  Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
  Fix bin nested fields issue (opensearch-project#4606)
  Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
  Pushdown sort aggregate metrics (opensearch-project#4603)
  Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
  Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
  Allow renaming group-by fields to existing field names (opensearch-project#4586)
  Publish internal modules separately for downstream reuse (opensearch-project#4484)
  Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
  Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
  Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
  Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
  Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
  Pushdown case function in aggregations as range queries (opensearch-project#4400)
  Update GEOIP function to support IP types as input (opensearch-project#4613)
  ...

# Conflicts:
#	docs/user/ppl/functions/conversion.rst
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 28, 2025
* default-main: (34 commits)
  Enhance dynamic source clause to support only metadata filters (opensearch-project#4554)
  Make nested alias type support referring to outer context (opensearch-project#4673)
  Update big5 ppl queries and check plans (opensearch-project#4668)
  Support push down sort after limit (opensearch-project#4657)
  Use table scan rowType in filter pushdown could fix rename issue (opensearch-project#4670)
  Fix: Support Alias Fields in MIN, MAX, FIRST, LAST, and TAKE Aggregations (opensearch-project#4621)
  Fix bin nested fields issue (opensearch-project#4606)
  Add `per_minute`, `per_hour`, `per_day` function support (opensearch-project#4531)
  Pushdown sort aggregate metrics (opensearch-project#4603)
  Followup: Change ComparableLinkedHashMap to compare Key than Value (opensearch-project#4648)
  Mitigate the CI failure caused by 500 Internal Server Error (opensearch-project#4646)
  Allow renaming group-by fields to existing field names (opensearch-project#4586)
  Publish internal modules separately for downstream reuse (opensearch-project#4484)
  Revert "Update grammar files and developer guide (opensearch-project#4301)" (opensearch-project#4643)
  Support Automatic Type Conversion for REX/SPATH/PARSE Command Extractions (opensearch-project#4599)
  Replace all dots in fields of table scan's PhysType (opensearch-project#4633)
  Return comparable LinkedHashMap in `valueForCalcite()` of ExprTupleValue (opensearch-project#4629)
  Refactor JsonExtractAllFunctionIT and MapConcatFunctionIT (opensearch-project#4623)
  Pushdown case function in aggregations as range queries (opensearch-project#4400)
  Update GEOIP function to support IP types as input (opensearch-project#4613)
  ...

Signed-off-by: Asif Bashar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19-dev backport-failed backport-manually Filed a PR to backport manually. bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] geoip() function fails with IP type field reference

3 participants