Add String datatype to stage2 #68
Conversation
implements #83
ellmau
added
enhancement
| Self::Item::Float(_) => None, | ||
| Self::Item::Double(_) => None, | ||
| Self::Item::String(_) => None, | ||
| }, | ||
| Self::Float(cs) => match value { | ||
| Self::Item::U64(_val) => None, | ||
| Self::Item::U64(_) => None, | ||
| Self::Item::Float(val) => cs.seek(val).map(DataValueT::Float), | ||
| Self::Item::Double(_val) => None, | ||
| Self::Item::Double(_) => None, | ||
| Self::Item::String(_) => None, | ||
| }, | ||
| Self::Double(cs) => match value { | ||
| Self::Item::U64(_val) => None, | ||
| Self::Item::Float(_val) => None, | ||
| Self::Item::U64(_) => None, | ||
| Self::Item::Float(_) => None, | ||
| Self::Item::Double(val) => cs.seek(val).map(DataValueT::Double), | ||
| Self::Item::String(_) => None, | ||
| }, | ||
| Self::String(cs) => match value { | ||
| Self::Item::String(val) => cs.seek(val).map(DataValueT::String), | ||
| _ => None, // no type mixing allowed, so in any other case it should be [None] |
There was a problem hiding this comment.
By changing _val to _ the memory footprint is reduced, as _val needs to stay on the stack while still in the current scope. The memory of _ is freed instantly.
| @@ -58,6 +58,7 @@ impl<'a> TrieUnion<'a> { | |||
| DataTypeName::U64 => init_scans_for_datatype!(U64, u64), | |||
| DataTypeName::Float => init_scans_for_datatype!(Float, Float), | |||
| DataTypeName::Double => init_scans_for_datatype!(Double, Double), | |||
| DataTypeName::String => init_scans_for_datatype!(String, usize), | |||
There was a problem hiding this comment.
It might be helpful if each DataTypeName knows its internal interpretation, so this information can be extracted "automatically" if it is needed more often.
I'm not too happy with this. On 64-bit architectures, |
I see your concern. It is possible to refactor the |
|
|
Apart from this discussion on |
Thank you for the thoughtful review, did all the changes except the |
It is still something to consider when using indices, as indices are always |
Adding
Stringas a variant toDataTypeNameand follow up all the necessary adaptions to promote the new datatype to all the implementations.To avoid datatype conversions and additional checks, the new datatype uses
usizeinternally to have the same datatype and range as it is provided by the currently implemented dictionaries.TODO: add a printer-object such that the internal numbers can be translated back to the original String. (This will probably be a new issue after this PR is accepted)
implements #63