From 9d8d2a0cbc8ea3f71decb461fd72f776dbfbf475 Mon Sep 17 00:00:00 2001 From: func25 Date: Sat, 11 Jul 2026 00:04:56 +0700 Subject: [PATCH] fix --- app/vlselect/logsql/logsql.go | 47 ++++++++++++++++-------------- apptest/tests/csv_response_test.go | 16 ++++++++++ docs/victorialogs/CHANGELOG.md | 1 + lib/logstorage/parser.go | 2 +- lib/logstorage/parser_test.go | 1 + 5 files changed, 44 insertions(+), 23 deletions(-) diff --git a/app/vlselect/logsql/logsql.go b/app/vlselect/logsql/logsql.go index 2599ca2da9..ef5d7cc7b5 100644 --- a/app/vlselect/logsql/logsql.go +++ b/app/vlselect/logsql/logsql.go @@ -1207,8 +1207,29 @@ func ProcessQueryRequest(ctx context.Context, w http.ResponseWriter, r *http.Req } ca.q.AddPipeOffsetLimit(uint64(offset), uint64(limit)) } + defer ca.updatePerQueryStatsMetrics() + startTime := time.Now() var csvHeader []byte + sw := &syncWriter{ + w: w, + } + writeResponseHeadersOnce := sync.OnceFunc(func() { + // Write response headers + h := w.Header() + + if format == "csv" { + h.Set("Content-Type", "text/csv") + } else { + h.Set("Content-Type", "application/stream+json") + } + ca.writeResponseHeaders(h, startTime) + + if format == "csv" { + _, _ = sw.Write(csvHeader) + } + }) + if format == "csv" { fields, ok := ca.q.GetFixedFields() if !ok { @@ -1224,15 +1245,15 @@ func ProcessQueryRequest(ctx context.Context, w http.ResponseWriter, r *http.Req fields[i] = fieldName.Value } sort.Strings(fields) + if len(fields) == 0 { + writeResponseHeadersOnce() + return + } ca.q.AddPipeFields(fields) } csvHeader = appendCSVLine(nil, fields) } - sw := &syncWriter{ - w: w, - } - var bwShards atomicutil.Slice[bytesutil.ByteBuffer] defer func() { shards := bwShards.All() @@ -1243,23 +1264,6 @@ func ProcessQueryRequest(ctx context.Context, w http.ResponseWriter, r *http.Req } }() - startTime := time.Now() - writeResponseHeadersOnce := sync.OnceFunc(func() { - // Write response headers - h := w.Header() - - if format == "csv" { - h.Set("Content-Type", "text/csv") - } else { - h.Set("Content-Type", "application/stream+json") - } - ca.writeResponseHeaders(h, startTime) - - if format == "csv" { - _, _ = sw.Write(csvHeader) - } - }) - var appendRow func(dst []byte, columns []logstorage.BlockColumn, rowIdx int) []byte needSortFields := !ca.q.IsFixedOutputFieldsOrder() @@ -1292,7 +1296,6 @@ func ProcessQueryRequest(ctx context.Context, w http.ResponseWriter, r *http.Req } qctx := ca.newQueryContext(ctx) - defer ca.updatePerQueryStatsMetrics() // Execute the query if err := vlstorage.RunQuery(qctx, writeBlock); err != nil { diff --git a/apptest/tests/csv_response_test.go b/apptest/tests/csv_response_test.go index 886bfa60ba..1db366ed97 100644 --- a/apptest/tests/csv_response_test.go +++ b/apptest/tests/csv_response_test.go @@ -62,6 +62,14 @@ stats_pipe,{},2025-06-06T14:30:19.088007Z,,,false,12345,"[""foo"",""bar""]" case 2,{},2025-06-06T14:30:19.088007Z,,,false,12345,"[""foo"",""bar""]" ` f(query, responseExpected) + + // empty result after stats and filter pipes + query = `_stream:{service="does-not-exist"} | stats count() as hits | filter hits:> 0` + responseExpected = "hits\n" + f(query, responseExpected) + + // empty result without fixed fields + f("plain-no-match", "") } func TestVlclusterQueryCSVResponse(t *testing.T) { @@ -118,4 +126,12 @@ stats_pipe,{},2025-06-06T14:30:19.088007Z,,,false,12345,"[""foo"",""bar""]" case 2,{},2025-06-06T14:30:19.088007Z,,,false,12345,"[""foo"",""bar""]" ` f(query, responseExpected) + + // empty result after stats and filter pipes + query = `_stream:{service="does-not-exist"} | stats count() as hits | filter hits:> 0` + responseExpected = "hits\n" + f(query, responseExpected) + + // empty result without fixed fields + f("plain-no-match", "") } diff --git a/docs/victorialogs/CHANGELOG.md b/docs/victorialogs/CHANGELOG.md index 6196803026..1cce417205 100644 --- a/docs/victorialogs/CHANGELOG.md +++ b/docs/victorialogs/CHANGELOG.md @@ -35,6 +35,7 @@ according to the following docs: * FEATURE: [vlagent](https://docs.victoriametrics.com/victorialogs/vlagent/): add `-remoteWrite.basicAuth.usernameFile` command-line flag for dynamically reloading basic auth username for the corresponding `-remoteWrite.url` from the given file. * BUGFIX: [syslog data ingestion](https://docs.victoriametrics.com/victorialogs/data-ingestion/syslog/): avoid stamping [RFC3164](https://datatracker.ietf.org/doc/html/rfc3164) messages received right after the new year with the previous year (which could drop them under a short `-retentionPeriod`), and compute the year in `-syslog.timezone` instead of the server local timezone. See [#1556](https://github.com/VictoriaMetrics/VictoriaLogs/pull/1556). +* BUGFIX: [`/select/logsql/query`](https://docs.victoriametrics.com/victorialogs/querying/#querying-logs): avoid a `vlselect` crash when a CSV query returns no fields. See [#1592](https://github.com/VictoriaMetrics/VictoriaLogs/issues/1592). * BUGFIX: [LogsQL](https://docs.victoriametrics.com/victorialogs/logsql/): fix [`field_names` pipe](https://docs.victoriametrics.com/victorialogs/logsql/#field_names-pipe) not respecting [`hidden_fields_filters`](https://docs.victoriametrics.com/victorialogs/querying/#hidden-fields). Fields listed in `hidden_fields_filters` still appeared in `field_names` output, leaking their existence to the caller. See [#1574](https://github.com/VictoriaMetrics/VictoriaLogs/pull/1574/). * BUGFIX: [LogsQL](https://docs.victoriametrics.com/victorialogs/logsql/): fix a `vlselect` crash in [cluster mode](https://docs.victoriametrics.com/victorialogs/cluster/) on some valid queries. This happened when a [`math` pipe](https://docs.victoriametrics.com/victorialogs/logsql/#math-pipe) operand had the same name as a function (such as `abs`, `round` or `floor`), or when `from` was used as a separator in the [`split` pipe](https://docs.victoriametrics.com/victorialogs/logsql/#split-pipe). See [#1518](https://github.com/VictoriaMetrics/VictoriaLogs/issues/1518). * BUGFIX: [LogsQL](https://docs.victoriametrics.com/victorialogs/logsql/): allow [`filter` pipes](https://docs.victoriametrics.com/victorialogs/logsql/#filter-pipe) without the `filter` prefix when the filter starts with a non-word token or the `not` keyword, such as `... | !foo`, `... | {host="x"}`, `... | >5`, `... | =foo` or `... | not foo`. These were unintentionally rejected with `unexpected pipe` in [v1.51.0](https://github.com/VictoriaMetrics/VictoriaLogs/releases/tag/v1.51.0). A non-word token (and the `not` operator) cannot clash with a pipe name, so it is unambiguously a filter. See [#1522](https://github.com/VictoriaMetrics/VictoriaLogs/issues/1522). diff --git a/lib/logstorage/parser.go b/lib/logstorage/parser.go index 83a098b941..714697e540 100644 --- a/lib/logstorage/parser.go +++ b/lib/logstorage/parser.go @@ -846,7 +846,7 @@ func getFixedFields(pipes []pipe) ([]string, int) { for i := len(pipes) - 1; i >= 0; i-- { p := pipes[i] switch t := p.(type) { - case *pipeSort, *pipeLimit, *pipeOffset: + case *pipeFilter, *pipeSort, *pipeLimit, *pipeOffset: // these pipes do not change the fixed fields, so they are allowed after `fields` and `stats` case *pipeFields: fields, ok := t.resultFields() diff --git a/lib/logstorage/parser_test.go b/lib/logstorage/parser_test.go index 11b9c202fc..a76087db21 100644 --- a/lib/logstorage/parser_test.go +++ b/lib/logstorage/parser_test.go @@ -4982,6 +4982,7 @@ func TestQueryGetFixedFields_Success(t *testing.T) { f("* | count(), sum(x) as y", []string{"count(*)", "y"}) f("* | stats by (a, b) count(), sum(x) as y", []string{"a", "b", "count(*)", "y"}) + f("* | stats count() as hits | filter hits:>0", []string{"hits"}) f("* | stats by (a, b) count(), sum(x) as y | sort by (c desc)", []string{"c", "a", "b", "count(*)", "y"}) f("* | stats by (a, b) count(), sum(x) as y | offset 5", []string{"a", "b", "count(*)", "y"}) f("* | stats by (a, b) count(), sum(x) as y | limit 10", []string{"a", "b", "count(*)", "y"})