Skip to content

Conversation

@mriskyp
Copy link

@mriskyp mriskyp commented Mar 30, 2020

-I want to fix two bug:
case when aggregate returns only 1 data grouped, when do aggregate groupby reducer function on CLI, it returns 1 data. but when do in Golang code, it returns empty data, but total = 1, and err is nil. then I try to set if total > 0 , then return aggregate response, total 1, and err nil expected.

case 2: when I want to select limit 0 2, it returns default limit 0 10. then try to debug, found that if limit serialise only shown when (offset != 0 && num != 10). If either offset 0, or num is default, then actual is no limit append on the quest stmt. example limit 0 2. when I fix the if condition, it return exactly limit 0 2 and returns expected response search

@codecov
Copy link

codecov bot commented Mar 30, 2020

Codecov Report

Merging #46 into master will not change coverage by %.
The diff coverage is 88.88%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #46   +/-   ##
=======================================
  Coverage   63.53%   63.53%           
=======================================
  Files          12       12           
  Lines         905      905           
=======================================
  Hits          575      575           
  Misses        272      272           
  Partials       58       58           
Impacted Files Coverage Δ
redisearch/client.go 67.25% <75.00%> (ø)
redisearch/query.go 86.23% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9613a4f...2778128. Read the comment docs.

@filipecosta90
Copy link
Collaborator

Hi there @mriskyp , thank you for bringing this up.
With regards to issue 1 can you please share a CLI example of the output you're getting?

-I want to fix two bug:
case when aggregate returns only 1 data grouped, when do aggregate groupby reducer function on CLI, it returns 1 data. but when do in Golang code, it returns empty data, but total = 1, and err is nil. then I try to set if total > 0 , then return aggregate response, total 1, and err nil expected.


case 2: when I want to select limit 0 2, it returns default limit 0 10. then try to debug, found that if limit serialise only shown when (offset != 0 && num != 10). If either offset 0, or num is default, then actual is no limit append on the quest stmt. example limit 0 2. when I fix the if condition, it return exactly limit 0 2 and returns expected response search

For this specific case the code is working as expected.


func TestPaging_serialize(t *testing.T) {
	type fields struct {
		Offset int
		Num    int
	}
	tests := []struct {
		name   string
		fields fields
		want   redis.Args
	}{
		{"default", fields{0, 10}, redis.Args{}},
		{"0-1000", fields{0, 1000}, redis.Args{"LIMIT", 0, 1000}},
		{"0-2", fields{0, 2}, redis.Args{"LIMIT", 0, 2}},
		{"100-200", fields{100, 200}, redis.Args{"LIMIT", 100, 200}},
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			p := Paging{
				Offset: tt.fields.Offset,
				Num:    tt.fields.Num,
			}
			if got := p.serialize(); !reflect.DeepEqual(got, tt.want) {
				t.Errorf("serialize() = %v, want %v", got, tt.want)
			}
		})
	}
}

I would suggest spliting the issues 1 and 2 into two different issues ( with the last one not really being an issue imo since the added test "0-2" is working as expected ).
Kind regards :)

@mriskyp
Copy link
Author

mriskyp commented Mar 31, 2020

Hi there @filipecosta90 ,
#Issue 2

for example, I have CLI
FT.SEARCH folder "@is_deleted:false @user_id:[1 1]" RETURN 10 folder_id folder_uuid folder_name folder_image_url description folder_status user_id is_deleted created_at updated_at LIMIT 0 2 SORTBY folder_id DESC
Then return exactly limited 2 data. But the total counter is 9.

I have params:
user_id=1&limit=2&page=1&offset=0
By the param, I setup limit(0 2) and append on the query builder

Then the builder search I have,
filterSelectFolderStatement := fmt.Sprintf("@is_deleted:false @user_id:[%d %d]", userID, userID)
querySelectListFolder := redisearch.NewQuery(filterSelectFolderStatement).Limit(0), 2).SetSortBy("folder_id", false).
SetReturnFields("folder_id", "folder_uuid", "folder_name",
"folder_image_url", "description", "folder_status",
"user_id", "is_deleted", "created_at",
"updated_at",
)

The debugged query statement stated as below on the paging struct
query select folder &{Raw:@is_deleted:false @user_id:[1 1] Paging:{Offset
:0 Num:2} Flags:0 Slop:0 Filters:[] InKeys:[] ReturnFields:[folder_id fol
der_uuid folder_name folder_image_url description folder_status user_id i
s_deleted created_at updated_at] Language: Expander: Scorer: Payload:[] S
ortBy:0xc0013c51c0 HighlightOpts: SummarizeOpts:}running select
list folder

