ARROW-4168: [GLib] Use property to keep GArrowDataType passed in garrow_field_new() #3322
Conversation
kou
left a comment
kou
left a comment
There was a problem hiding this comment.
Thanks!
Can you check my comments?
c_glib/arrow-glib/column.cpp
Outdated
| auto arrow_field = arrow_column->field(); | ||
| return garrow_field_new_raw(&arrow_field); | ||
| return garrow_field_new_raw(&arrow_field, | ||
| garrow_column_get_data_type(column)); |
There was a problem hiding this comment.
We need to call g_object_unref() for GArrowDataType * returned by garrow_column_get_data_type(). Because garrow_column_get_data_type() creates a new GArrowDataType.
| * @field: A #GArrowField. | ||
| * | ||
| * Returns: (transfer full): The data type of the field. | ||
| * Returns: (transfer none): The data type of the field. |
There was a problem hiding this comment.
This breaks backward compatibility for C API users (not for GObject Introspection based bindings users).
Normally, it's not desired. But we can accept this for this case because users who use this function may not exist.
| auto field = garrow_field_new_raw(&arrow_field); | ||
| auto field = garrow_field_new_raw(&arrow_field, | ||
| GARROW_DATA_TYPE(list_data_type)); | ||
|
|
There was a problem hiding this comment.
Can we reuse GARROW_DATA_TYPE(list_data_type) result for garrow_data_type_get_raw(GARROW_DATA_TYPE(list_data_type))?
There was a problem hiding this comment.
I've reused GARROW_DATA_TYPE(list_data_type).
kou
left a comment
kou
left a comment
There was a problem hiding this comment.
+1
I'll merge this when CI is passed.
2 participants
This is follow-up of #3197 (review)