fix: ts range filter bug #220
Conversation
| if filter.Equals != nil { | ||
| equality = fmt.Sprintf(`"%s" = ?`, filter.SortKeyName) | ||
| rangeParams = append(rangeParams, filter.Equals) | ||
| paramVal := convertTimestampParam(filter.Equals) |
There was a problem hiding this comment.
This would convert normal Int64 values to timestamps as well. Why not just do this data conversion in typeconversion.go?
There was a problem hiding this comment.
I think we would need to know the expected type of the sortkey to know what to convert it to. We pass that into the request. For example,
"sort_key_filters": [ { "sort_key_name": "feature_5", "range": { "range_end": { "double_val": "60" }, "end_inclusive": true } } ]
However, we don't pass this to the onlinereadrange method. My proposed solution would be to add a ValueType field to the SortKeyFilter struct, so that we can know the expected type of it upon a request. Then I'd add a method in the type conversion package that translates the request type to the data type cassandra expects (based on the Valuetype of the SortKeyFilter).
I want to get your input on this before implementation in case there is a more simple solution I'm not considering.
There was a problem hiding this comment.
We already validate that the sort key filter values are the correct type. Since both the proto and go are strictly typed there shouldn't be any need to explicitly pass the type information.
There was a problem hiding this comment.
But how can the OnlineReadRange method know what the correct type is? I think it needs to know that prior to constructing the cql statement, otherwise it won't be able to know if an int64 should be converted to an int64 or to unix timestamp.
What this PR does / why we need it:
OnlineReadRange method was previously passing raw filter value to the cql query statement. This fix converts the timestamp value to its go type prior to constructing the cql statement.
Which issue(s) this PR fixes:
Misc