Then I want to show output from the appended redis value on the serialise
args := redis.Args{i.name}
args = append(args, q.serialize()...)

fmt.Printf("appended args %+v", args...)

res, err := redis.Values(conn.Do("FT.SEARCH", args...))
if err != nil {
    return
}

appended args folder%!(EXTRA string=@is_deleted:false @user_id:[1 1], string=RETURN, int=10, string=folder_id, string=folder_uuid, string=folder_name, string=folder_image_url, string=description, string=folder_status, string=user_id, string=is_deleted, string=created_at, string=updated_at, string=SORTBY, string=folder_id, string=DESC)

It returns data redis total 9 caused the limit not appended on the serialise.

Then have a look on the code:
// only serialize something if it's different than the default
// The default is 0 10
if p.Offset != DefaultOffset && p.Num != DefaultNum {
args = args.Add("LIMIT", p.Offset, p.Num)
}
return args

It means, when I have default offset 0 and not default num 2,
It will have false condition on the first if.
If 0 != 0 && 2 != 10
Then the limit not appended on the args. Then serialise paging {0 2} return args without limit

Then I try to fix the if statement, meaning if offset is default and num is default too, just return the args without limit. Otherwise, add limit.
if !(p.Offset == DefaultOffset && p.Num == DefaultNum) {
args = args.Add("LIMIT", p.Offset, p.Num)
}
return args

When do this, the output is expected
    "page": 1,
    "totalData": 2,

and the limit finally appended too
appended args folder%!(EXTRA string=@is_deleted:false @user_id:[1 1], str
ing=LIMIT, int=0, int=2, string=RETURN, int=10, string=folder_id, string=
folder_uuid, string=folder_name, string=folder_image_url, string=descript
ion, string=folder_status, string=user_id, string=is_deleted, string=crea
ted_at, string=updated_at, string=SORTBY, string=folder_id, string=DESC)

And the data actual is expected return which is 2 limited data and total counter is 2

@mriskyp
Copy link
Author

mriskyp commented Mar 31, 2020

Issue #1

Issue 1
For example I have cli

On the cli, it returns the data,

FT.AGGREGATE folder_article "@is_deleted:false @user_id:[1 1]" GROUPBY 1 @folder_id REDUCE count 1 @folder_article_id as num
[
1,
[
"folder_id",
"17",
"num",
"1"
]
]

Then I try to running the Golang aggregate based on my redis data specified by the aggregate statement.
Then the return data is empty, but the total value is 1, and the err nil.

Then I try to debug when the data is return 1 on the cli, run the code

debug aggregate properties struct:
stmt agr &{Query:0xc00104a7e0 AggregatePlan:[GROUPBY 1 @folder_id REDUCE
COUNT 1 @folder_article_id AS article_count] Paging: Max:0 WithSche
ma:false Verbatim:false WithCursor:false Cursor:}

Debug:
stmt aggregate val folder_article%!(EXTRA string=@is_deleted:false @user_
id:[1 1], string=GROUPBY, int=1, string=@folder_id, string=REDUCE, redise
arch.GroupByReducers=COUNT, int=1, string=@folder_article_id, string=AS,
string=article_count)
aggregate data response [] total 1 err nil

total = len(res) - 1
The total now only 1
if total > 1 {
aggregateReply = ProcessAggResponse(res[1:])
}

Then, I fix by if total > 0 for both !hasValidCursor and the partial

Now, It returns the expected output same as the cli returns

@mriskyp
Copy link
Author

mriskyp commented Mar 31, 2020

I would suggest spliting the issues 1 and 2 into two different issues ( with the last one not really being an issue imo since the added test "0-2" is working as expected ).
Kind regards :)

sure @filipecosta90 , will be splitting into two diff issues later.
Additionally, the issue I had, being explained above by inquiry of debugging the cli redi insight browser, and the installed Golang package client itself by running few endpoint regard the search and aggregate. kindly check it too mr @filipecosta90 thank you for the reply

@filipecosta90
Copy link
Collaborator

hi there @mriskyp , thank you for the explanation. It's indeed correct the aggregation's issue. Merging your fix and adding more testing.
Excelent catch 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FT.AGGREGATE response parsing failing for single aggregate result

2 participants