-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
fix(search&aggregate):fix error overwrite and typo #3220 #3224
base: master
Are you sure you want to change the base?
Conversation
* LOAD has NO AS param(https://redis.io/docs/latest/commands/ft.aggregate/) * fix typo: WITHCOUT -> WITHCOUNT
Fixes #3220 |
@bitsark the RE build is falling, a test related to ft.aggregate states it expects an error, but got nil. Seems related, can you look at it? |
sure, i will check the problem |
@bitsark I do see |
* Compatible with known RediSearch issue in test
@ndyakov i will recheck this problem |
@ndyakov the bug fixed in the pr is like: |
@bitsark i can see the As field removed in the diff. I do think the calculation of the arguments look odd and is probably broken, but the As should be left since it is possible to have an alias for a loaded field, don't you think? Can you provide an example go code to reproduce the bug you are referring to? |
@ndyakov , I reproduce the bug now. you are right. I misunderstood the bug. I will commit a new fix. |
* fixed the calculation bug of the count of load params
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bitsark let's fix the tests. I would suggest trying to update the RE version instead of having such condition in the tests.
@bitsark merged an update to fix the linter. |
LOAD has NO AS param(https://redis.io/docs/latest/commands/ft.aggregate/)
fix typo: WITHCOUT -> WITHCOUNT