lib/logstorage: skip unsafe last-N optimization when pipes may overwrite _time

[immanuwell]

This fixes a kinda nasty last-N edge case.

If unpack_json, unpack_logfmt, extract, or extract_regexp can overwrite _time, a wide enough time range may hit the last-N fast path and fail with missing _time field in the query results. A narrow range works, so it is pretty easy to miss.

Fix is simple: skip that optimization for those unsafe cases. Safe cases with result_prefix still keep the fast path.

Repro

1. Insert a row like {"_msg":"{\"_time\":\"not-a-timestamp\",\"msg\":\"bad\"}","_time":"2025-01-01T01:00:00Z"}
2. Run * | unpack_json | keep _msg, _time | sort by (_time desc) limit 1 on 2025-01-01T01:00:00Z..2025-01-01T01:00:00.000000001Z
3. Run the same query on 2025-01-01T01:00:00Z..2025-01-01T01:00:03Z
4. Before this fix, step 3 returns 400. After this fix, both ranges work.

Tests:

• go test ./lib/logstorage
• go test ./apptest/tests -run "TestVlsingleLastnOptimization$|TestVlsingleLastnOptimizationWithUnpackedInvalidTime$" -count=1

Related to #1360.

[cubic-dev-ai]

1 issue found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="lib/logstorage/pipe_unpack_logfmt.go">

<violation number="1" location="lib/logstorage/pipe_unpack_logfmt.go:61">
P2: `resultPrefix != ""` is not a sufficient safety guarantee: `fieldsUnpackerContext.addField` concatenates `fieldPrefix + name` directly, so a prefix like `"_"` with a field named `"time"` produces `_time`. The `canReturnLastNResults` guard should account for whether any combination of `resultPrefix` and a matched field name could resolve to `_time`.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: resultPrefix != "" is not a sufficient safety guarantee: fieldsUnpackerContext.addField concatenates fieldPrefix + name directly, so a prefix like "_" with a field named "time" produces _time. The canReturnLastNResults guard should account for whether any combination of resultPrefix and a matched field name could resolve to _time.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At lib/logstorage/pipe_unpack_logfmt.go, line 61:

<comment>`resultPrefix != ""` is not a sufficient safety guarantee: `fieldsUnpackerContext.addField` concatenates `fieldPrefix + name` directly, so a prefix like `"_"` with a field named `"time"` produces `_time`. The `canReturnLastNResults` guard should account for whether any combination of `resultPrefix` and a matched field name could resolve to `_time`.</comment>

<file context>
@@ -58,9 +58,7 @@ func (pu *pipeUnpackLogfmt) canLiveTail() bool {
-	// TODO: verify that the unpacked fields do not overwrite _time with non-timestamp values.
-
-	return true
+	return pu.resultPrefix != "" || !prefixfilter.MatchFilters(pu.fieldFilters, "_time")
 }
 
</file context>