Fix computation of GZip filter overhead.

test/regression/CMakeLists.txt

@@ -51,6 +51,7 @@ if (TILEDB_CPP_API)
   list(APPEND SOURCES targets/sc-35424.cc)
   list(APPEND SOURCES targets/sc-36372.cc)
   list(APPEND SOURCES targets/sc-38300.cc)
+  list(APPEND SOURCES targets/sc-48428.cc)
   list(APPEND SOURCES targets/sc-52975.cc)
   list(APPEND SOURCES targets/sc-53334.cc)
   list(APPEND SOURCES targets/sc-53791.cc)

test/regression/targets/sc-48428.cc

@@ -0,0 +1,88 @@
+#include <climits>
+
+#include <tiledb/tiledb>
+#include <tiledb/tiledb_experimental>
+
+#include <test/support/tdb_catch.h>
+
+using namespace tiledb;
+
+static void create_array(Context& ctx, const std::string& array_uri);
+static void write_array(Context& ctx, const std::string& array_uri);
+static void read_array(Context& ctx, const std::string& array_uri);
+
+TEST_CASE(
+    "SC-48428: Empty var tile compressed with GZip breaks reads",
+    "[hilbert][bug][sc48428]") {
+  Context ctx;
+  std::string array_uri = "test_empty_write";
+
+  // Test setup
+  create_array(ctx, array_uri);
+  write_array(ctx, array_uri);
+  read_array(ctx, array_uri);
+}
+
+void create_array(Context& ctx, const std::string& array_uri) {
+  auto obj = Object::object(ctx, array_uri);
+  if (obj.type() != Object::Type::Invalid) {
+    Object::remove(ctx, array_uri);
+  }
+
+  auto dim = Dimension::create<uint8_t>(ctx, "d", {{69, 105}}, 1);
+
+  Domain dom(ctx);
+  dom.add_dimension(dim);
+
+  auto filter = Filter(ctx, TILEDB_FILTER_GZIP);
+  auto flist = FilterList(ctx).add_filter(filter);
+
+  auto attr = Attribute::create(ctx, "a", TILEDB_STRING_UTF16)
+                  .set_cell_val_num(std::numeric_limits<uint32_t>::max())
+                  .set_filter_list(flist);
+
+  auto schema = ArraySchema(ctx, TILEDB_DENSE)
+                    .set_cell_order(TILEDB_ROW_MAJOR)
+                    .set_tile_order(TILEDB_COL_MAJOR)
+                    .set_domain(dom)
+                    .add_attribute(attr);
+
+  Array::create(array_uri, schema);
+}
+
+void write_array(Context& ctx, const std::string& array_uri) {
+  auto data = std::vector<uint16_t>();
+  auto offsets = std::vector<uint64_t>();
+  offsets.push_back(0);
+
+  Array array(ctx, array_uri, TILEDB_WRITE);
+  auto subarray = Subarray(ctx, array);
+  uint8_t start = 97;
+  uint8_t end = 97;
+  subarray.add_range(0, start, end);
+
+  Query query(ctx, array, TILEDB_WRITE);
+
+  query.set_layout(TILEDB_ROW_MAJOR)
+      .set_subarray(subarray)
+      .set_data_buffer("a", (void*)data.data(), data.size())
+      .set_offsets_buffer("a", offsets.data(), offsets.size());
+  REQUIRE(query.submit() == Query::Status::COMPLETE);
+}
+
+void read_array(Context& ctx, const std::string& array_uri) {
+  Array array(ctx, array_uri, TILEDB_READ);
+  auto subarray = Subarray(ctx, array);
+  uint8_t start = 97;
+  uint8_t end = 97;
+  subarray.add_range(0, start, end);
+
+  std::vector<uint16_t> a(1000000);
+  std::vector<uint64_t> offsets(256);
+  Query query(ctx, array, TILEDB_READ);
+  query.set_layout(TILEDB_ROW_MAJOR)
+      .set_subarray(subarray)
+      .set_data_buffer("a", (void*)a.data(), a.size())
+      .set_offsets_buffer("a", offsets);
+  REQUIRE(query.submit() == Query::Status::COMPLETE);
+}

