Skip to content

Commit

Permalink
fix: Ensure tags are never null (#25680)
Browse files Browse the repository at this point in the history
* fix: Ensure tags are never null

This injects empty strings into tags for any rows in the buffer where the tag value is null. This is required because the tags are what make up the series key, which must have all non-null values.

There is an ongoing discussion about what the real behavior should be here, but for now this will get our users running that break without this behavior. Discussion is in #25674.

Fixes #25648

* fix: clippy failures
  • Loading branch information
pauldix authored Dec 18, 2024
1 parent 93222f7 commit 5657640
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 4 deletions.
46 changes: 46 additions & 0 deletions influxdb3/tests/server/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1330,3 +1330,49 @@ async fn api_v3_query_sql_meta_cache() {
resp
);
}

/// Test that if we write in a row of LP that is missing a tag value, we're still able to query.
#[tokio::test]
async fn api_v3_query_null_tag_values() {
let server = TestServer::spawn().await;

server
.write_lp_to_db(
"foo",
"cpu,host=a,region=us-east usage=0.9 1
cpu,host=b usage=0.80 4",
Precision::Second,
)
.await
.unwrap();

let client = reqwest::Client::new();
let url = format!("{base}/api/v3/query_sql", base = server.client_addr());

let resp = client
.get(&url)
.query(&[
("db", "foo"),
(
"q",
"SELECT host, region, time, usage FROM cpu ORDER BY time",
),
("format", "pretty"),
])
.send()
.await
.unwrap()
.text()
.await
.unwrap();

assert_eq!(
"+------+---------+---------------------+-------+\n\
| host | region | time | usage |\n\
+------+---------+---------------------+-------+\n\
| a | us-east | 1970-01-01T00:00:01 | 0.9 |\n\
| b | | 1970-01-01T00:00:04 | 0.8 |\n\
+------+---------+---------------------+-------+",
resp
);
}
11 changes: 7 additions & 4 deletions influxdb3_write/src/write_buffer/table_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ impl MutableTableChunk {
let mut tag_builder = StringDictionaryBuilder::new();
// append nulls for all previous rows
for _ in 0..(row_index + self.row_count) {
tag_builder.append_null();
tag_builder.append_value("");
}
e.insert(Builder::Tag(tag_builder));
}
Expand Down Expand Up @@ -390,8 +390,8 @@ impl MutableTableChunk {
Builder::I64(b) => b.append_null(),
Builder::U64(b) => b.append_null(),
Builder::String(b) => b.append_null(),
Builder::Tag(b) => b.append_null(),
Builder::Key(b) => b.append_null(),
Builder::Tag(b) => b.append_value(""),
Builder::Key(b) => b.append_value(""),
Builder::Time(b) => b.append_null(),
}
}
Expand Down Expand Up @@ -467,7 +467,10 @@ impl MutableTableChunk {
schema_builder.influx_column(col_name.as_ref(), InfluxColumnType::Tag);
let mut tag_builder: StringDictionaryBuilder<Int32Type> =
StringDictionaryBuilder::new();
tag_builder.append_nulls(self.row_count);
for _ in 0..self.row_count {
tag_builder.append_value("");
}

cols.push(Arc::new(tag_builder.finish()));
}
}
Expand Down

0 comments on commit 5657640

Please sign in to comment.