Copilot commented on code in PR #948:
URL:
https://github.com/apache/skywalking-banyandb/pull/948#discussion_r2702792165
##########
banyand/internal/sidx/interfaces.go:
##########
@@ -75,10 +75,11 @@ type SIDX interface {
// WriteRequest contains data for a single write operation within a batch.
// The user provides the ordering key as an int64 value that sidx treats
opaquely.
type WriteRequest struct {
- Data []byte
- Tags []Tag
- SeriesID common.SeriesID
- Key int64
+ Data []byte
+ Tags []Tag
+ SeriesID common.SeriesID
+ Key int64
+ Timestamp int64 // Unix nanoseconds timestamp for time range filtering
Review Comment:
The Timestamp field in WriteRequest should have a comment explaining that a
value of 0 is treated as "not set" and will be ignored during timestamp range
computation. This would help API users understand the expected behavior.
```suggestion
Timestamp int64 // Unix nanoseconds timestamp for time range filtering;
0 means "not set" and is ignored during timestamp range computation
```
##########
banyand/internal/sidx/block_writer.go:
##########
@@ -324,6 +330,26 @@ func (bw *blockWriter) mustWriteBlock(sid common.SeriesID,
b *block) {
}
bw.minKeyLast = minKey
+ // Update timestamp ranges
+ if len(timestamps) > 0 {
+ for _, ts := range timestamps {
+ if ts != 0 {
+ if !bw.hasTimestamp {
+ bw.totalMinTimestamp = ts
+ bw.totalMaxTimestamp = ts
+ bw.hasTimestamp = true
+ } else {
+ if ts < bw.totalMinTimestamp {
+ bw.totalMinTimestamp = ts
+ }
+ if ts > bw.totalMaxTimestamp {
+ bw.totalMaxTimestamp = ts
+ }
+ }
+ }
+ }
+ }
Review Comment:
The timestamp tracking logic skips timestamps with value 0 (line 336). While
this is likely intentional to distinguish "not set" from actual timestamps,
consider documenting this behavior in the code or interface documentation. Unix
epoch (timestamp 0) is technically a valid timestamp but would be ignored by
this implementation. For a time-series database, this is probably acceptable as
data from 1970 is unlikely, but it should be documented for clarity.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
