Skip to content

Commit faaf231

Browse files
committed
Add test on bug fixed by PR #157.
1 parent ed4e6fa commit faaf231

File tree

2 files changed

+134
-2
lines changed

2 files changed

+134
-2
lines changed

pkg/quickwit/response_parser.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,9 +280,11 @@ func processDocsToDataFrameFields(docs []map[string]interface{}, propNames []str
280280
rawPropSlice := getDocPropSlice[json.Number](docs, propName, size)
281281
propSlice := make([]*float64, size)
282282
for i, val := range rawPropSlice {
283+
// Skip nil values - can occur when field is missing from some documents
283284
if val == nil {
284285
continue
285286
}
287+
// Convert json.Number to float64, skip on conversion errors
286288
val_f64, err := val.Float64()
287289
if err == nil {
288290
propSlice[i] = &val_f64

pkg/quickwit/response_parser_test.go

Lines changed: 132 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,138 @@ func TestProcessLogsResponse(t *testing.T) {
272272
})
273273
}
274274

275+
func TestJsonNumberNilHandling(t *testing.T) {
276+
t.Run("processDocsToDataFrameFields handles nil json.Number values", func(t *testing.T) {
277+
// This test verifies the fix for PR #157 - prevents panic when some documents
278+
// are missing numeric fields that are present in other documents
279+
280+
// Create test documents where some have numeric fields and others don't
281+
docs := []map[string]any{
282+
{
283+
"timestamp": "2023-01-01T00:00:00Z",
284+
"message": "First log",
285+
"count": json.Number("123.45"), // This doc has count field
286+
"level": "info",
287+
},
288+
{
289+
"timestamp": "2023-01-01T00:01:00Z",
290+
"message": "Second log",
291+
// count field is missing in this doc - this would cause panic before fix
292+
"level": "warn",
293+
},
294+
{
295+
"timestamp": "2023-01-01T00:02:00Z",
296+
"message": "Third log",
297+
"count": json.Number("678.90"), // This doc has count field again
298+
"level": "error",
299+
},
300+
}
301+
302+
// Create sorted prop names including the problematic numeric field
303+
sortedPropNames := []string{"timestamp", "message", "count", "level"}
304+
305+
configuredFields := es.ConfiguredFields{
306+
TimeField: "timestamp",
307+
LogMessageField: "message",
308+
LogLevelField: "level",
309+
}
310+
311+
// This should not panic and should handle the missing field gracefully
312+
fields := processDocsToDataFrameFields(docs, sortedPropNames, configuredFields)
313+
314+
require.NotNil(t, fields)
315+
require.Len(t, fields, 4) // timestamp, message, count, level
316+
317+
// Find the count field
318+
var countField *data.Field
319+
for _, field := range fields {
320+
if field.Name == "count" {
321+
countField = field
322+
break
323+
}
324+
}
325+
326+
require.NotNil(t, countField, "count field should exist")
327+
require.Equal(t, data.FieldTypeNullableFloat64, countField.Type())
328+
require.Equal(t, 3, countField.Len())
329+
330+
// Verify the values: should have values for docs 0 and 2, nil for doc 1
331+
val0, ok := countField.At(0).(*float64)
332+
require.True(t, ok, "first value should be *float64")
333+
require.NotNil(t, val0, "first value should not be nil")
334+
require.Equal(t, 123.45, *val0)
335+
336+
val1 := countField.At(1)
337+
require.Nil(t, val1, "second value should be nil (missing field)")
338+
339+
val2, ok := countField.At(2).(*float64)
340+
require.True(t, ok, "third value should be *float64")
341+
require.NotNil(t, val2, "third value should not be nil")
342+
require.Equal(t, 678.90, *val2)
343+
})
344+
345+
t.Run("json.Number field conversion errors are handled gracefully", func(t *testing.T) {
346+
// Test that malformed json.Numbers don't crash the system
347+
docs := []map[string]any{
348+
{
349+
"timestamp": "2023-01-01T00:00:00Z",
350+
"message": "First log",
351+
"count": json.Number("123.45"), // Valid number
352+
},
353+
{
354+
"timestamp": "2023-01-01T00:01:00Z",
355+
"message": "Second log",
356+
"count": json.Number("invalid"), // Invalid number - should be skipped
357+
},
358+
{
359+
"timestamp": "2023-01-01T00:02:00Z",
360+
"message": "Third log",
361+
"count": json.Number("789.12"), // Valid number
362+
},
363+
}
364+
365+
sortedPropNames := []string{"timestamp", "message", "count"}
366+
configuredFields := es.ConfiguredFields{
367+
TimeField: "timestamp",
368+
LogMessageField: "message",
369+
}
370+
371+
// This should not panic and should handle conversion errors gracefully
372+
fields := processDocsToDataFrameFields(docs, sortedPropNames, configuredFields)
373+
374+
require.NotNil(t, fields)
375+
require.Len(t, fields, 3)
376+
377+
// Find the count field
378+
var countField *data.Field
379+
for _, field := range fields {
380+
if field.Name == "count" {
381+
countField = field
382+
break
383+
}
384+
}
385+
386+
require.NotNil(t, countField)
387+
require.Equal(t, data.FieldTypeNullableFloat64, countField.Type())
388+
389+
// First value should be valid
390+
val0, ok := countField.At(0).(*float64)
391+
require.True(t, ok)
392+
require.NotNil(t, val0)
393+
require.Equal(t, 123.45, *val0)
394+
395+
// Second value should be nil due to conversion error
396+
val1 := countField.At(1)
397+
require.Nil(t, val1, "invalid json.Number should result in nil")
398+
399+
// Third value should be valid
400+
val2, ok := countField.At(2).(*float64)
401+
require.True(t, ok)
402+
require.NotNil(t, val2)
403+
require.Equal(t, 789.12, *val2)
404+
})
405+
}
406+
275407
func TestProcessRawDataResponse(t *testing.T) {
276408
t.Run("Simple raw data query", func(t *testing.T) {
277409
targets := map[string]string{
@@ -3181,8 +3313,6 @@ func TestFlatten(t *testing.T) {
31813313

31823314
t.Run("Flattens multiple nested branches consistently", func(t *testing.T) {
31833315
// This test would have caught the currentDepth bug that was fixed in PR #156.
3184-
// The bug caused currentDepth to accumulate across sibling branches, leading to
3185-
// inconsistent flattening behavior where later branches hit maxDepth prematurely.
31863316
// We create deep nesting (8 levels) so the bug causes second branch to exceed maxDepth.
31873317
obj := map[string]interface{}{
31883318
"a": map[string]interface{}{

0 commit comments

Comments
 (0)