Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Backport release-2.9] Sparse global order reader: Check correct incomplete reason in case of too small user buffer #3186

Merged
merged 1 commit into from May 17, 2022
Merged

[Backport release-2.9] Sparse global order reader: Check correct incomplete reason in case of too small user buffer #3186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions test/src/unit-capi-incomplete.cc
View file
Open in desktop
Expand Up @@ -1131,6 +1131,11 @@ void IncompleteFx::check_sparse_unsplittable_overflow() {
CHECK(status == TILEDB_INCOMPLETE);
CHECK(buffer_sizes[0] == 0);

tiledb_query_status_details_t details;
rc = tiledb_query_get_status_details(ctx_, query, &details);
CHECK(rc == TILEDB_OK);
CHECK(details.incomplete_reason == TILEDB_REASON_USER_BUFFER_SIZE);

// Finalize query
rc = tiledb_query_finalize(ctx_, query);
REQUIRE(rc == TILEDB_OK);
Expand Down
112 changes: 100 additions & 12 deletions test/src/unit-capi-string_dims.cc
View file
Open in desktop
Expand Up @@ -151,7 +151,8 @@ struct StringDimsFx {
std::vector<uint64_t>* d_off,
std::string* d_val,
std::vector<int32_t>* a,
tiledb_query_status_t* status);
tiledb_query_status_t* status,
tiledb_query_status_details_t* details);
void read_array_2d(
tiledb_ctx_t* ctx,
tiledb_array_t* array,
Expand Down Expand Up @@ -964,7 +965,8 @@ void StringDimsFx::read_array_1d(
std::vector<uint64_t>* d_off,
std::string* d_val,
std::vector<int32_t>* a,
tiledb_query_status_t* status) {
tiledb_query_status_t* status,
tiledb_query_status_details_t* details) {
// Create query
tiledb_query_t* query;
int rc = tiledb_query_alloc(ctx, array, TILEDB_READ, &query);
Expand Down Expand Up @@ -1030,6 +1032,10 @@ void StringDimsFx::read_array_1d(
rc = tiledb_query_get_status(ctx, query, status);
CHECK(rc == TILEDB_OK);

// Get details
rc = tiledb_query_get_status_details(ctx, query, details);
CHECK(rc == TILEDB_OK);

// Resize the result buffers
d_off->resize(d_off_size / sizeof(uint64_t));
d_val->resize(d_val_size / sizeof(char));
Expand Down Expand Up @@ -1802,8 +1808,18 @@ TEST_CASE_METHOD(
r_d_val.resize(20);
std::vector<int32_t> r_a(10);
tiledb_query_status_t status;
tiledb_query_status_details_t details;
read_array_1d(
ctx_, array, layout, "a", "ee", &r_d_off, &r_d_val, &r_a, &status);
ctx_,
array,
layout,
"a",
"ee",
&r_d_off,
&r_d_val,
&r_a,
&status,
&details);
CHECK(status == TILEDB_COMPLETED);
CHECK(r_d_val == "aabbccdddd");
std::vector<uint64_t> c_d_off = {0, 2, 4, 6};
Expand All @@ -1816,7 +1832,16 @@ TEST_CASE_METHOD(
r_d_val.resize(20);
r_a.resize(10);
read_array_1d(
ctx_, array, layout, "aab", "cc", &r_d_off, &r_d_val, &r_a, &status);
ctx_,
array,
layout,
"aab",
"cc",
&r_d_off,
&r_d_val,
&r_a,
&status,
&details);
CHECK(status == TILEDB_COMPLETED);
CHECK(r_d_val == "bbcc");
c_d_off = {0, 2};
Expand All @@ -1829,24 +1854,44 @@ TEST_CASE_METHOD(
r_d_val.resize(20);
r_a.resize(10);
read_array_1d(
ctx_, array, layout, "aa", "cc", &r_d_off, &r_d_val, &r_a, &status);
ctx_,
array,
layout,
"aa",
"cc",
&r_d_off,
&r_d_val,
&r_a,
&status,
&details);
CHECK(status == TILEDB_INCOMPLETE);
CHECK(r_d_val == "aabb");
c_d_off = {0, 2};
CHECK(r_d_off == c_d_off);
c_a = {1, 2};
CHECK(r_a == c_a);
CHECK(details.incomplete_reason == TILEDB_REASON_USER_BUFFER_SIZE);

// Read [aa, cc] - INCOMPLETE, no result
r_d_off.resize(1);
r_d_val.resize(1);
r_a.resize(10);
read_array_1d(
ctx_, array, layout, "aa", "bb", &r_d_off, &r_d_val, &r_a, &status);
ctx_,
array,
layout,
"aa",
"bb",
&r_d_off,
&r_d_val,
&r_a,
&status,
&details);
CHECK(status == TILEDB_INCOMPLETE);
CHECK(r_d_val.size() == 0);
CHECK(r_d_off.size() == 0);
CHECK(r_a.size() == 0);
CHECK(details.incomplete_reason == TILEDB_REASON_USER_BUFFER_SIZE);

// Close array
rc = tiledb_array_close(ctx_, array);
Expand Down Expand Up @@ -1946,8 +1991,18 @@ TEST_CASE_METHOD(
r_d_val.resize(20);
std::vector<int32_t> r_a(10);
tiledb_query_status_t status;
tiledb_query_status_details_t details;
read_array_1d(
ctx_, array, layout, "a", "ee", &r_d_off, &r_d_val, &r_a, &status);
ctx_,
array,
layout,
"a",
"ee",
&r_d_off,
&r_d_val,
&r_a,
&status,
&details);
CHECK(status == TILEDB_COMPLETED);
CHECK(r_d_val == "aaabbbccddddee");
std::vector<uint64_t> c_d_off = {0, 1, 3, 4, 6, 8, 12};
Expand Down Expand Up @@ -1999,7 +2054,16 @@ TEST_CASE_METHOD(
r_d_val.resize(20);
r_a.resize(10);
read_array_1d(
ctx_, array, layout, "a", "ee", &r_d_off, &r_d_val, &r_a, &status);
ctx_,
array,
layout,
"a",
"ee",
&r_d_off,
&r_d_val,
&r_a,
&status,
&details);
CHECK(status == TILEDB_COMPLETED);
CHECK(r_d_val == "aaabbbccddddee");
CHECK(r_d_off == c_d_off);
Expand Down Expand Up @@ -2093,8 +2157,18 @@ TEST_CASE_METHOD(
r_d_val.resize(20);
std::vector<int32_t> r_a(10);
tiledb_query_status_t status;
tiledb_query_status_details_t details;
read_array_1d(
ctx_, array, layout, "a", "e", &r_d_off, &r_d_val, &r_a, &status);
ctx_,
array,
layout,
"a",
"e",
&r_d_off,
&r_d_val,
&r_a,
&status,
&details);
CHECK(status == TILEDB_COMPLETED);
CHECK(r_d_val == "aaccccdddd");
std::vector<uint64_t> c_d_off = {0, 2, 4, 6};
Expand Down Expand Up @@ -2211,8 +2285,18 @@ TEST_CASE_METHOD(
r_d_val.resize(20);
std::vector<int32_t> r_a(10);
tiledb_query_status_t status;
tiledb_query_status_details_t details;
read_array_1d(
ctx, array, layout, "a", "e", &r_d_off, &r_d_val, &r_a, &status);
ctx,
array,
layout,
"a",
"e",
&r_d_off,
&r_d_val,
&r_a,
&status,
&details);
CHECK(status == TILEDB_COMPLETED);
CHECK(r_d_val == "aaccdddd");
std::vector<uint64_t> c_d_off = {0, 2, 4};
Expand Down Expand Up @@ -2587,6 +2671,7 @@ TEST_CASE_METHOD(
r_d_val.resize(20);
std::vector<int32_t> r_a(10);
tiledb_query_status_t status;
tiledb_query_status_details_t details;
read_array_1d(
ctx_,
array,
Expand All @@ -2596,7 +2681,8 @@ TEST_CASE_METHOD(
&r_d_off,
&r_d_val,
&r_a,
&status);
&status,
&details);
CHECK(status == TILEDB_COMPLETED);
CHECK(r_d_val == "ab");
std::vector<uint64_t> c_d_off = {0, 1};
Expand Down Expand Up @@ -2716,6 +2802,7 @@ TEST_CASE_METHOD(
r_d_val.resize(20);
std::vector<int32_t> r_a(10);
tiledb_query_status_t status;
tiledb_query_status_details_t details;
read_array_1d(
ctx_,
array,
Expand All @@ -2725,7 +2812,8 @@ TEST_CASE_METHOD(
&r_d_off,
&r_d_val,
&r_a,
&status);
&status,
&details);
CHECK(status == TILEDB_COMPLETED);
CHECK(r_d_val == "abcde");
std::vector<uint64_t> c_d_off = {0, 1, 2, 3, 4};
Expand Down
88 changes: 85 additions & 3 deletions test/src/unit-sparse-global-order-reader.cc
View file
Open in desktop
Expand Up @@ -33,6 +33,7 @@
#include "test/src/helpers.h"
#include "tiledb/sm/c_api/tiledb.h"
#include "tiledb/sm/c_api/tiledb_struct_def.h"
#include "tiledb/sm/cpp_api/tiledb"
#include "tiledb/sm/query/sparse_global_order_reader.h"

#ifdef _WIN32
Expand All @@ -43,7 +44,7 @@

#include <catch.hpp>

using namespace tiledb::sm;
using namespace tiledb;
using namespace tiledb::test;

/* ********************************* */
Expand Down Expand Up @@ -449,7 +450,8 @@ TEST_CASE_METHOD(
CHECK(rc == TILEDB_OK);

// Check the internal loop count against expected value.
auto stats = ((SparseGlobalOrderReader*)query->query_->strategy())->stats();
auto stats =
((sm::SparseGlobalOrderReader*)query->query_->strategy())->stats();
REQUIRE(stats != nullptr);
auto counters = stats->counters();
REQUIRE(counters != nullptr);
Expand Down Expand Up @@ -643,7 +645,8 @@ TEST_CASE_METHOD(
CHECK(rc == TILEDB_OK);

// Check the internal loop count against expected value.
auto stats = ((SparseGlobalOrderReader*)query->query_->strategy())->stats();
auto stats =
((sm::SparseGlobalOrderReader*)query->query_->strategy())->stats();
REQUIRE(stats != nullptr);
auto counters = stats->counters();
REQUIRE(counters != nullptr);
Expand Down Expand Up @@ -801,4 +804,83 @@ TEST_CASE_METHOD(
int data_c[] = {8, 8, 9, 9, 10, 10};
CHECK(!std::memcmp(coords_c, coords_r, coords_r_size));
CHECK(!std::memcmp(data_c, data_r, data_r_size));
}

TEST_CASE(
"Sparse global order reader: user buffer cannot fit single cell",
"[sparse-global-order][user-buffer][too-small]") {
std::string array_name = "test_sparse_global_order";
Context ctx;
VFS vfs(ctx);

if (vfs.is_dir(array_name)) {
vfs.remove_dir(array_name);
}

// Create array with var-sized attribute.
Domain dom(ctx);
dom.add_dimension(Dimension::create<int64_t>(ctx, "d1", {{1, 4}}, 2));

Attribute attr(ctx, "a", TILEDB_STRING_ASCII);
attr.set_cell_val_num(TILEDB_VAR_NUM);

ArraySchema schema(ctx, TILEDB_SPARSE);
schema.add_attribute(attr);
schema.set_tile_order(TILEDB_ROW_MAJOR);
schema.set_cell_order(TILEDB_ROW_MAJOR);
schema.set_domain(dom);
schema.set_allows_dups(true);

Array::create(array_name, schema);

// Write a fragment.
Array array(ctx, array_name, TILEDB_WRITE);
Query query(ctx, array, TILEDB_WRITE);
query.set_layout(TILEDB_GLOBAL_ORDER);
std::vector<int64_t> d1 = {1, 2, 3};
std::string a1_data{
"astringofsize15"
"foo"
"bar"};
std::vector<uint64_t> a1_offsets{0, 15, 18};

query.set_data_buffer("d1", d1);
query.set_data_buffer("a", a1_data);
query.set_offsets_buffer("a", a1_offsets);
CHECK_NOTHROW(query.submit());

// Finalize is necessary in global writes, otherwise a no-op
query.finalize();

// Read using a buffer that can't fit a single result
Array array2(ctx, array_name, TILEDB_READ);
Query query2(ctx, array2, TILEDB_READ);

std::string attr_val;
// The first result is 15 bytes long so it won't fit in 10 byte user buffer
attr_val.resize(10);
std::vector<uint64_t> attr_off(sizeof(uint64_t));

auto layout = GENERATE(TILEDB_GLOBAL_ORDER, TILEDB_UNORDERED);

query2.set_layout(layout);
query2.set_data_buffer("a", (char*)attr_val.data(), attr_val.size());
query2.set_offsets_buffer("a", attr_off);

// The user buffer cannot fit a single result so it should return Incomplete
// with the right reason
auto st = query2.submit();
CHECK(st == Query::Status::INCOMPLETE);

tiledb_query_status_details_t details;
int rc = tiledb_query_get_status_details(
ctx.ptr().get(), query2.ptr().get(), &details);
CHECK(rc == TILEDB_OK);
CHECK(details.incomplete_reason == TILEDB_REASON_USER_BUFFER_SIZE);

array2.close();

if (vfs.is_dir(array_name)) {
vfs.remove_dir(array_name);
}
}
6 changes: 3 additions & 3 deletions tiledb/sm/enums/query_status_details.h
View file
Open in desktop
@@ -1,5 +1,5 @@
/**
* @file query_status.h
* @file query_status_details.h
*
* @section LICENSE
*
Expand Down Expand Up @@ -27,8 +27,8 @@
*
* @section DESCRIPTION
*
* This defines the tiledb QueryStatus enum that maps to tiledb_query_status_t
* C-api enum.
* This defines the tiledb QueryStatusDetailsReason enum that maps to
* tiledb_query_status_details_t C-api enum.
*/

#ifndef TILEDB_QUERY_STATUS_DETAILS_H
Expand Down