Fix binning udf resolution / Add type coercion support for binning UDFs #4742
Conversation
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
ahkcs
force-pushed
the
fix-binning-udf-resolution-4740
branch
3 times, most recently
from
1adc46f to
2c662ed
Compare
ahkcs
requested review from
GumpacG,
LantaoJin,
MaxKsyunz,
RyanL1997,
Swiddis,
YANG-DB,
Yury-Fridlyand,
acarbonetto,
anirudha,
dai-chen,
derek-ho,
forestmvey,
joshuali925,
kavithacm,
mengweieric,
noCharger,
penghuo,
ps48,
qianheng-aws,
seankao-az,
vamsimanohar,
ykmr1224 and
yuancu
as code owners
Collaborator
|
@ahkcs CI is failing. I retried but still failed. Could you double check? |
Signed-off-by: Kai Huang <ahkcs@amazon.com>
Signed-off-by: Kai Huang <ahkcs@amazon.com>
Signed-off-by: Kai Huang <ahkcs@amazon.com>
Signed-off-by: Kai Huang <ahkcs@amazon.com>
Signed-off-by: Kai Huang <ahkcs@amazon.com>
ahkcs
force-pushed
the
fix-binning-udf-resolution-4740
branch
from
3ae31ab to
a93dcf8
Compare
Contributor
Author
Unit tests passed after rebase |
ahkcs
changed the title
Signed-off-by: Kai Huang <ahkcs@amazon.com>
ahkcs
force-pushed
the
fix-binning-udf-resolution-4740
branch
from
c2d3dd0 to
d0ddb19
Compare
Signed-off-by: Kai Huang <ahkcs@amazon.com>
Signed-off-by: Kai Huang <ahkcs@amazon.com>
penghuo
previously approved these changes
Nov 7, 2025
penghuo
requested changes
Nov 7, 2025
docs/user/ppl/cmd/bin.rst
Outdated
| Example 20: Type coercion with string fields | ||
| ============================================== | ||
|
|
||
| The ``bin`` command automatically converts string fields containing valid numeric values to numeric types, enabling binning operations on keyword/text fields. Only strings that represent valid numbers (e.g., "25", "30.5", "100") are supported. Non-numeric strings will cause an error. |
Contributor
Author
There was a problem hiding this comment.
Updated to remove comments
docs/user/ppl/cmd/bin.rst
Outdated
| bin <field> [span=<interval>] [minspan=<interval>] [bins=<count>] [aligntime=(earliest | latest | <time-specifier>)] [start=<value>] [end=<value>] | ||
|
|
||
| * field: mandatory. The numeric field to bin. | ||
| * field: mandatory. The field to bin. Accepts numeric fields, time-based fields, or string fields containing valid numeric values (e.g., "25", "30.5"). String fields with numeric values are automatically coerced to numeric type. Non-numeric strings will result in an error. |
Collaborator
There was a problem hiding this comment.
I suggest remove this section.
Type conversion is a generic feature applied across all commands and functions, so it does not need to be explicitly mentioned in each one.
Additionally, enabling binning operations should target string-type fields, not keyword or text fields specifically.
Contributor
Author
There was a problem hiding this comment.
Updated to remove this section
Comment on lines
78
to
80
| throw new IllegalArgumentException( | ||
| "Cannot apply binning: field contains non-numeric string values. " | ||
| + "Only numeric types or string types with valid numeric values are supported."); |
Collaborator
There was a problem hiding this comment.
It should return a null value. For example, if only one document contains an invalid field, the entire query should not fail.
Contributor
Author
There was a problem hiding this comment.
I think it makes sense, I have updated to remove the validation logic. I kept the validation logic in BinnableField to only verify if it's supported type(Reject truly unsupported types (e.g., BOOLEAN, ARRAY, MAP))
dai-chen
approved these changes
Nov 11, 2025
penghuo
approved these changes
Nov 11, 2025
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Nov 11, 2025
…Fs (#4742) * Fix binning udf resolution Signed-off-by: Kai Huang <ahkcs@amazon.com> * spotless fix Signed-off-by: Kai Huang <ahkcs@amazon.com> * update tes Signed-off-by: Kai Huang <ahkcs@amazon.com> * add comments Signed-off-by: Kai Huang <ahkcs@amazon.com> * removal Signed-off-by: Kai Huang <ahkcs@amazon.com> * update yaml test Signed-off-by: Kai Huang <ahkcs@amazon.com> * rerun Signed-off-by: Kai Huang <ahkcs@amazon.com> * update support for type coercion Signed-off-by: Kai Huang <ahkcs@amazon.com> * update doc Signed-off-by: Kai Huang <ahkcs@amazon.com> * enhance error handling and doc Signed-off-by: Kai Huang <ahkcs@amazon.com> * fixes Signed-off-by: Kai Huang <ahkcs@amazon.com> * fixes Signed-off-by: Kai Huang <ahkcs@amazon.com> * fixes Signed-off-by: Kai Huang <ahkcs@amazon.com> * Fix tests Signed-off-by: Kai Huang <ahkcs@amazon.com> --------- Signed-off-by: Kai Huang <ahkcs@amazon.com> (cherry picked from commit 20f2234) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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
Fixes #4740 - Binning UDFs are now resolved through
PPLFuncImpTable.resolve()instead of being instantiated directly viamakeCall().Implemented automatic type coercion to allow string fields with numeric values to work seamlessly with all binning operations (bins, span, minspan, range).
Related Issues
PPLFuncImpTable#4740Type coercion support for binning UDFs #4356