-
Notifications
You must be signed in to change notification settings - Fork 116
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
group: support single reference weight vector #1366
Conversation
5f1e2fc
to
85e7ebb
Compare
lib/group.c
Outdated
@@ -756,10 +756,19 @@ grn_table_group_single_key_records_foreach_var_size_reference( | |||
grn_io_win jw; | |||
uint32_t value_size; | |||
grn_id *references = grn_ja_ref(ctx, ja, value_id, &jw, &value_size); |
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.
How about renaming it to value
because this may be reference + weight
array?
grn_id *references = grn_ja_ref(ctx, ja, value_id, &jw, &value_size); | |
uint8_t *value = grn_ja_ref(ctx, ja, value_id, &jw, &value_size); |
lib/group.c
Outdated
@@ -756,10 +756,19 @@ grn_table_group_single_key_records_foreach_var_size_reference( | |||
grn_io_win jw; | |||
uint32_t value_size; | |||
grn_id *references = grn_ja_ref(ctx, ja, value_id, &jw, &value_size); | |||
uint32_t n_references = value_size / sizeof(grn_id); | |||
uint32_t element_size; | |||
grn_bool is_weight = grn_obj_is_weight_vector_column(ctx, data->key.object); |
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.
We should use bool
instead of grn_bool
for newly written code. And could you use with_weight
instead of is_weight
because the variable name doesn't include _column
or something?
grn_bool is_weight = grn_obj_is_weight_vector_column(ctx, data->key.object); | |
bool with_weight = grn_obj_is_weight_vector_column(ctx, data->key.object); |
lib/group.c
Outdated
uint32_t i; | ||
|
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.
I like no new line here because the following for
is related to this i
:
85e7ebb
to
d239b37
Compare
Thanks! |
Thanks for your reviewing and merged! |
No description provided.