Lexicographical comparison for string queries #7273
Conversation
Pull Request Test Coverage Report for Build james.stone_480
💛 - Coveralls |
|
There is a discrepancy between how nulls behave in this and the way it works in the string based queries; converting this to a draft until I investigate further. |
ironage
force-pushed
the
js/query-string-lex
branch
from
070690f to
23d5873
Compare
src/realm/query_engine.hpp
Outdated
| if (cond(StringData(m_value), m_ucase.c_str(), m_lcase.c_str(), t)) | ||
| return s; | ||
| if constexpr (case_sensitive_comparison) { | ||
| // case insensitive not implemented for these comparisons |
There was a problem hiding this comment.
nit: probably a better comment here is needed, we are talking about string lexicographical comparisons, right?
There was a problem hiding this comment.
right, updated the comment to be specific
| @@ -1545,9 +1544,14 @@ class StringNodeBase : public ParentNode { | |||
| template <class TConditionFunction> | |||
| class StringNode : public StringNodeBase { | |||
| public: | |||
| constexpr static bool case_sensitive_comparison = | |||
There was a problem hiding this comment.
nit: I guess you want this constexpr, available at compile time, but do you need this to be static as well, in order to be used like this StringNode::case_sensitive_comparison?
There was a problem hiding this comment.
it is a compile error to remove the static on a constexpr member
There was a problem hiding this comment.
I was more thinking whether you needed it as public member, or you could just have a constexpr variable defined in the method.
There was a problem hiding this comment.
Oh I see. Well I suppose it doesn't need to be public, but I'd like it at the class scope because I use it for a small optimization in the constructor as well.
This feature was requested by realm/realm-swift#8008.
I also added a small performance improvement for string queries; no need to always convert the constant
std::optional<std::string>to aStringDatain the hot query path, it can be done once in the constructor of the query node.☑️ ToDos
bindgen/spec.yml, if public C++ API changed