Add deserialization support for Table<R, C, V> #163
Conversation
Abhishekkr3003
force-pushed
the
table-deserializer
branch
from
63792ad to
d8442de
Compare
|
Excellent, thank you @Abhishekkr3003 ! I will need to review this in detail, but looks good so far. One thing we will need (if not yet done -- only needed once) is CLA, from: and it's usually easiest to print, fill & sign, scan/photo, email to Once I have CLA & reviewed code can merge it. Thank you again & looking forward to merging this! |
|
@cowtowncoder Thank you for sharing CLA. I have just signed and shared it. |
| @@ -258,9 +261,6 @@ public JsonDeserializer<?> findMapLikeDeserializer(MapLikeType type, | |||
| elementTypeDeserializer, elementDeserializer); | |||
| } | |||
|
|
|||
| if (Table.class.isAssignableFrom(raw)) { | |||
There was a problem hiding this comment.
@cowtowncoder, should we use MapLikeType/MapLikeDeserializer or not? What I found is if we keep it MapLikeType, then we have to use Type something like Map<R, Map<C, V>>, and then we'll have KeyDeserializer for RowKey and JsonDeserializer for ColumnValueMap, and then while deserializing the Table<R, C, V> we'll have to deserialize Map<C, V> first and then have to put each C, V in Table<R, C, V> for R, which is inefficient in my opinion. Contrarily, if we don't use MapLikeType and keep it JavaType (Using Bean Deserializer), we can deserialize directly to Table<R, C, V>.
What do you think about this?
There was a problem hiding this comment.
Hmmmh. Tough question. I think that:
- Type should be retained as
MapLikeType(to help Jackson databind be somewhat aware of type's semantic) - But! We do NOT need to use
MapLikeDeserializer, if that is possible
If (2) can't be done for some reason (I may be overlooking something), I'm ok in change to make Tables not be considered "Map-like". This will add some work on (de)serialization handling, possibly.
There was a problem hiding this comment.
Sure, I have used MapLikeType without using any other MapLikeType deserialization arguments, as if we use elementDeserializer, then we might have to first deserialize to Map<C, V> and then have to put in Table<R,C,V>, since we cannot get C's deserializer and V's deserializer separately from JsonDeserializer<?> elementDeserializer of this API:
public JsonDeserializer<?> findMapLikeDeserializer(MapLikeType type, DeserializationConfig config, BeanDescription beanDesc, KeyDeserializer keyDeserializer, TypeDeserializer elementTypeDeserializer, JsonDeserializer<?> elementDeserializer)
Well, this is from my limited knowledge of this codebase; please do correct me if I'm missing something here.
There was a problem hiding this comment.
That sounds good -- hoping to review this in near future, just having bit of overload with everything going on. But thank you again for this contribution!
There was a problem hiding this comment.
Thanks for the update! I completely understand, and no worries at all. Take your time. I’m happy to contribute and look forward to any feedback whenever you’re able to review. Thanks again!
cowtowncoder
changed the title
|
Ok, merging to Thank you very much @Abhishekkr3003 -- this is a great addition for Jackson 2.19! |
|
Thank you @cowtowncoder. It was a pleasure working on this! |
|
@cowtowncoder when is this planned to be released? |
|
@anuragagarwal561994 "when 2.19.0 is ready". No specific date in mind, probably not before early March tho -- focusing most of work for completing first release candidate for 3.0.0 (to be done by end of February). |
|
Hi @cowtowncoder any updates on the release of 2.19? |
|
@anuragagarwal561994 No. Probably sometime in April. |
Closes #1