Skip to content
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

Implement XPath regexp function matches #57

Closed
jf-tech opened this issue Nov 21, 2020 · 3 comments
Closed

Implement XPath regexp function matches #57

jf-tech opened this issue Nov 21, 2020 · 3 comments

Comments

@jf-tech
Copy link
Contributor

jf-tech commented Nov 21, 2020

See matches spec at: https://www.w3.org/TR/xpath-functions-31/#func-matches

Note 1: https://www.w3.org/TR/xpath-functions-31/#flags shows all the supported flags, but will only implement flags
's', 'm', 'i' and 'q'. Will not include 'x' due to lack of native support in https://golang.org/pkg/regexp/syntax/.

Note 2 / Question: for perf reason, we want to compile regexp and cache them. Thus we need a caching solution. Options:

  1. Direct use sync.Map and a simple helper function to load/compile regexp.
    Pro: simple, no external dependency;
    Con, at risk of unbounded cache growth, should we have a use case where regexp variety/cardinality is huge.
  2. Use some LRU cache with fixed capacity such as github.com/hashicorp/golang-lru.
    Pro: bounded cache size
    Con: external dependency; slightly slower than sync.Map.

@zhengchun What do you think?

@zhengchun
Copy link
Contributor

@jf-tech , I will vote for the second option that using LRU.

@jf-tech
Copy link
Contributor Author

jf-tech commented Nov 21, 2020

@zhengchun have a second thought - your xpath currently has zero external dep (other than go SDK). Intro a 3rd party dep complicates your compatibility story - xpath runs on go 1.6, 1.9 and 1.10 (which are fairly ancient) and not compatible with most LRU libs. I think I will stick with use built-in constructs (map or sync.Map) with a simple reset if cap is reached. We will see. PR will come tomorrow.

@jf-tech
Copy link
Contributor Author

jf-tech commented Nov 22, 2020

On a third thought ^_^, seems like no need to support flags, anyone wants to use flags can directly implement the flags in the pattern argument, such as (?i) for flag i, (?s) for flag s.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants