[Enhancement] Error handling for illegal character usage in java regex named capture group

[RyanL1997]

Description

[Enhancement] Error handling for underscore usage in rex regex

• Better handling for invalid named capture group
• Updated the doc
• Add ITs

Related Issue

• Resolve [FEATURE] Support _/- as parsed field name #3944
• Resolve [Enhancement] Error handling for unsupported characters in java regex library #4467
• Relate to all the named capture group extraction related command (parse/rex/replace...)

Behavior after enhancement

# rex command
curl -X POST "localhost:9200/_plugins/_ppl" \
    -H "Content-Type: application/json" \
    -d '{
      "query": "source=accounts | rex field=email \".+@(?<domain_name>.+)\" | fields email, domain_name"
}'

{
  "error": {
    "reason": "Invalid Query",
    "details": "Invalid capture group name 'domain_name'. Java regex group names must start with a letter and contain only letters and digits.",
    "type": "IllegalArgumentException"
  },
  "status": 400
}


# parse command
curl -X POST "localhost:9200/_plugins/_ppl" \
  -H "Content-Type: application/json" \
  -d '{
    "query": "source=accounts | parse email \".+@(?<domain-name>.+)\" | fields email, domain-name | head 5"
}'

{
  "error": {
    "reason": "Invalid Query",
    "details": "Invalid capture group name 'domain-name'. Java regex group names must start with a letter and contain only letters and digits.",
    "type": "IllegalArgumentException"
  },
  "status": 400
}% 

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• 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.

[RyanL1997]

I think same as issue #3944
If yes, add a yamlRestTest to verify the issued fixed.

Hi @penghuo, good callout. I was actually taking a look for the - and ended up that is another limitation from java regex library. So rechecked and in summary:

Java Regex Named Group Character Rules

According to Java's Pattern class documentation, named capture groups follow these rules:

Valid Characters:

• Must start with: Letter (a-z, A-Z)
• Can contain: Letters (a-z, A-Z) and digits (0-9)

Invalid Characters (will cause PatternSyntaxException):

• Underscore (_) - This is what we're handling with our validation
• Hyphen/Dash (-) - This is what you tested with curl
• Period/Dot (.)
• Space ( )
• Special characters (@, #, $, %, ^, &, *, etc.)
• Unicode characters beyond ASCII
• Cannot start with digit (0-9)

Java Documentation:

The official Java documentation for this is in:

• Java Pattern class: java.util.regex.Pattern
• Specific section: "Named-capturing group" in the Pattern syntax documentation
• Rule: Named groups use the syntax (?X) where name must be a valid Java identifier but with more restrictions

The exact Java rule is:

  // From Java's Pattern class source code:
  // Named group name must match: [a-zA-Z][a-zA-Z0-9]*

Quick Test Examples:

# These will FAIL in Java regex:
(?<domain_name>...)  # underscore
(?<domain-name>...)  # hyphen  
(?<domain.name>...)  # dot
(?<domain name>...)  # space
(?<@domain>...)      # special char
(?<1domain>...)      # starts with digit

# These will WORK in Java regex:
(?<domain>...)       # letters only
(?<domainName>...)   # camelCase
(?<domain1>...)      # letters + numbers
(?<DOMAIN>...)       # uppercase

[RyanL1997]

According to the above, since the limitation is coming from the upstream library we are using, I just unified the error msg to handle all the illegal character cases in extraction commands like rex and parse and for the above parse related issue I also left a comment of work around by using rename command: #3944 (comment).

cc @penghuo

[ykmr1224]

Technically we should be able to support the invalid characters by rewriting regex and map extracted values back to original name like:

1. original pattern: .+@(?<domain_name>.+)
2. rewritten pattern: .+@(?<domainname>.+) (maintain mapping domainname -> domain_name internally)
3. assign extracted value: domain_name = extracted['domainname']

This could cause name overwrap, but we can workaround by adding suffix, etc. to come up with unique name.

[RyanL1997]

Hi @ykmr1224 , thanks for the input. And that was an interesting approach and I was actually did a PoC of your suggestion on my local. However, I do find some limitations of this approach:

• If the renaming of the pattern is just removing the unsupported character, what if the renamed pattern after removal is duplicate with a existing field? (e.g. the index field contains a username, user would like to extract the alias of the email (xyz@email.com -> xyz) as a column called user_name and in the above logic it will be renamed as username which is conflict with the index field.
• Stack traces and error messages refer to rewritten names (e.g. in explain) , which I think will cause the confusions for users.
• I haven't try it on any of the benchmarking dataset yet for the re-writing approach, but from the time complexity wise
  • Current approach: O(1) per validation
  • Rewriting approach: O(k) where k=number of named groups (for mapping tables as we are re-writing the column names)

According to the above, I think we can keep as the current limitation handling approach for now, if there are some feature request coming from the users we can think about actually supporting these characters as a new feature requests. But if that day comes, I truly think the correct approach is to find a PCRE style library for regex patterns.

[ykmr1224]

@RyanL1997
Thanks for PoCing.
In case of name overwrap, we can add suffix like username1, and increment in case it still overwrap. (it will eventually find unique name)
i.e: (?<user_name>.+)(?<username>.+)(?<username1>.+) => (?<username2>.+)(?<username>(?<username1>.+), mapping = {username2 => user_name, username => username, username1 => username1}

Computation cost for regex rewrite should be ignorable since this happens only once during query analysis. And rename after extraction should be just an alias or projection (should not affect performance in my understanding).

It can be separate task from this PR, btw.
And I think we want to quickly check the possible usage of this use case to see the priority.

[RyanL1997]

It can be separate task from this PR, btw.
And I think we want to quickly check the possible usage of this use case to see the priority.

Transferring some internal communication over here - since the above issue is not blocking this change. This change is ready to be merged and I have created a issue #4549 to discover the feasibility of the above suggestion.