tiledb/sm/compressors/gzip_compressor.cc

@@ -40,8 +40,7 @@
 
 using namespace tiledb::common;
 
-namespace tiledb {
-namespace sm {
+namespace tiledb::sm {
 
 class GZipException : public StatusException {
  public:
@@ -88,8 +87,13 @@ void GZip::compress(
   (void)deflateEnd(&strm);
 
   // Return
-  if (ret == Z_STREAM_ERROR || strm.avail_in != 0)
-    throw GZipException("Cannot compress with GZIP");
+  if (ret != Z_STREAM_END || strm.avail_in != 0) {
+    if (ret == Z_OK) {
+      throw GZipException("Cannot compress with GZIP; output buffer too small");
+    }
+    throw GZipException(
+        "Cannot compress with GZIP; error code " + std::to_string(ret));
+  }
 
   // Set size of compressed data
   uint64_t compressed_size = output_buffer->free_space() - strm.avail_out;
@@ -143,8 +147,10 @@ void GZip::decompress(
 }
 
 uint64_t GZip::overhead(uint64_t buffer_size) {
-  return 6 + 5 * uint64_t((std::ceil(buffer_size / 16834.0)));
+  // The zlib encoding adds 6 bytes (we don't use compression dictionary)
+  // and the overhead of deflate was taken from
+  // https://stackoverflow.com/a/23578269.
+  return 6 + 5 * uint64_t(std::floor(buffer_size / 16834.0) + 1);
 }
 
-};  // namespace sm
-}  // namespace tiledb
+}  // namespace tiledb::sm

tiledb/sm/compressors/test/CMakeLists.txt

@@ -27,5 +27,5 @@ include(unit_test)
 
 commence(unit_test compressors)
     this_target_object_libraries(compressors)
-    this_target_sources(main.cc unit_dict_compressor.cc unit_delta_compressor.cc)
+    this_target_sources(main.cc unit_dict_compressor.cc unit_delta_compressor.cc unit_gzip_compressor.cc)
 conclude(unit_test)

tiledb/sm/compressors/test/unit_gzip_compressor.cc

@@ -0,0 +1,76 @@
+/**
+ * @file   unit_gzip_compressor.cc
+ *
+ * @section LICENSE
+ *
+ * The MIT License
+ *
+ * @copyright Copyright (c) 2024 TileDB Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * @section DESCRIPTION
+ *
+ * Tests for the GZip compressor.
+ */
+
+#include "../gzip_compressor.h"
+#include "tiledb/sm/buffer/buffer.h"
+
+#include <catch2/catch_test_macros.hpp>
+#include <catch2/generators/catch_generators.hpp>
+#include <cstring>
+#include <iterator>
+#include <sstream>
+
+using namespace tiledb::common;
+using namespace tiledb::sm;
+
+TEST_CASE(
+    "Compression-GZip: Test compression with too small output buffer fails",
+    "[compression][gzip]") {
+  auto size = GENERATE(0, 64, 1024);
+  std::string str(size, 'a');
+  ConstBuffer in_buf{str.data(), str.size()};
+
+  Buffer out_buf;
+  throw_if_not_ok(out_buf.realloc(1));
+
+  CHECK_THROWS(GZip::compress(&in_buf, &out_buf));
+}
+
+TEST_CASE(
+    "Compression-GZip: Test compression of empty buffer",
+    "[compression][gzip]") {
+  ConstBuffer in_buf{"", 0};
+
+  Buffer out_buf;
+  throw_if_not_ok(out_buf.realloc(GZip::overhead(in_buf.size())));
+
+  GZip::compress(&in_buf, &out_buf);
+
+  ConstBuffer in_buf_dec{out_buf.data(), out_buf.size()};
+  std::vector<char> out_buf_dec_storage(1024);
+  PreallocatedBuffer out_buf_dec{
+      out_buf_dec_storage.data(), out_buf_dec_storage.size()};
+
+  GZip::decompress(&in_buf_dec, &out_buf_dec);
+  // Check that the empty buffer is compressed and decompressed correctly.
+  CHECK(out_buf_dec.size() - out_buf_dec.free_space() == in_buf.size());
+}