merge column: small refactors #2579
Conversation
| block_data: OwnedBytes, | ||
| block_metas: Arc<[BlockMeta]>, | ||
| } | ||
|
|
||
| impl Iterable<u32> for &OptionalIndex { | ||
| fn boxed_iter(&self) -> Box<dyn Iterator<Item = u32> + '_> { | ||
| Box::new(self.iter_rows()) | ||
| Box::new(self.iter_docs()) |
There was a problem hiding this comment.
@PSeitz The reason why columnar was originally not using "docs" but "rows" is because this is a columnar format. The concept of docs is specific to search engine or document DBs.
I am not sure this makes the code any better, but ok.
There was a problem hiding this comment.
It's been some time, but we decided to use docs everywhere. (I can make a full PR later)
The naming is confusing to me, as the meaning of row is different to the meaning in the DB I worked before. The row docs naming also caused a bug before (or 2? not sure).
| } else { | ||
| term_ord_mapping.add_segment(0); | ||
| field_term_streams.push(Streamer::empty()); | ||
| field_term_streams.push(TermsWithSegmentOrd { |
There was a problem hiding this comment.
This is indeed more explicit and easier to proofread.
| @@ -391,7 +394,6 @@ fn is_empty_after_merge( | |||
| fn group_columns_for_merge<'a>( | |||
| columnar_readers: &'a [&'a ColumnarReader], | |||
| required_columns: &'a [(String, ColumnType)], | |||
| _merge_row_order: &'a MergeRowOrder, | |||
There was a problem hiding this comment.
merge order is not relevant to group columns by (string, type category)
Some refactors & tests when investigating a crash in
merge_dict_column