Skip to content

Add time-related query tests and enhance query result handling for da… #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MuhammadQadora wants to merge 8 commits into
base: master
Choose a base branch
from
+90 −1
Open

Add time-related query tests and enhance query result handling for da… #34

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7e49f57
Add time-related query tests and enhance query result handling for da…
MuhammadQadora Jul 9, 2025
d5d7598
Update TestGetDate to include month and day in date query; restore VA…
MuhammadQadora Jul 9, 2025
0a000c3
fix example names make go build . greate again
barakb Sep 10, 2025
7db0a62
add duration
barakb Sep 10, 2025
8d39a9c
fix test
barakb Sep 10, 2025
97b3357
Merge branch 'master' into 32-add-support-for-date-time-and-datetime-…
MuhammadQadora Jan 1, 2026
649290c
Improve error handling in time-related test cases
MuhammadQadora Jan 1, 2026
e452100
Merge branch 'master' into 32-add-support-for-date-time-and-datetime-…
MuhammadQadora Jan 1, 2026
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
35 changes: 35 additions & 0 deletions client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package falkordb
import (
"os"
"testing"
"time"

"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -307,7 +308,41 @@ func TestVectorF32(t *testing.T) {
vec := r.GetByIndex(0).([]float32)
assert.Equal(t, vec, []float32{1.0, 2.0, 3.0}, "Unexpected vector value")
}
func TestGetTime(t *testing.T) {
q := "RETURN localtime({hour: 12}) AS time"
res, err := graph.Query(q, nil, nil)
if err != nil {
t.Error(err)
}
res.Next()
r := res.Record()
timeValue := r.GetByIndex(0).(time.Time)
assert.Equal(t, timeValue.Hour(), 12, "Unexpected Time value")
}
Comment on lines +358 to +377

@coderabbitai coderabbitai Bot Sep 10, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Harden TestGetTime: fix assert arg order and add error/empty checks.

