Simpler float index conv., xsd:double fix #196 #199
Conversation
In another change we may want to also restore if the original value was xsd:double or xsd:decimal
This is similar to our previous treatment of xsd:integer vs xsd:float and allows us to filter on decimals and doubles while retrieving them with the correct type information instead of just treating them as aliases for xsd:float NOTE: This requires an index rebuild!
|
Ok I've just tested this on Wikidata Full and with the queries from #196 and it works really well (after previously running the wrong version on the new index). @floriankramer or @joka921 can you give this a quick look? |
| @@ -226,8 +226,7 @@ void processGroup(const GroupBy::Aggregate& a, size_t blockStart, | |||
| } else { | |||
| // Remove the trailing character indicating if the value | |||
| // is an integer or a float. | |||
| res += ad_utility::convertIndexWordToFloat( | |||
| entity.substr(0, entity.size() - 1)); | |||
| res += ad_utility::convertIndexWordToFloat(entity); | |||
There was a problem hiding this comment.
Given that we no longer remove the trailing character, we should probably update the comment.
src/util/Conversions.h
Outdated
| DECIMAL = 'T' | ||
| }; | ||
|
|
||
| //! Converts like "12.34" to :v:float:PP0*2E0*1234F and -0.123 to |
There was a problem hiding this comment.
Converts like -> Converts strings like
src/util/Conversions.h
Outdated
| inline string convertFloatStringToIndexWord( | ||
| const string& value, const NumericType type = NumericType::FLOAT); | ||
|
|
||
| //! Converts like this: :v:float:PP0*2E0*1234F to "12.34" and |
There was a problem hiding this comment.
Converts like -> Converts strings like
src/util/Conversions.h
Outdated
| //! by convertNumericToIndexWord() and convertValueLiteralToIndexWord() | ||
| //! as these add :v:<type>: in front | ||
| //! TODO(schnelle): Rename this to reflect the above | ||
| //! Converts like this: ":v:float:PP0*2E0*1234F to "12.34 and |
There was a problem hiding this comment.
Converts like -> Converts strings like
src/util/Conversions.h
Outdated
| @@ -159,16 +170,25 @@ string convertIndexWordToValueLiteral(const string& indexWord) { | |||
| if (startsWith(indexWord, VALUE_FLOAT_PREFIX)) { | |||
| if (endsWith(indexWord, "F")) { | |||
There was a problem hiding this comment.
If we use indexWord.back() (which is save as long as VALUE_FLOAT_PREFIX is non empty) we could use the new NumericType enumeration for the comparison here.
| @@ -246,8 +265,9 @@ string convertFloatToIndexWord(const string& orig, size_t nofExponentDigits, | |||
| } | |||
|
|
|||
| // Add padding to the exponent; | |||
| AD_CHECK_GT(nofExponentDigits, expoString.size()); | |||
| size_t nofPaddingElems = nofExponentDigits - expoString.size(); | |||
| AD_CHECK_GT(DEFAULT_NOF_VALUE_EXPONENT_DIGITS, expoString.size()); | |||
There was a problem hiding this comment.
Given that we no longer override its value we could remove the DEFAULT from DEFAULT_NOF_VALUE_EXPONENT_DIGITS
Also addresses a few other nitpicks from the review
This is WIP, we may want to also restore if the original value was
xsd:double or xsd:decimal