Skip to content

Commit d1c10f4

Browse files
hiltontjhiltontj
authored
fix: backfill new tags with NULL instead of empty string (#26446)
* fix: backfill new tags with NULL instead of empty string * refactor: use helper for append_null * test: add a test to check null back/forward fill
1 parent 4a917c5 commit d1c10f4

File tree

5 files changed

+92
-34
lines changed

5 files changed

+92
-34
lines changed

‎influxdb3/tests/cli/mod.rs‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2918,3 +2918,53 @@ async fn test_wal_overwritten() {
29182918
);
29192919
assert!(!p2.is_stopped(), "p2 should not be stopped");
29202920
}
2921+
2922+
#[test_log::test(tokio::test)]
2923+
async fn test_query_with_null_tags() {
2924+
use influxdb3_client::Precision;
2925+
let server = TestServer::configure().spawn().await;
2926+
server
2927+
.write_lp_to_db(
2928+
"mydb",
2929+
"\
2930+
foo val=10 1234\n\
2931+
foo,tag=bar val=42 1235\n\
2932+
foo val=1337 1236\n\
2933+
",
2934+
Precision::Second,
2935+
)
2936+
.await
2937+
.unwrap();
2938+
2939+
// query response is s JSON array:
2940+
let json = server
2941+
.query_sql("mydb")
2942+
.with_sql(
2943+
"\
2944+
SELECT \
2945+
tag as value, \
2946+
(tag IS NULL) AS is_null, \
2947+
(tag = '') AS is_empty \
2948+
FROM foo \
2949+
ORDER BY time",
2950+
)
2951+
.run()
2952+
.unwrap();
2953+
2954+
debug!("JSON output:\n\n{json:#}");
2955+
2956+
// first row has a NULL:
2957+
assert_eq!(json[0]["is_null"], true);
2958+
assert!(json[0]["is_empty"].is_null());
2959+
assert!(json[0]["value"].is_null());
2960+
2961+
// second row has a value:
2962+
assert_eq!(json[1]["is_null"], false);
2963+
assert_eq!(json[1]["is_empty"], false);
2964+
assert_eq!(json[1]["value"], "bar");
2965+
2966+
// third row has a NULL:
2967+
assert_eq!(json[2]["is_null"], true);
2968+
assert!(json[2]["is_empty"].is_null());
2969+
assert!(json[2]["value"].is_null());
2970+
}

‎influxdb3/tests/server/snapshots/lib__server__query__api_v1_query_group_by_with_nulls-2.snap‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,16 @@ expression: values
1515
],
1616
"name": "bar",
1717
"tags": {
18-
"t1": ""
18+
"t1": "a"
1919
},
2020
"values": [
2121
[
22-
"2065-01-07T17:28:52Z",
23-
2.0
22+
"2065-01-07T17:28:51Z",
23+
1.0
24+
],
25+
[
26+
"2065-01-07T17:28:53Z",
27+
3.0
2428
]
2529
]
2630
}
@@ -40,16 +44,12 @@ expression: values
4044
],
4145
"name": "bar",
4246
"tags": {
43-
"t1": "a"
47+
"t1": ""
4448
},
4549
"values": [
4650
[
47-
"2065-01-07T17:28:51Z",
48-
1.0
49-
],
50-
[
51-
"2065-01-07T17:28:53Z",
52-
3.0
51+
"2065-01-07T17:28:52Z",
52+
2.0
5353
]
5454
]
5555
}

‎influxdb3/tests/server/snapshots/lib__server__query__api_v1_query_group_by_with_nulls.snap‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,12 @@ expression: values
1515
],
1616
"name": "bar",
1717
"tags": {
18-
"t1": "a"
18+
"t1": ""
1919
},
2020
"values": [
2121
[
22-
"2065-01-07T17:28:51Z",
23-
1.0
24-
],
25-
[
26-
"2065-01-07T17:28:53Z",
27-
3.0
22+
"2065-01-07T17:28:52Z",
23+
2.0
2824
]
2925
]
3026
},
@@ -35,12 +31,16 @@ expression: values
3531
],
3632
"name": "bar",
3733
"tags": {
38-
"t1": ""
34+
"t1": "a"
3935
},
4036
"values": [
4137
[
42-
"2065-01-07T17:28:52Z",
43-
2.0
38+
"2065-01-07T17:28:51Z",
39+
1.0
40+
],
41+
[
42+
"2065-01-07T17:28:53Z",
43+
3.0
4444
]
4545
]
4646
}

‎influxdb3_server/src/http/v1.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,14 @@ impl QueryResponseStream {
628628
let column_name = field.name();
629629

630630
let mut cell_value = if !column.is_valid(row_index) {
631-
continue;
631+
// NB(trevor): when doing a GROUP BY, the /query API will insert an empty
632+
// string instead of a JSON `null` for tags in the group by clause whose
633+
// value is `NULL`.
634+
if column_map.is_group_by_tag(column_name) {
635+
Value::String("".to_string())
636+
} else {
637+
continue;
638+
}
632639
} else {
633640
cast_column_value(column, row_index)?
634641
};

‎influxdb3_write/src/write_buffer/table_buffer.rs‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ impl MutableTableChunk {
252252
let mut tag_builder = StringDictionaryBuilder::new();
253253
// append nulls for all previous rows
254254
for _ in 0..(row_index + self.row_count) {
255-
tag_builder.append_value("");
255+
tag_builder.append_null();
256256
}
257257
e.insert(Builder::Tag(tag_builder));
258258
}
@@ -337,18 +337,7 @@ impl MutableTableChunk {
337337
// add nulls for any columns not present
338338
for (column_id, builder) in &mut self.data {
339339
if !value_added.contains(column_id) {
340-
match builder {
341-
Builder::Bool(b) => b.append_null(),
342-
Builder::F64(b) => b.append_null(),
343-
Builder::I64(b) => b.append_null(),
344-
Builder::U64(b) => b.append_null(),
345-
Builder::String(b) => b.append_null(),
346-
Builder::Tag(b) => {
347-
// NOTE: we use an empty string "" for tags that are omitted
348-
b.append_value("");
349-
}
350-
Builder::Time(b) => b.append_null(),
351-
}
340+
builder.append_null();
352341
}
353342
}
354343
}
@@ -513,6 +502,18 @@ impl Builder {
513502
}
514503
}
515504

505+
fn append_null(&mut self) {
506+
match self {
507+
Builder::Bool(b) => b.append_null(),
508+
Builder::I64(b) => b.append_null(),
509+
Builder::F64(b) => b.append_null(),
510+
Builder::U64(b) => b.append_null(),
511+
Builder::String(b) => b.append_null(),
512+
Builder::Tag(b) => b.append_null(),
513+
Builder::Time(b) => b.append_null(),
514+
}
515+
}
516+
516517
fn into_influxcol_and_arrow(self) -> (InfluxColumnType, ArrayRef) {
517518
match self {
518519
Self::Bool(mut b) => (

0 commit comments

Comments
 (0)