-
Notifications
You must be signed in to change notification settings - Fork 178
issue #4514 tonumber function as part of roadmap #4287 #4605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
issue #4514 tonumber function as part of roadmap #4287 #4605
Conversation
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
… default duration is in seconds which will be used for duration format , added cast call for tostring with 1 argument Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
…ensearch-project#4287 Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
| void testToNumberWithDecimalBase16() { | ||
| assertEquals(255.5, ToNumberFunction.toNumber("FF.8", 16)); | ||
| assertEquals(16.25, ToNumberFunction.toNumber("10.4", 16)); | ||
| assertEquals(171.6875, ToNumberFunction.toNumber("AB.B", 16)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any standard we are following?
Seems SPL does not support tonumber("AB.B", 16)
If not, we should define the behaviour of tonumber as
- if base=10, convert to int / double, depend on whether there is
. - if base!=10, always convert to long, e.g.
new BigInteger("FF", 16).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made the changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@asifabashar what is change?
I still see test
@Test
void testToNumberWithDecimalBase16() {
assertEquals(255L, ToNumberFunction.toNumber("FF.8", 16));
assertEquals(16L, ToNumberFunction.toNumber("10.4", 16));
assertEquals(171L, ToNumberFunction.toNumber("AB.B", 16));
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, fixed it now
Signed-off-by: Asif Bashar <[email protected]>
|
@penghuo Please review. |
|
Please fix IT. |
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
|
Please help set PR label. |
Signed-off-by: Asif Bashar <[email protected]>
Signed-off-by: Asif Bashar <[email protected]>
core/src/main/java/org/opensearch/sql/expression/function/udf/ToNumberFunction.java
Show resolved
Hide resolved
Signed-off-by: Asif Bashar <[email protected]>
core/src/main/java/org/opensearch/sql/expression/function/udf/ToNumberFunction.java
Show resolved
Hide resolved
Signed-off-by: Asif Bashar <[email protected]>
a745afb to
804db79
Compare
Signed-off-by: Asif Bashar <[email protected]>
|
Looks good to me, Please merge conflict. @dai-chen , please take another look. |
Signed-off-by: Asif Bashar <[email protected]>
| os> source=people | eval int_value = tonumber('010101',2) | fields int_value | head 1 | ||
| fetched rows / total rows = 1/1 | ||
| +-----------+ | ||
| | int_value | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is int_value an integer or double?
|
|
||
| The following usage options are available, depending on the parameter types and the number of parameters. | ||
|
|
||
| Usage: tonumber(string, [base]) converts the value in first argument to provided base type string in second argument. If second argument is not provided, then it converts to base 10 number representation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not converting 1st arg to provided base type in 2nd arg, right? The 2nd arg describe the base of 1st arg instead?
| public Expression implement( | ||
| RexToLixTranslator translator, RexCall call, List<Expression> translatedOperands) { | ||
| Expression fieldValue = translatedOperands.get(0); | ||
| int base = 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useless?
| result = bigInteger.longValue(); | ||
| } | ||
| } catch (Exception e) { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this by design?
| import org.apache.calcite.test.CalciteAssert; | ||
| import org.junit.Test; | ||
|
|
||
| public class CalcitePPLToNumberFunctionTest extends CalcitePPLAbstractTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No IT? I think we need to also cover NULL and error case for invalid input.
Description
tonumber(, )
This function converts a string to a number.
Usage
You can use this function with the eval and where commands, in the WHERE clause of the from command, and as part of evaluation expressions with other commands.
The argument can be a string or the name of a field that contains a string. If the string contains a decimal point ( . ), then the tonumber function converts it to a double. Otherwise, the function converts the string to an integer.
Be aware that integers are supported differently in different product contexts:
The argument is optional. It defines the base of the number in the argument. It defaults to 10, which corresponds to the decimal system. You can set to a number between 2 and 36, inclusive.
If the tonumber function cannot parse a literal string to a number, the function returns an error.
Basic examples
The following example converts the string values from the store_sales field to numbers, and then stores the numbers in a field named n. This example uses the default base of 10.
... | eval n=tonumber(store_sales)
The following example takes the hexadecimal number and uses a base of 16 to return the number "164".
... | eval n=tonumber("0A4",16)
Resolves #4514
Related Issues
Resolves #4514
Check List
--signoffor-s.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.