Skip to content

Commit 96806ac

Browse files
authored
fix: Embed JSON-aliased VARCHAR columns as nested JSON (#38) (#39)
DuckDB's JSON type is a logical-type alias over VARCHAR; the dispatch in QueryResult::convertVectorEntryToJson switched on the physical type id and routed JSON cells to the VARCHAR handler, so API consumers got an escaped string and had to JSON.parse a second time. Inspect the logical-type alias before dispatching. When it equals the literal "JSON" that DuckDB sets on LogicalType::JSON(), route to a new convertVectorJsonToJson helper that parses with crow::json::load and embeds the result via wvalue(const rvalue&). Nested objects and arrays now travel through the response unchanged. Malformed JSON degrades to the raw string rather than nulling the row. Also destroy the logical type on every exit path of convertVectorEntryToJson; the dead destroy below the switch was unreachable, leaking one logical-type allocation per emitted cell. Tests added TDD-style (red → green): - unit: nested object, JSON array, NULL JSON, plain-VARCHAR regression guard. - integration: GET /json-demo/ returns the exact "Expected" shape from the issue. Closes #38
1 parent 8241569 commit 96806ac

6 files changed

Lines changed: 160 additions & 4 deletions

File tree

src/include/query_executor.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ class QueryResult {
4848

4949
static crow::json::wvalue convertVectorEntryToJson(const duckdb_vector &vector, const idx_t row_idx);
5050
static crow::json::wvalue convertVectorVarcharToJson(const duckdb_vector &vector, const idx_t row_idx);
51+
static crow::json::wvalue convertVectorJsonToJson(const duckdb_vector &vector, const idx_t row_idx);
5152
static crow::json::wvalue convertVectorDecimalToJson(const duckdb_vector &vector, const idx_t row_idx);
5253
static crow::json::wvalue convertVectorTimestampToJson(const duckdb_vector &vector, const idx_t row_idx);
5354
static crow::json::wvalue convertVectorDateToJson(const duckdb_vector &vector, const idx_t row_idx);

src/query_executor.cpp

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,23 @@ std::vector<crow::json::wvalue> QueryResult::convertChunkToJson(const std::vecto
203203
crow::json::wvalue QueryResult::convertVectorEntryToJson(const duckdb_vector &vector, const idx_t row_idx) {
204204
auto type = duckdb_vector_get_column_type(vector);
205205
auto type_id = duckdb_get_type_id(type);
206+
207+
// DuckDB's JSON type is a logical-type alias over VARCHAR (see
208+
// DuckDB's LogicalType::JSON()), so `duckdb_get_type_id` returns
209+
// VARCHAR for it. Detect the alias before dispatching to the VARCHAR
210+
// handler so JSON columns are embedded as nested JSON rather than
211+
// emitted as escaped strings.
212+
bool is_json_alias = false;
213+
if (type_id == DUCKDB_TYPE_VARCHAR) {
214+
DuckDBString alias(duckdb_logical_type_get_alias(type));
215+
is_json_alias = !alias.is_null() && std::string(alias.get()) == "JSON";
216+
}
217+
duckdb_destroy_logical_type(&type);
218+
219+
if (is_json_alias) {
220+
return convertVectorJsonToJson(vector, row_idx);
221+
}
222+
206223
switch (type_id) {
207224
case DUCKDB_TYPE_SQLNULL:
208225
return crow::json::wvalue(nullptr);
@@ -278,17 +295,14 @@ crow::json::wvalue QueryResult::convertVectorEntryToJson(const duckdb_vector &ve
278295
CROW_LOG_WARNING << "Unknown type: " << type_id;
279296
return crow::json::wvalue(nullptr);
280297
}
281-
282-
duckdb_destroy_logical_type(&type);
283-
return crow::json::wvalue();
284298
}
285299

286300
crow::json::wvalue QueryResult::convertVectorVarcharToJson(const duckdb_vector &vector, const idx_t row_idx) {
287301
auto validity = duckdb_vector_get_validity(vector);
288302
if (!duckdb_validity_row_is_valid(validity, row_idx)) {
289303
return crow::json::wvalue(nullptr);
290304
}
291-
305+
292306
auto data = (duckdb_string_t *)duckdb_vector_get_data(vector);
293307
auto str = duckdb_string_is_inlined(data[row_idx])
294308
? std::string(data[row_idx].value.inlined.inlined, data[row_idx].value.inlined.length)
@@ -297,6 +311,30 @@ crow::json::wvalue QueryResult::convertVectorVarcharToJson(const duckdb_vector &
297311
return crow::json::wvalue(str);
298312
}
299313

314+
crow::json::wvalue QueryResult::convertVectorJsonToJson(const duckdb_vector &vector, const idx_t row_idx) {
315+
auto validity = duckdb_vector_get_validity(vector);
316+
if (!duckdb_validity_row_is_valid(validity, row_idx)) {
317+
return crow::json::wvalue(nullptr);
318+
}
319+
320+
auto data = (duckdb_string_t *)duckdb_vector_get_data(vector);
321+
const char* str_ptr = duckdb_string_is_inlined(data[row_idx])
322+
? data[row_idx].value.inlined.inlined
323+
: (const char*)data[row_idx].value.pointer.ptr;
324+
const idx_t str_len = duckdb_string_is_inlined(data[row_idx])
325+
? data[row_idx].value.inlined.length
326+
: data[row_idx].value.pointer.length;
327+
328+
auto parsed = crow::json::load(str_ptr, str_len);
329+
if (!parsed) {
330+
// Source row contains malformed JSON; degrade to the raw string
331+
// rather than dropping the row or returning null. The cell stays
332+
// queryable, just as a string.
333+
return crow::json::wvalue(std::string(str_ptr, str_len));
334+
}
335+
return crow::json::wvalue(parsed);
336+
}
337+
300338
crow::json::wvalue QueryResult::convertVectorDecimalToJson(const duckdb_vector &vector, const idx_t row_idx) {
301339
auto validity = duckdb_vector_get_validity(vector);
302340

test/cpp/query_executor_test.cpp

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,76 @@ TEST_CASE("QueryExecutor error handling", "[query_executor]") {
7979
duckdb_close(&database);
8080
}
8181

82+
TEST_CASE("QueryExecutor JSON column", "[query_executor][json]") {
83+
duckdb_database database;
84+
REQUIRE(duckdb_open(NULL, &database) == DuckDBSuccess);
85+
86+
QueryExecutor executor(database);
87+
executor.execute("INSTALL json; LOAD json;");
88+
89+
SECTION("nested JSON value is embedded, not escaped") {
90+
// Reproduction from issue #38: a column with the DuckDB `JSON`
91+
// logical-type alias must serialise as nested JSON, not as a
92+
// JSON-escaped string the caller has to parse a second time.
93+
executor.execute(R"SQL(
94+
SELECT
95+
1 AS id,
96+
'{"a": 1, "b": [10, 20], "c": {"nested": true}}'::JSON AS payload
97+
)SQL");
98+
99+
auto doc = crow::json::load(executor.toJson().dump());
100+
REQUIRE(doc.size() == 1);
101+
REQUIRE(doc[0]["id"].i() == 1);
102+
103+
// The crux: payload must be an Object, not a String.
104+
REQUIRE(doc[0]["payload"].t() == crow::json::type::Object);
105+
REQUIRE(doc[0]["payload"]["a"].i() == 1);
106+
REQUIRE(doc[0]["payload"]["b"].t() == crow::json::type::List);
107+
REQUIRE(doc[0]["payload"]["b"].size() == 2);
108+
REQUIRE(doc[0]["payload"]["b"][0].i() == 10);
109+
REQUIRE(doc[0]["payload"]["b"][1].i() == 20);
110+
REQUIRE(doc[0]["payload"]["c"]["nested"].b() == true);
111+
}
112+
113+
SECTION("JSON array column is embedded as array") {
114+
executor.execute(R"SQL(
115+
SELECT '[1, 2, 3]'::JSON AS arr
116+
)SQL");
117+
118+
auto doc = crow::json::load(executor.toJson().dump());
119+
REQUIRE(doc.size() == 1);
120+
REQUIRE(doc[0]["arr"].t() == crow::json::type::List);
121+
REQUIRE(doc[0]["arr"].size() == 3);
122+
REQUIRE(doc[0]["arr"][0].i() == 1);
123+
REQUIRE(doc[0]["arr"][2].i() == 3);
124+
}
125+
126+
SECTION("NULL JSON column stays null") {
127+
executor.execute(R"SQL(
128+
SELECT CAST(NULL AS JSON) AS payload
129+
)SQL");
130+
131+
auto doc = crow::json::load(executor.toJson().dump());
132+
REQUIRE(doc.size() == 1);
133+
REQUIRE(doc[0]["payload"].t() == crow::json::type::Null);
134+
}
135+
136+
SECTION("plain VARCHAR is still emitted as a string") {
137+
// Regression guard: only the JSON-aliased VARCHAR path changes;
138+
// bare VARCHAR must continue to render as a JSON string.
139+
executor.execute(R"SQL(
140+
SELECT '{"looks":"like json"}'::VARCHAR AS not_json
141+
)SQL");
142+
143+
auto doc = crow::json::load(executor.toJson().dump());
144+
REQUIRE(doc.size() == 1);
145+
REQUIRE(doc[0]["not_json"].t() == crow::json::type::String);
146+
REQUIRE(doc[0]["not_json"].s() == "{\"looks\":\"like json\"}");
147+
}
148+
149+
duckdb_close(&database);
150+
}
151+
82152
TEST_CASE("QueryExecutor type coverage", "[query_executor]") {
83153
duckdb_database database;
84154
REQUIRE(duckdb_open(NULL, &database) == DuckDBSuccess);
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-- Reproduction of issue #38. `::JSON` triggers DuckDB's autoload of the
2+
-- json extension, so no explicit LOAD is needed here.
3+
SELECT
4+
1 AS id,
5+
'{"a": 1, "b": [10, 20], "c": {"nested": true}}'::JSON AS payload
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# Reproduces issue #38: a DuckDB JSON-aliased VARCHAR column must serialise
2+
# as nested JSON, not as a JSON-escaped string the caller has to parse a
3+
# second time. The endpoint returns the literal payload from the issue's
4+
# reproduction so a test can assert on the structure directly.
5+
url-path: /json-demo/
6+
method: GET
7+
8+
template-source: json_demo.sql
9+
10+
# A connection is required by the endpoint loader; the SQL itself does not
11+
# touch the connection, so the existing parquet connection works.
12+
connection:
13+
- data-types-parquet
14+
15+
auth:
16+
enabled: false
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# Regression test for issue #38:
2+
# A column declared as DuckDB's JSON logical type must appear in the HTTP
3+
# response body as a nested JSON object, not as a JSON-escaped string.
4+
test_name: JSON-aliased VARCHAR columns are returned as nested JSON
5+
6+
stages:
7+
- name: GET /json-demo embeds payload as a nested object
8+
request:
9+
url: "{base_url}/json-demo/"
10+
method: GET
11+
response:
12+
status_code: 200
13+
headers:
14+
content-type: application/json
15+
json:
16+
data:
17+
- id: 1
18+
payload:
19+
a: 1
20+
b:
21+
- 10
22+
- 20
23+
c:
24+
nested: true
25+
next: !anything
26+
total_count: !anything

0 commit comments

Comments
 (0)