Skip to content

Commit ab902db

Browse files
authored
fix: Serialize native LIST/STRUCT/ARRAY/UNION columns per-row in REST JSON (#89) (#90)
* fix: Serialize LIST/STRUCT columns per-row in REST JSON (#89) - convertVectorListToJson read the entire list child vector for every row instead of the row's own slice, so multi-row LIST(VARCHAR) columns reported the concatenation of all rows' lists. - convertVectorStructToJson always read child element 0, so structs nested in a list (LIST(STRUCT)) repeated the first element N times. - Use the per-row duckdb_list_entry offset/length for lists and pass row_idx through to struct children; honor row validity (NULL lists and structs now serialize as null). - Add regression tests covering single/multi-row LIST(STRUCT), LIST(VARCHAR), multi-row STRUCT, and NULL list entries. Closes #89 * fix: Serialize fixed-size ARRAY columns per-row (#89) ARRAY columns were routed through the LIST serializer, which after the list fix reads per-row duckdb_list_entry offset/length metadata. ARRAY vectors have no such metadata (each row is a constant array_size run in the child vector), so this read garbage and risked UB. - Add convertVectorArrayToJson using duckdb_array_type_array_size and duckdb_array_vector_get_child, indexing child at row_idx*array_size+i. - Route DUCKDB_TYPE_ARRAY to the new serializer; honor row validity. - Add a multi-row fixed-size ARRAY regression test. Found in codex review of the LIST/STRUCT fix. * fix: Serialize UNION columns as active member only (#89) UNION columns were routed through the generic struct serializer, which emitted the tag plus every candidate member for each row, exposing inactive members instead of the one selected by the row's tag. - Add convertVectorUnionToJson: read the tag (struct child 0, uint8), resolve the active member (struct child tag+1) and emit it as {member_name: value}, matching DuckDB's own to_json output. - Route DUCKDB_TYPE_UNION to the new serializer; honor row validity and fail safe on out-of-range tags. - Add a multi-row UNION regression test. Found in codex review of the LIST/STRUCT fix.
1 parent 961b7fb commit ab902db

3 files changed

Lines changed: 259 additions & 5 deletions

File tree

src/include/query_executor.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ class QueryResult {
5656
static crow::json::wvalue convertVectorIntervalToJson(const duckdb_vector &vector, const idx_t row_idx);
5757
static crow::json::wvalue convertVectorEnumToJson(const duckdb_vector &vector, const idx_t row_idx);
5858
static crow::json::wvalue convertVectorListToJson(const duckdb_vector &vector, const idx_t row_idx);
59+
static crow::json::wvalue convertVectorArrayToJson(const duckdb_vector &vector, const idx_t row_idx);
5960
static crow::json::wvalue convertVectorStructToJson(const duckdb_vector &vector, const idx_t row_idx);
61+
static crow::json::wvalue convertVectorUnionToJson(const duckdb_vector &vector, const idx_t row_idx);
6062

6163
template<typename T>
6264
static crow::json::wvalue convertVectorEntryToJson(const duckdb_vector &vector, const idx_t row_idx) {

src/query_executor.cpp

Lines changed: 87 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,9 +288,9 @@ crow::json::wvalue QueryResult::convertVectorEntryToJson(const duckdb_vector &ve
288288
case DUCKDB_TYPE_MAP:
289289
return convertVectorStructToJson(vector, row_idx); // Treat as struct for JSON
290290
case DUCKDB_TYPE_ARRAY:
291-
return convertVectorListToJson(vector, row_idx); // Treat as list for JSON
291+
return convertVectorArrayToJson(vector, row_idx);
292292
case DUCKDB_TYPE_UNION:
293-
return convertVectorStructToJson(vector, row_idx); // Treat as struct for JSON
293+
return convertVectorUnionToJson(vector, row_idx);
294294
default:
295295
CROW_LOG_WARNING << "Unknown type: " << type_id;
296296
return crow::json::wvalue(nullptr);
@@ -422,18 +422,61 @@ crow::json::wvalue QueryResult::convertVectorEnumToJson(const duckdb_vector &vec
422422
}
423423

424424
crow::json::wvalue QueryResult::convertVectorListToJson(const duckdb_vector &vector, const idx_t row_idx) {
425+
auto validity = duckdb_vector_get_validity(vector);
426+
if (!duckdb_validity_row_is_valid(validity, row_idx)) {
427+
return crow::json::wvalue(nullptr);
428+
}
429+
430+
// A LIST column stores every row's elements back-to-back in a single child
431+
// vector. The per-row slice is described by the list_entry at `row_idx`
432+
// (offset + length); emitting the whole child vector instead would repeat
433+
// every other row's elements (issue #89).
434+
auto entries = static_cast<duckdb_list_entry *>(duckdb_vector_get_data(vector));
435+
auto entry = entries[row_idx];
425436
auto child_vector = duckdb_list_vector_get_child(vector);
426437
auto child_size = duckdb_list_vector_get_size(vector);
427438

428439
std::vector<crow::json::wvalue> result;
429-
for (idx_t i = 0; i < child_size; i++) {
430-
result.push_back(convertVectorEntryToJson(child_vector, i));
440+
for (idx_t i = 0; i < entry.length; i++) {
441+
idx_t child_idx = entry.offset + i;
442+
if (child_idx >= child_size) {
443+
break;
444+
}
445+
result.push_back(convertVectorEntryToJson(child_vector, child_idx));
446+
}
447+
448+
return crow::json::wvalue(result);
449+
}
450+
451+
crow::json::wvalue QueryResult::convertVectorArrayToJson(const duckdb_vector &vector, const idx_t row_idx) {
452+
auto validity = duckdb_vector_get_validity(vector);
453+
if (!duckdb_validity_row_is_valid(validity, row_idx)) {
454+
return crow::json::wvalue(nullptr);
455+
}
456+
457+
// A fixed-size ARRAY has no per-row list_entry: every row occupies a
458+
// constant `array_size` run in the child vector, so this row's slice
459+
// starts at row_idx * array_size (unlike LIST, which carries offsets).
460+
auto array_type = duckdb_vector_get_column_type(vector);
461+
auto array_size = duckdb_array_type_array_size(array_type);
462+
duckdb_destroy_logical_type(&array_type);
463+
464+
auto child_vector = duckdb_array_vector_get_child(vector);
465+
466+
std::vector<crow::json::wvalue> result;
467+
for (idx_t i = 0; i < array_size; i++) {
468+
result.push_back(convertVectorEntryToJson(child_vector, row_idx * array_size + i));
431469
}
432470

433471
return crow::json::wvalue(result);
434472
}
435473

436474
crow::json::wvalue QueryResult::convertVectorStructToJson(const duckdb_vector &vector, const idx_t row_idx) {
475+
auto validity = duckdb_vector_get_validity(vector);
476+
if (!duckdb_validity_row_is_valid(validity, row_idx)) {
477+
return crow::json::wvalue(nullptr);
478+
}
479+
437480
auto struct_type = duckdb_vector_get_column_type(vector);
438481
auto child_size = duckdb_struct_type_child_count(struct_type);
439482

@@ -443,12 +486,51 @@ crow::json::wvalue QueryResult::convertVectorStructToJson(const duckdb_vector &v
443486
DuckDBString child_name_wrapper(duckdb_struct_type_child_name(struct_type, i));
444487
auto str_name = child_name_wrapper.to_string();
445488

489+
// Each struct field is a parallel child vector; read this row's entry
490+
// (`row_idx`), not element 0, so multi-row results and structs nested
491+
// inside a list resolve the correct element (issue #89).
446492
auto child_vector = duckdb_struct_vector_get_child(vector, i);
447-
result[str_name] = convertVectorEntryToJson(child_vector, 0);
493+
result[str_name] = convertVectorEntryToJson(child_vector, row_idx);
448494
}
449495

450496
duckdb_destroy_logical_type(&struct_type);
451497
return result;
452498
}
453499

500+
crow::json::wvalue QueryResult::convertVectorUnionToJson(const duckdb_vector &vector, const idx_t row_idx) {
501+
auto validity = duckdb_vector_get_validity(vector);
502+
if (!duckdb_validity_row_is_valid(validity, row_idx)) {
503+
return crow::json::wvalue(nullptr);
504+
}
505+
506+
// A UNION is physically a STRUCT: child 0 is the tag (uint8_t), children
507+
// 1..n are the candidate members. Only the member selected by the row's
508+
// tag is valid, so emit just that member as {name: value} (matching
509+
// DuckDB's own to_json) rather than every member, which the generic
510+
// struct path did, exposing inactive members.
511+
auto union_type = duckdb_vector_get_column_type(vector);
512+
auto member_count = duckdb_union_type_member_count(union_type);
513+
514+
auto tag_vector = duckdb_struct_vector_get_child(vector, 0);
515+
auto tags = static_cast<uint8_t *>(duckdb_vector_get_data(tag_vector));
516+
idx_t member_idx = static_cast<idx_t>(tags[row_idx]);
517+
if (member_idx >= member_count) {
518+
// Out-of-range tag should not happen for well-formed unions; fail
519+
// safe rather than reading past the struct children.
520+
duckdb_destroy_logical_type(&union_type);
521+
return crow::json::wvalue(nullptr);
522+
}
523+
524+
DuckDBString member_name_wrapper(duckdb_union_type_member_name(union_type, member_idx));
525+
auto member_name = member_name_wrapper.to_string();
526+
duckdb_destroy_logical_type(&union_type);
527+
528+
// Member i lives at struct child index i + 1 (the tag occupies slot 0).
529+
auto member_vector = duckdb_struct_vector_get_child(vector, member_idx + 1);
530+
531+
crow::json::wvalue result;
532+
result[member_name] = convertVectorEntryToJson(member_vector, row_idx);
533+
return result;
534+
}
535+
454536
} // namespace flapi

test/cpp/query_executor_test.cpp

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,176 @@ TEST_CASE("QueryExecutor chunk experiment", "[query_executor]") {
228228
duckdb_close(&database);
229229
}
230230

231+
TEST_CASE("QueryExecutor LIST/STRUCT per-row serialization", "[query_executor][list]") {
232+
// Regression for issue #89: native LIST(STRUCT) / LIST(VARCHAR) columns
233+
// must serialize each row's own slice of the list child vector, not the
234+
// whole child vector, and structs nested inside a list must read their
235+
// own element rather than always element[0].
236+
duckdb_database database;
237+
REQUIRE(duckdb_open(NULL, &database) == DuckDBSuccess);
238+
239+
SECTION("single-row LIST(STRUCT) emits distinct elements") {
240+
QueryExecutor executor(database);
241+
executor.execute(R"SQL(
242+
SELECT [
243+
{'stage': 'prepare', 'n_dropped': 11945},
244+
{'stage': 'features', 'n_dropped': 57203},
245+
{'stage': 'postprocess', 'n_dropped': 238505}
246+
] AS by_stage
247+
)SQL");
248+
249+
auto doc = crow::json::load(executor.toJson().dump());
250+
REQUIRE(doc.size() == 1);
251+
REQUIRE(doc[0]["by_stage"].size() == 3);
252+
REQUIRE(doc[0]["by_stage"][0]["stage"].s() == "prepare");
253+
REQUIRE(doc[0]["by_stage"][0]["n_dropped"].i() == 11945);
254+
REQUIRE(doc[0]["by_stage"][1]["stage"].s() == "features");
255+
REQUIRE(doc[0]["by_stage"][1]["n_dropped"].i() == 57203);
256+
REQUIRE(doc[0]["by_stage"][2]["stage"].s() == "postprocess");
257+
REQUIRE(doc[0]["by_stage"][2]["n_dropped"].i() == 238505);
258+
}
259+
260+
SECTION("multi-row LIST(VARCHAR) keeps each row's own list") {
261+
QueryExecutor executor(database);
262+
executor.execute(R"SQL(
263+
SELECT * FROM (VALUES
264+
(1, ['a1', 'a2', 'a3']),
265+
(2, ['b1', 'b2', 'b3']),
266+
(3, ['c1', 'c2', 'c3']),
267+
(4, ['d1', 'd2', 'd3'])
268+
) AS t(id, flags)
269+
ORDER BY id
270+
)SQL");
271+
272+
auto doc = crow::json::load(executor.toJson().dump());
273+
REQUIRE(doc.size() == 4);
274+
275+
REQUIRE(doc[0]["flags"].size() == 3);
276+
REQUIRE(doc[0]["flags"][0].s() == "a1");
277+
REQUIRE(doc[0]["flags"][2].s() == "a3");
278+
279+
REQUIRE(doc[1]["flags"].size() == 3);
280+
REQUIRE(doc[1]["flags"][0].s() == "b1");
281+
REQUIRE(doc[1]["flags"][2].s() == "b3");
282+
283+
REQUIRE(doc[3]["flags"].size() == 3);
284+
REQUIRE(doc[3]["flags"][0].s() == "d1");
285+
REQUIRE(doc[3]["flags"][2].s() == "d3");
286+
}
287+
288+
SECTION("multi-row LIST(STRUCT) keeps each row's own elements") {
289+
QueryExecutor executor(database);
290+
executor.execute(R"SQL(
291+
SELECT * FROM (VALUES
292+
(1, [{'k': 'a', 'v': 1}, {'k': 'b', 'v': 2}]),
293+
(2, [{'k': 'c', 'v': 3}, {'k': 'd', 'v': 4}])
294+
) AS t(id, items)
295+
ORDER BY id
296+
)SQL");
297+
298+
auto doc = crow::json::load(executor.toJson().dump());
299+
REQUIRE(doc.size() == 2);
300+
301+
REQUIRE(doc[0]["items"].size() == 2);
302+
REQUIRE(doc[0]["items"][0]["k"].s() == "a");
303+
REQUIRE(doc[0]["items"][0]["v"].i() == 1);
304+
REQUIRE(doc[0]["items"][1]["k"].s() == "b");
305+
REQUIRE(doc[0]["items"][1]["v"].i() == 2);
306+
307+
REQUIRE(doc[1]["items"].size() == 2);
308+
REQUIRE(doc[1]["items"][0]["k"].s() == "c");
309+
REQUIRE(doc[1]["items"][0]["v"].i() == 3);
310+
REQUIRE(doc[1]["items"][1]["k"].s() == "d");
311+
REQUIRE(doc[1]["items"][1]["v"].i() == 4);
312+
}
313+
314+
SECTION("multi-row STRUCT column reads its own row") {
315+
QueryExecutor executor(database);
316+
executor.execute(R"SQL(
317+
SELECT * FROM (VALUES
318+
(1, {'name': 'alice', 'age': 30}),
319+
(2, {'name': 'bob', 'age': 40})
320+
) AS t(id, person)
321+
ORDER BY id
322+
)SQL");
323+
324+
auto doc = crow::json::load(executor.toJson().dump());
325+
REQUIRE(doc.size() == 2);
326+
REQUIRE(doc[0]["person"]["name"].s() == "alice");
327+
REQUIRE(doc[0]["person"]["age"].i() == 30);
328+
REQUIRE(doc[1]["person"]["name"].s() == "bob");
329+
REQUIRE(doc[1]["person"]["age"].i() == 40);
330+
}
331+
332+
SECTION("multi-row fixed-size ARRAY keeps each row's own elements") {
333+
QueryExecutor executor(database);
334+
executor.execute(R"SQL(
335+
SELECT * FROM (VALUES
336+
(1, [10, 11, 12]::INTEGER[3]),
337+
(2, [20, 21, 22]::INTEGER[3]),
338+
(3, [30, 31, 32]::INTEGER[3])
339+
) AS t(id, coords)
340+
ORDER BY id
341+
)SQL");
342+
343+
auto doc = crow::json::load(executor.toJson().dump());
344+
REQUIRE(doc.size() == 3);
345+
346+
REQUIRE(doc[0]["coords"].size() == 3);
347+
REQUIRE(doc[0]["coords"][0].i() == 10);
348+
REQUIRE(doc[0]["coords"][2].i() == 12);
349+
350+
REQUIRE(doc[2]["coords"].size() == 3);
351+
REQUIRE(doc[2]["coords"][0].i() == 30);
352+
REQUIRE(doc[2]["coords"][2].i() == 32);
353+
}
354+
355+
SECTION("multi-row UNION emits only the active member per row") {
356+
QueryExecutor executor(database);
357+
executor.execute(R"SQL(
358+
SELECT id, u FROM (
359+
SELECT 1 AS id, union_value(num := 42)::UNION(num INTEGER, str VARCHAR) AS u
360+
UNION ALL
361+
SELECT 2, union_value(str := 'hi')::UNION(num INTEGER, str VARCHAR)
362+
UNION ALL
363+
SELECT 3, union_value(num := 7)::UNION(num INTEGER, str VARCHAR)
364+
) ORDER BY id
365+
)SQL");
366+
367+
auto doc = crow::json::load(executor.toJson().dump());
368+
REQUIRE(doc.size() == 3);
369+
370+
// Each row exposes only its active member ({name: value}), matching
371+
// DuckDB's to_json — not every union member.
372+
REQUIRE(doc[0]["u"]["num"].i() == 42);
373+
REQUIRE_FALSE(doc[0]["u"].has("str"));
374+
375+
REQUIRE(doc[1]["u"]["str"].s() == "hi");
376+
REQUIRE_FALSE(doc[1]["u"].has("num"));
377+
378+
REQUIRE(doc[2]["u"]["num"].i() == 7);
379+
REQUIRE_FALSE(doc[2]["u"].has("str"));
380+
}
381+
382+
SECTION("NULL list entry stays null") {
383+
QueryExecutor executor(database);
384+
executor.execute(R"SQL(
385+
SELECT * FROM (VALUES
386+
(1, ['x', 'y']),
387+
(2, CAST(NULL AS VARCHAR[]))
388+
) AS t(id, flags)
389+
ORDER BY id
390+
)SQL");
391+
392+
auto doc = crow::json::load(executor.toJson().dump());
393+
REQUIRE(doc.size() == 2);
394+
REQUIRE(doc[0]["flags"].size() == 2);
395+
REQUIRE(doc[1]["flags"].t() == crow::json::type::Null);
396+
}
397+
398+
duckdb_close(&database);
399+
}
400+
231401
TEST_CASE("QueryExecutor::executeWithBindings - prepared path", "[query_executor][prepared]") {
232402
duckdb_database database;
233403
REQUIRE(duckdb_open(NULL, &database) == DuckDBSuccess);

0 commit comments

Comments
 (0)