yuqi1129 commented on code in PR #13432:
URL: https://github.com/apache/gravitino/pull/13432#discussion_r4123421756
##########
clients/client-python/gravitino/dto/rel/indexes/json_serdes/index_serdes.py:
##########
@@ -51,5 +53,8 @@ def deserialize(cls, data: dict[str, Any]) -> Index:
index_type = Index.IndexType(data[cls.INDEX_TYPE].upper())
return IndexDTO(
- index_type, data.get(cls.INDEX_NAME), data[cls.INDEX_FIELD_NAMES]
+ index_type,
+ data.get(cls.INDEX_NAME),
+ data[cls.INDEX_FIELD_NAMES],
+ data.get(cls.INDEX_PROPERTIES),
Review Comment:
Could we also preserve `properties` in `IndexSerdes.serialize()` and add a
round-trip test for both legacy index types? This change retains the properties
on deserialization, but the serializer still only emits `indexType`, `name`,
and `fieldNames`. I reproduced `deserialize -> serialize -> deserialize` with
the Annoy fixture: the first object has `annoy_trees`, `clickhouse_type_full`,
and `granularity`, while the round-tripped object has `{}`.
`TableDTO.to_json()` uses this serializer too, so exporting loaded table
metadata loses these values. Please emit the properties when present and assert
the actual property maps after the round trip; the current tests only verify
deserialization.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
