Fix support for empty strings for Dictionary and RLE encodings

[ypatia]

This is a problem found in VCF by @gspowley where if using a string dimension compressed with a dictionary filter, we get an error if the value of the string dimension is always an empty string ("") :

terminate called after throwing an instance of 'tiledb::TileDBError'
  what():  [TileDB::Task] Error: Caught std::exception: Failed decompressing dictionary-encoded strings; empty input arguments.

The failure would not occur when reading an array that was written using a string dimension data buffer that contained N entries with value 0, but only when writing using an empty string dimension data buffer .

I also added support for encoding empty strings with RLE by removing a check (this was previously done for Dictionary in https://github.com/TileDB-Inc/TileDB/pull/3493/files .

Note that in order to fix this issue I had to correct the on disk serialization of dictionaries, so the fix won't work for existing arrays written in the so far non-working way.

---

TYPE: BUG
DESC: Fix support for empty strings for Dictionary and RLE encodings

[shortcut-integration]

This pull request has been linked to Shortcut Story #25823: Error when reading an annotation VCF.

[davisp]

I've only had one coffee and have only ever read this code, but I'm pretty sure the only bug here was that we were checking if output is empty or not. That's a broken assumption (that output must have size > 0) when all strings were empty during encoding.

To be specific, the encoded dictionary and data in the case of all empty strings should be represented by a dictionary of 0x00, and the input buffer of encoded strings would be a single 0x00 (zero valued byte) per string.

[davisp]

Having just read the code, I'm pretty sure the removal of checking output.empty() here and in dict_compressor.h:186 is the entire fix for the issue.

[ypatia]

So, there are 2 reasons those 2 checks are not enough:

• In deserialize_dictionary, if I have an empty string as entry, I am not properly incrementing the index to the serialized_dictionary (in_index) in dict_compressor.h:261 and I am accessing out of bounds. I special-cased for str_len = 0 there and reverted the rest of the changes, can you check if you like that solution or you have a better idea on how to handle that?
• In decompress I still need the special case for output.empty() in dict_compressor.h:193 because otherwise I get a SIGABRT because I am violating a contract of the implementation of span we are using that assumes that when writing to a span at some index idx : TCB_SPAN_EXPECT(idx < size()); , and in case of an empty output we get a failing 0 < 0 .

[ypatia]

My fix is still not correct for the all empty strings case because of the last special case I mentioned where I exit if output.empty(), because I don't reconstruct the offsets_buffer. If I do reconstruct them I get a heap buffer overflow though, I am still fighting it to understand why.

[davisp]

Ah, yeah, that span behavior is odd since we're indexing with a zero length array. How does this approach look to you?

https://github.com/TileDB-Inc/TileDB/compare/pd/ch25823/support_for_empty_strings_dict_rle

Locally it passes the new test with --enable-debug --enable-assertions on the bootstrap line. I haven't run a full test suite with it though.

[davisp]

I should explain that encoding to make sure I'm not just a single coffee awake.

For the encoded dictionary, its a single 0x00 byte because all string lengths are less than 255 bytes (becuase they're all zero). Thus reading the dictionary, we read a single byte for a length, which is zero. Because its the first string in the dictionary, its "string id" is 0.

For the encoded strings that are all empty, we end up with the an array of 0x00 bytes that all represent empty strings. The reason here is that we know we had fewer than 255 encoded strings, thus the data type for each encoded string is a single byte. So the logic is we read a single byte, the byte contains the word id (0x00 for zero, which means the first string in the encoded dictionary) which is an empty string, which means we copy zero bytes to the output buffer and increment our decoding position by one (the size of our encoded word_id data type).

[davisp]

+1

[gspowley]

Thanks @ypatia and @davisp! This fixes the issue in VCF.