Skip to content
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.

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

build_args method for the AggregateRequest enforces incorrect order of clauses #40

Closed
filipecosta90 opened this issue Oct 1, 2019 · 0 comments · Fixed by #41
Closed

Comments

@filipecosta90
Copy link
Collaborator

filipecosta90 commented Oct 1, 2019

quoting @stryt2,

Hello,

Under the APPLY {expr} AS {name} section in parameters in detail from the official Redisearch page, it is said that

APPLY ... can be referenced by further APPLY / SORTBY / GROUPBY / REDUCE operations down the pipeline.

However, looking at the current build_args method for the AggregateRequest, the GROUPBY keyword and its fields always come before the APPLY keyword and its fields. E.g.

import redisearch

aggregate_request = redisearch.aggregation.AggregateRequest()
# Call 1
aggregate_request.apply(foo="@bar / 2").group_by("@foo", redisearch.reducers.count())
# or Call 2
# aggregate_request.group_by("@foo", redisearch.reducers.count()).apply(foo="@bar / 2")

print(aggregate_request.build_args())

would have 2 calls (Call 1 and Call 2) both resulting as (irrespective of the order of methods),

['*', 'GROUPBY', '1', '@baz', 'REDUCE', 'COUNT', '0', 'APPLY', '@bar / 2', 'AS', 'foo']

 

However, shouldn't the expected behaviour of Call 1 being

['*', 'APPLY', '@bar / 2', 'AS', 'foo', 'GROUPBY', '1', '@baz', 'REDUCE', 'COUNT', '0']

i.e. the order of the keywords will be dependant upon the order of call?

 

The reason why this is an issue is that Call 2 would result an error (if field foo does not originally exist) saying

No such property foo

whereas Call 1 should not.

 

Any help to get around this issue is greatly appreciated. Thanks.

Originally posted by @stryt2 in #21 (comment)

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 a pull request may close this issue.

1 participant