ESQL: TO_IP can handle leading zeros #126294
Conversation
|
Hi @nik9000, I've created a changelog YAML for you. |
| @@ -33,6 +33,7 @@ base { | |||
| dependencies { | |||
| compileOnly project(path: xpackModule('core')) | |||
| compileOnly project(':modules:lang-painless:spi') | |||
| compileOnly project(':libs:simdvec') | |||
| ) }, | ||
| description = "(Optional) Additional options.", | ||
| optional = true | ||
| ) Expression options |
There was a problem hiding this comment.
@fang-xing-esql could you tell me if this is a sane idea? Or should I just make two more functions instead?
There was a problem hiding this comment.
@fang-xing-esql could you tell me if this is a sane idea? Or should I just make two more functions instead?
It looks fine to me to extend to_ip, instead of making more functions. :)
| public class ParseIp { | ||
| private static final byte[] IPV4_PREFIX = new byte[] { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -1, -1 }; | ||
|
|
||
| @ConvertEvaluator(extraName = "LeadingZerosRejected", warnExceptions = { IllegalArgumentException.class }) |
There was a problem hiding this comment.
I'm going to extract these to a separate PR because the leadingZerosRejected version can be used even in older versions of Elasticsearch. I can backport that PR to 9.0 and 8.19.
There was a problem hiding this comment.
Thank you @nik9000! I left a comment related to the format of entries in a MapExpression.
| return new TypeResolution("map keys must be strings"); | ||
| } | ||
| if (e.key() instanceof Literal keyl) { | ||
| key = (BytesRef) keyl.value(); |
There was a problem hiding this comment.
The key and value of an entry in the MapExpression can be either a String or BytesRef, depending on when resolveType() is called. If it is called by Analyzer, they are in (human readable)String format, if it is called by LogicalPlanOptimizer, they might be in BytesRef.
Having some CsvTests should expose the different format, after the interface of to_ip is finalized.
|
Superseded by #126532 |
Modifies TO_IP so it can handle leading
0s in ipv4s. It's not done, but the goal is to keep the current behavior:But to allow
I didn't manage to plug in the map arguments properly - and I'm not entirely sure this is the right thing to do with
TO_IPanyway. So this is a draft. There's some change we say that we needTO_IP_LEADING_ZEROS_OCTALandTO_IP_LEADING_ZEROS_DECIMALinstead of using named parameters. That's becauseTO_IPis a conversion function which is sort of defined as being unary - and adding theoptionsto it breaks that contract.