Current failure stems from reversed assert args and potential timezone drift. After normalizing to UTC in parser, update the test for robustness.

 func TestGetTime(t *testing.T) {
   q := "RETURN localtime({hour: 12}) AS time"
   res, err := graph.Query(q, nil, nil)
-  if err != nil {
-    t.Error(err)
-  }
-  res.Next()
-  r := res.Record()
-  timeValue := r.GetByIndex(0).(time.Time)
-  assert.Equal(t, timeValue.Hour(), 12, "Unexpected Time value")
+  if err != nil { t.Fatal(err) }
+  if res == nil { t.Fatal("Query result is nil") }
+  if !res.Next() { t.Fatal("empty result") }
+  r := res.Record()
+  val := r.GetByIndex(0)
+  timeValue, ok := val.(time.Time)
+  if !ok { t.Fatalf("Expected time.Time, got %T", val) }
+  assert.Equal(t, 12, timeValue.Hour(), "Unexpected Time value")
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func TestGetTime(t *testing.T) {
q := "RETURN localtime({hour: 12}) AS time"
res, err := graph.Query(q, nil, nil)
if err != nil {
t.Error(err)
}
res.Next()
r := res.Record()
timeValue := r.GetByIndex(0).(time.Time)
assert.Equal(t, timeValue.Hour(), 12, "Unexpected Time value")
}
func TestGetTime(t *testing.T) {
q := "RETURN localtime({hour: 12}) AS time"
res, err := graph.Query(q, nil, nil)
if err != nil { t.Fatal(err) }
if res == nil { t.Fatal("Query result is nil") }
if !res.Next() { t.Fatal("empty result") }
r := res.Record()
val := r.GetByIndex(0)
timeValue, ok := val.(time.Time)
if !ok { t.Fatalf("Expected time.Time, got %T", val) }
assert.Equal(t, 12, timeValue.Hour(), "Unexpected Time value")
}
🧰 Tools
🪛 GitHub Actions: Go

[error] 320-320: TestGetTime failed: Not equal: expected: 10, actual: 12. Unexpected Time value. Command: go test -v ./...

🤖 Prompt for AI Agents 
In client_test.go around lines 311 to 321, the TestGetTime has reversed assert
args and lacks checks for query errors/empty results and type assertion; update
the test to (1) fail immediately on err from graph.Query, (2) ensure res.Next()
returned true and fail the test if not, (3) retrieve the record and safely
type-assert the first field with an ok check, (4) convert the obtained time to
UTC before asserting the hour, and (5) call assert.Equal with expected value
first (12) and actual value second to fix argument order.


func TestGetDate(t *testing.T) {
q := "RETURN date({year : 1984}) as date"
res, err := graph.Query(q, nil, nil)
if err != nil {
t.Error(err)
}
res.Next()
r := res.Record()
dateValue := r.GetByIndex(0).(time.Time)
assert.Equal(t, dateValue.Year(), 1984, "Unexpected Date value")
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

func TestGetDateTime(t *testing.T) {
q := "RETURN localdatetime({year : 1984}) as date"
res, err := graph.Query(q, nil, nil)
if err != nil {
t.Error(err)
}
res.Next()
r := res.Record()
dateTimeValue := r.GetByIndex(0).(time.Time)
assert.Equal(t, dateTimeValue.Year(), 1984, "Unexpected DateTime value")
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
func TestParameterizedQuery(t *testing.T) {
createGraph()
params := []interface{}{int64(1), 2.3, "str", true, false, nil, []interface{}{int64(0), int64(1), int64(2)}, []interface{}{"0", "1", "2"}}
Expand Down
13 changes: 13 additions & 0 deletions query_result.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"strconv"
"strings"
"time"

"github.com/olekukonko/tablewriter"
)
Expand Down Expand Up @@ -48,6 +49,9 @@ const (
VALUE_MAP
VALUE_POINT
VALUE_VECTORF32
VALUE_DATETIME // Deprecated, use VALUE_POINT instead
Comment thread
barakb marked this conversation as resolved.
Outdated

Copilot AI Jul 9, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The VALUE_DATETIME constant is marked deprecated but is still actively parsed. Consider removing it or updating the comment to reflect current usage or migration plan.

Suggested change
VALUE_DATETIME // Deprecated, use VALUE_POINT instead
VALUE_DATETIME // Deprecated: Retained for backward compatibility. Migrate to VALUE_POINT where possible.

Copilot uses AI. Check for mistakes.
VALUE_DATE
VALUE_TIME
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
)

type QueryResultHeader struct {
Expand Down Expand Up @@ -341,6 +345,15 @@ func (qr *QueryResult) parseScalar(cell []interface{}) (interface{}, error) {
case VALUE_VECTORF32:
return qr.parseVectorF32(v)

case VALUE_DATETIME:
return time.Unix(v.(int64), 0), nil

case VALUE_DATE:
return time.Unix(v.(int64), 0), nil

case VALUE_TIME:
return time.UnixMilli(v.(int64)), nil
Comment on lines +351 to +357

Copilot AI Jul 9, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Directly asserting v.(int64) may panic if the stored value isn’t an int64. Use a safe type check (t, ok := v.(int64)) and return an error if the assertion fails.

Suggested change
return time.Unix(v.(int64), 0), nil
case VALUE_DATE:
return time.Unix(v.(int64), 0), nil
case VALUE_TIME:
return time.UnixMilli(v.(int64)), nil
timestamp, ok := v.(int64)
if !ok {
return nil, errors.New("expected int64 for VALUE_DATETIME")
}
return time.Unix(timestamp, 0), nil
case VALUE_DATE:
timestamp, ok := v.(int64)
if !ok {
return nil, errors.New("expected int64 for VALUE_DATE")
}
return time.Unix(timestamp, 0), nil
case VALUE_TIME:
timestamp, ok := v.(int64)
if !ok {
return nil, errors.New("expected int64 for VALUE_TIME")
}
return time.UnixMilli(timestamp), nil

Copilot uses AI. Check for mistakes.

Comment on lines +350 to +358

@coderabbitai coderabbitai Bot Sep 10, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix timezone drift: normalize temporal values to UTC.

Tests show hour mismatch for localtime; time.Unix/UnixMilli returns a Time in Local zone. Normalize to UTC to keep deterministic values across environments.

Apply:

 case VALUE_DATETIME:
-    return time.Unix(v.(int64), 0), nil
+    return time.Unix(v.(int64), 0).UTC(), nil

 case VALUE_DATE:
-    return time.Unix(v.(int64), 0), nil
+    return time.Unix(v.(int64), 0).UTC(), nil

 case VALUE_TIME:
-    return time.UnixMilli(v.(int64)), nil
+    return time.UnixMilli(v.(int64)).UTC(), nil
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
case VALUE_DATETIME:
return time.Unix(v.(int64), 0), nil
case VALUE_DATE:
return time.Unix(v.(int64), 0), nil
case VALUE_TIME:
return time.UnixMilli(v.(int64)), nil
case VALUE_DATETIME:
return time.Unix(v.(int64), 0).UTC(), nil
case VALUE_DATE:
return time.Unix(v.(int64), 0).UTC(), nil
case VALUE_TIME:
return time.UnixMilli(v.(int64)).UTC(), nil
🤖 Prompt for AI Agents 
In query_result.go around lines 349 to 357, the temporal cases return time
values in the local zone causing timezone drift; update each return to normalize
to UTC by calling .UTC() on the constructed Time (e.g. time.Unix(v.(int64),
0).UTC(), and time.UnixMilli(v.(int64)).UTC()) so returned times are
deterministic across environments.

case VALUE_UNKNOWN:
return nil, errors.New("unknown scalar type")
}
Expand Down
Loading