Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 25 additions & 22 deletions app/vlselect/logsql/logsql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()
Expand 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()
Expand Down Expand Up @@ -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 {
Expand Down
16 changes: 16 additions & 0 deletions apptest/tests/csv_response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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", "")
}
1 change: 1 addition & 0 deletions docs/victorialogs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
2 changes: 1 addition & 1 deletion lib/logstorage/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions lib/logstorage/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
Expand Down