[Analytics Engine] Fix IP-field comparison 400 (Unable to convert call EQUALS_IP) on the DataFusion route - #22681
Merged
mch2 merged 2 commits intoAug 10, 2026
Conversation
PPL =, !=, <, > (and if/case/count(eval())) over ip/binary fields on the analytics route failed with HTTP 400 "Unable to convert call EQUALS_IP(binary?, binary?)". ip/binary fields reach the route as VARBINARY, so the SQL plugin selects the EQUALS_IP (and sibling) IP-comparison UDFs over the standard comparators; those UDFs are Enumerable-only, have no Substrait binding, and no backend adapter, so isthmus rejects the call. Add IpComparisonNormalizationAdapter: when a comparison call's operator carries a comparison SqlKind but is not the canonical SqlStdOperatorTable singleton and both operands are VARBINARY, rewrite it to the native comparator (result type preserved). DataFusion then runs its native byte-wise VARBINARY comparison after BinaryFunctionAdapter resolves the string literal. Registered in the six comparison slots, wrapping ComparisonTemporalCoercionAdapter so temporal coercion on those slots is preserved. Mirrors CidrMatchFunctionAdapter. Resolves opensearch-project#22672. Signed-off-by: Kai Huang <ahkcs@amazon.com>
Contributor
PR Reviewer Guide 🔍(Review updated until commit 0a3a594)Here are some key observations to aid the review process:
|
Contributor
|
Persistent review updated to latest commit 0a3a594 |
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22681 +/- ##
============================================
+ Coverage 71.48% 71.53% +0.04%
- Complexity 76960 77045 +85
============================================
Files 6156 6156
Lines 358444 358444
Branches 52246 52246
============================================
+ Hits 256240 256417 +177
+ Misses 81792 81646 -146
+ Partials 20412 20381 -31 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mch2
approved these changes
Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
PPL equality/comparison predicates and projection expressions over
ip-mapped (andbinary-mapped) fields on the analytics-engine DataFusion route fail deterministically with HTTP 400:{ "error": { "reason": "Invalid Query", "details": "Unable to convert call EQUALS_IP(binary?, binary?).", "type": "IllegalArgumentException" }, "status": 400 }This makes
ip/binary=,!=,<,>filters andif/case/count(eval(...))project shapes over those fields unusable on the analytics route, and fails twosandbox-checkCI tests on every run (FieldTypeCoverageIT.testIpFilters,FieldTypeCoverageIT.testIpAndBinaryProjectExpressions).Resolves #22672.
Root cause
ipandbinaryfields reach the analytics route typed asVARBINARY. When the SQL plugin resolves a comparison such aswhere ip_field = '1.2.3.4', itsPPLFuncImpTableregisters two operators per PPL comparator — the IP user-defined function (e.g.EQUALS_IP) first, then the standard Calcite comparator. Because the IP-UDF's(IP_UDT, IP_UDT)operand checker accepts a plainVARBINARYoperand, the UDF overload wins, and the plan carriesEQUALS_IP(binary, binary).EQUALS_IP(and its five siblingsNOT_EQUALS_IP,LESS_IP,GREATER_IP,LTE_IP,GTE_IP) are SQL-pluginImplementorUDFs with an Enumerable (codegen) implementation only. On the analytics route the RelNode is lowered to Substrait, and there is no Substrait binding and no backend adapter for these UDFs, so isthmus rejects the call withUnable to convert call EQUALS_IP(binary?, binary?). The capability gate does not catch it earlier — the UDFs carry a comparisonSqlKind, soScalarFunction.fromSqlOperatorWithFallbackresolves them to the standardEQUALS/LESS_THAN/... which the backend does support; the operator's true identity only surfaces at Substrait conversion.This regressed when
ip/binarygained their UDT representation (#21807): before that,ipwas plainVARBINARY, the IP-UDF operand checker did not match, and the standard comparator was selected — the working path. #21807 shipped adapters forCIDRMATCHand the ip/binary→string casts but not for the six IP comparison UDFs.Fix
A backend adapter (
IpComparisonNormalizationAdapter) that normalizes an IP-comparison UDF call back to its nativeSqlStdOperatorTablecomparator before Substrait conversion, when both operands areVARBINARY. DataFusion then runs its native byte-wiseVARBINARYcomparison — the same path a plain=takes — afterBinaryFunctionAdapterhas resolved the string literal into matching on-disk bytes (adapter recursion runs operands bottom-up, so the literal is alreadyVARBINARYwhen this parent adapter runs). The BOOLEAN result type is preserved, so the plan schema is unchanged.Detection is structural, not by operator identity: the analytics backend cannot reference
PPLBuiltinOperators.EQUALS_IP(it lives in the SQL plugin'scoremodule, off the backend classpath). A call qualifies when its operator carries a comparisonSqlKindbut is not the canonicalSqlStdOperatorTablesingleton for that kind, and both operands areVARBINARY. This mirrorsCidrMatchFunctionAdapter, which already rewrites a UDF into native Calcite comparators overVARBINARY.The adapter is registered in the six comparison-operator slots that previously held
ComparisonTemporalCoercionAdapter, and wraps that adapter — every call (rewritten or not) is still delegated to it, so char↔temporal / TIME coercion on those slots is preserved unchanged (IP operands are never temporal, so the delegation is a no-op for the rewritten calls).Scope note. Only the four direct comparators (
=,!=,<,>) emit the IP UDFs.inandbetweenoverip/binaryalready lower to native operators overVARBINARY(their literals go through the sameBINARY(varchar)placeholder thatBinaryFunctionAdapterresolves), andcidrmatchhas its own adapter — those shapes were never broken. The failing test crashes at its first=assertion, before reaching them.Testing
IpComparisonNormalizationAdapterTests(all 6 IP comparators → native op; native/non-VARBINARY passthrough; VARBINARY guard; type preserved; temporal coercion still delegated)ComparisonTemporalCoercionAdapterTests(regression — behavior preserved through the wrapper)FieldTypeCoverageIT.testIpFilters(=,!=,>,<,in,between,cidrmatch, AND-combination overip)Unable to convert call EQUALS_IPFieldTypeCoverageIT.testIpAndBinaryProjectExpressions(if/case/count(eval())overip=andbinary=)Unable to convert call EQUALS_IPCheck List
--signoffor-s.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.