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

Add support for = ContactQL operator on date and numeric fields #224

Closed
nicpottier opened this issue Jan 28, 2020 · 6 comments
Closed

Add support for = ContactQL operator on date and numeric fields #224

nicpottier opened this issue Jan 28, 2020 · 6 comments
Labels
enhancement New feature or request

Comments

@nicpottier
Copy link
Member

nicpottier commented Jan 28, 2020

So looks like for numeric fields we are using a scaled_float with a scale factor of 10,000, so basically a Decimal with 5 digits of precision:
https://github.com/nyaruka/rp-indexer/blob/master/indexer.go#L499

So should be fine for equality with the gotcha that non-elastic searching will need to evaluate equality to only 5 decimal places. (at least to my reading, we should try some queries to check)
https://www.elastic.co/guide/en/elasticsearch/reference/master/number.html

Dates are, well, date:
https://www.elastic.co/guide/en/elasticsearch/reference/master/date.html

That said we probably want to play with these as I think perhaps you can only do range queries on both of these:
https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-range-query.html

Which doesn't support an eq operator. So I think we will need to rewrite these somehow to be an and with a gte and lt.

For numbers I suppose we can just add the smallest value based on our precision (.00001) and for dates a second? Although I don't think we ever add dates without times set, the date field in Elastic does have time as optional so may need to figure out what comparison does between a date with a time set and one without.

cc @rowanseymour

@nicpottier
Copy link
Member Author

For dates specifically we could use the precision of the operand as a way of allowing "day" specific queries. IE, if you say created_on = "2020-01-20" then that would match anything on the 20th. Though we'd have to write the query in such a way that timezone is accounted for, IE, the gte and lt would likely not be only on the 20th.

@rowanseymour
Copy link
Contributor

For numbers = already works (how?) but what about != ?

It should be straightforward enough to make goflow do comparisons with at most 5 decimal places of accuracy... I think that avoids having to add an error margin.

For dates, we did have it that it was an error to use a value with time information: nyaruka/rapidpro#2453 but that job would now fall to the goflow parser if we want to maintain that behavior.

Evaluation in python/goflow has been by converting a date to a range of UTC timestamps and then matching things in that range. Seems like that would work in ES too?

@nicpottier
Copy link
Member Author

It's amazing what we keep discovering!

Yes, turns out the Mailroom side does support = for numbers:
https://github.com/nyaruka/mailroom/blob/master/search/search.go#L184

using a match query:
https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-match-query.html

Would be interesting to see if foo = 5.0 matches the same as foo = 5 for a foo numeric field, so should experiment with that. But at first blush see no reason why we can't add != there easily.

And sure enough it looks like we are doing the same kind of range thing on dates and =, even taking into account timezones:
https://github.com/nyaruka/mailroom/blob/master/search/search.go#L207

So ya, maybe we just need to add != versions of these?

@nicpottier
Copy link
Member Author

Can confirm that age=42 is the same as age=42.0

@nicpottier
Copy link
Member Author

I guess last open question is what is the parser doing with dates on the right hand side? Does that need to be only date-level accuracy? (that seems like a reasonable start)

@nicpottier
Copy link
Member Author

(oh see your comment about that, ok, then I think that's all we need)

nicpottier added a commit that referenced this issue Feb 7, 2020
add != operator for numbers, dates, created_on, fixes #224
rasoro pushed a commit to Ilhasoft/mailroom that referenced this issue Mar 10, 2023
Queue welcome message event to be handle by mailroom
rasoro pushed a commit to Ilhasoft/mailroom that referenced this issue Dec 16, 2024
Remove no longer used contact/resolve endpoint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants