Skip to content

Conversation

@AnaNek
Copy link
Contributor

@AnaNek AnaNek commented Feb 11, 2021

Closes #116

@AnaNek AnaNek requested a review from dokshina February 11, 2021 15:33
@AnaNek AnaNek marked this pull request as draft February 11, 2021 15:49
Copy link
Contributor

@olegrok olegrok left a comment

Choose a reason for hiding this comment

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

Thanks for your patch! Consider several comments below.

{name = 'age', type = 'number'},
})
local objects = crud.unflatten_rows(result.rows, result.metadata)
t.assert_equals(objects, {{id = 1, name = 'Elizabeth', age = 24, bucket_id = 477}})
Copy link
Contributor

Choose a reason for hiding this comment

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

Why 477?

Copy link
Contributor

Choose a reason for hiding this comment

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

tarantool> vshard.router.bucket_id_strcrc32(1)
---
- 477
...

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, but maybe should do it like this?

t.assert_equals(objects, {{id = 1, name = 'Elizabeth', age = 24, bucket_id = vshard.router.bucket_id_strcrc32(1)}}) 

or maybe leave a comment on how 477 is formed

@AnaNek AnaNek marked this pull request as ready for review February 18, 2021 09:15
@AnaNek AnaNek force-pushed the support-partial-results branch from ff28ef2 to dab0848 Compare February 18, 2021 13:52
@dokshina
Copy link
Contributor

Thanks a lot for your patch! Me and @olegrok left two little comments.

Please, also update CHANGELOG and describe new option in README and then we can merge this PR!

Copy link
Contributor

@dokshina dokshina left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your patch!

Copy link
Contributor

@olegrok olegrok left a comment

Choose a reason for hiding this comment

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

Bless you

Don't forget to squash all your commit into single patch.

@AnaNek AnaNek merged commit d370ac3 into master Feb 19, 2021
@AnaNek AnaNek deleted the support-partial-results branch February 19, 2021 11:39
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.

[2pt] Simple operations: Partial results

5 participants