-
Notifications
You must be signed in to change notification settings - Fork 141
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
#3145 Add IP Address Data Type #3175
base: main
Are you sure you want to change the base?
Conversation
@currantw can we test an IP range query as well? |
Woops! Didn't mean to close this... 😬 |
@currantw can u plz resolve conflicts ? |
cd9f94b
to
201697b
Compare
I have added a test to Let me know which additional tests you think may be needed. I didn't find any existing integration tests for comparisons -- perhaps this is something that should be added? I'm not sure exactly what you mean by "in range" operations: can you give an example query? |
@currantw LGTM 👍 |
@currantw can u plz check the failed CI plz ? |
9cbc60b
to
3f9e428
Compare
Signed-off-by: currantw <[email protected]>
…g unused sorting syntax. Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
…s` test data. Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
3f9e428
to
b45114a
Compare
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
: (PLUS | MINUS)? sortFieldExpression | ||
; | ||
|
||
sortFieldExpression |
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.
@currantw was this sortFieldExpression
used in a query anywhere ?
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 is used in sortField
, which is used by Trendline
and SORT BY
. @currantw we sure we want to include these changes?
public class ExprIpValue extends AbstractExprValue { | ||
private final IPAddress value; | ||
|
||
public ExprIpValue(String s) { |
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.
nit:
public ExprIpValue(String s) { | |
public ExprIpValue(String addressStr) { |
} | ||
|
||
return ExprValueUtils.booleanValue(range.contains(address)); | ||
return ExprValueUtils.LITERAL_TRUE; |
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.
nit:
return (IPUtils.compare(address, range.getLower()) < 0) || (IPUtils.compare(address, range.getUpper()) > 0) ? ExprValueUtils.LITERAL_FALSE : ExprValueUtils.LITERAL_TRUE;
private static DefaultFunctionResolver castToIp() { | ||
return FunctionDSL.define( | ||
BuiltinFunctionName.CAST_TO_IP.getName(), | ||
impl(nullMissingHandling((v) -> v), IP, IP), |
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.
ordering matters - make sure to but the naive cast last
} | ||
|
||
private static final double TOLERANCE = 1E-5; |
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.
why did you move this?
void test_non_date_field_type() { | ||
String dateString = "2021-11-08"; | ||
void test_string_field_type() { | ||
String dateString = "STRING"; |
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.
revert?
AUTO: 'AUTO'; | ||
STR: 'STR'; | ||
IP: 'IP'; | ||
NUM: 'NUM'; |
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.
revert?
: (PLUS | MINUS)? sortFieldExpression | ||
; | ||
|
||
sortFieldExpression |
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 is used in sortField
, which is used by Trendline
and SORT BY
. @currantw we sure we want to include these changes?
Signed-off-by: currantw [email protected]
Description
#3145 Add IP Address Data Type
Related Issues
Resolves #3145
#3036
Check List
API changes companion pull request created.--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.