Skip to content

Add support for Guava ImmutableIntArray / ImmutableDoubleArray (2.22)#157

Merged
cowtowncoder merged 2 commits intoFasterXML:2.xfrom
kilink:immutable-int-double-array-support
Jan 23, 2026
Merged

Add support for Guava ImmutableIntArray / ImmutableDoubleArray (2.22)#157
cowtowncoder merged 2 commits intoFasterXML:2.xfrom
kilink:immutable-int-double-array-support

Conversation

@kilink
Copy link
Copy Markdown
Contributor

@kilink kilink commented Aug 22, 2024

Add serde support for ImmutableIntArray and ImmutableDoubleArray.

Note that for simplicity, the deserializers currently delegate to the implementations in PrimitiveArrayDeserializers, passing the deserialize primitive arrays to the respective copyOf factory methods. The benefit is that all of the various edge cases of reading ints / doubles is handled already in those implementations. The downside is that we make an extra copy of the array when we pass it to the copyOf method, but at least there is no waste in the array sizing in this case.

@yawkat
Copy link
Copy Markdown
Member

yawkat commented Aug 27, 2024

LGTM, but have you already signed the CLA?

@cowtowncoder cowtowncoder changed the title Add support for ImmutableIntArray / ImmutableDoubleArray Add support for Guava ImmutableIntArray / ImmutableDoubleArray Aug 27, 2024
@cowtowncoder
Copy link
Copy Markdown
Member

Ah yes -- happy to help get this reviewed, but before merging will need CLA (if not already sent earlier; only needs to be done once before the first contribution). It's this:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

and the usual way is to print, fill & sign, scan/photo, email to cla at fasterxml dot com.
ONce this is done I can proceed with merging (assuming review goes fine etc).

@kilink
Copy link
Copy Markdown
Contributor Author

kilink commented Aug 27, 2024

Sure, I can get the CLA signed; but also I realized that ImmutableIntArray / ImmutableDoubleArray were added in Guava 22.0, so this change cannot really be merged anyway if the minimum supported Guava version is still 20. For some reason I thought these classes had been around much longer.

@cowtowncoder
Copy link
Copy Markdown
Member

cowtowncoder commented Aug 27, 2024

Ok, one immediate problem tho: looks like our CI is still assuming min Guava level of 20, and ImmutableIntArray was added in Guava 22 (as per Javadocs).
Note that this is different from default dependency indicated, Guava 25.

So: do we want to bump minimum supported Guava to 22 for Jackson 2.18?
I don't have strong opinion here, but will need to file separate issue to increase that, if agreed.

EDIT: filed #158

@cowtowncoder
Copy link
Copy Markdown
Member

@kilink One possibility: postpone Guava baseline upgrade to Jackson 2.19, merge this in after 2.18.0 released? This gives us more time.

@kilink
Copy link
Copy Markdown
Contributor Author

kilink commented Aug 28, 2024

@kilink One possibility: postpone Guava baseline upgrade to Jackson 2.19, merge this in after 2.18.0 released? This gives us more time.

I'm fine with that. Not really an urgent need for this, I just saw it as a gap. I can continue just using the custom serializers / deserializers directly for now.

@cowtowncoder cowtowncoder added the cla-needed PR requires CLA before merging (not yet provided by author) label Nov 5, 2024
@cowtowncoder
Copy link
Copy Markdown
Member

Would like to proceed with this, wrt merging into 2.19, but need 2 things:

  1. PR needs to re-based (or re-created against 2.19)
  2. CLA

with that, I'd be happy to merge this!

@cowtowncoder cowtowncoder added 2.20 and removed 2.19 labels May 23, 2025
@cowtowncoder
Copy link
Copy Markdown
Member

Now that #158 complete, baseline not a blocker. But looks like this pr is not mergeable as-is...

@kilink
Copy link
Copy Markdown
Contributor Author

kilink commented Jan 22, 2026

@cowtowncoder sorry for the delay, I've found a printer finally and signed the CLA. Is this change still needed? If so, I can rebase it.

I noticed that 3.x already has support for these types, however the implementations look like they allocate and box unnecessarily, and may have a bug (doesn't write anything for empty arrays). Anyway I am fine with rebasing this PR if you'd still want it in 2.x, or I can just close it.

@cowtowncoder
Copy link
Copy Markdown
Member

@kilink I'd be happy to get 2.x (for 2.22) version. And maybe could replace 3.x implementation with improvements you mention?

@kilink kilink force-pushed the immutable-int-double-array-support branch from bcd0897 to 5aaabd2 Compare January 23, 2026 16:21
@kilink kilink changed the base branch from 2.18 to 2.x January 23, 2026 16:22
@kilink kilink force-pushed the immutable-int-double-array-support branch from 5aaabd2 to 3d937a0 Compare January 23, 2026 16:26
Add serde support for ImmutableIntArray and ImmutableDoubleArray.
@kilink kilink force-pushed the immutable-int-double-array-support branch from 3d937a0 to 299f969 Compare January 23, 2026 17:30
@kilink
Copy link
Copy Markdown
Contributor Author

kilink commented Jan 23, 2026

@cowtowncoder okay, I've rebased onto 2.x and updated the PR to point to that. I also added support for WRITE_SINGLE_ELEM_ARRAYS_UNWRAPPED and added tests for that as well as UNWRAP_SINGLE_VALUE_ARRAYS.

Copy link
Copy Markdown
Member

@cowtowncoder cowtowncoder left a comment

Choose a reason for hiding this comment

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

Looks good; couple of small suggestions (I might apply changes too if need be).

Copy link
Copy Markdown
Member

@cowtowncoder cowtowncoder left a comment

Choose a reason for hiding this comment

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

LGTM!

@cowtowncoder cowtowncoder added cla-received Marker to denote that required CLA received and removed cla-needed PR requires CLA before merging (not yet provided by author) labels Jan 23, 2026
@cowtowncoder
Copy link
Copy Markdown
Member

cowtowncoder commented Jan 23, 2026

@kilink Will merge this --as a follow-up, it would be great to get get long variants too, if you have time & interest (if not that's fine, this is already good!).

EDIT: realized how straight-forward this is so went ahead and did #213 . So no need for extra work for long[].

@cowtowncoder cowtowncoder merged commit dad5566 into FasterXML:2.x Jan 23, 2026
9 checks passed
@cowtowncoder cowtowncoder added 2.22 and removed 2.20 labels Jan 23, 2026
@cowtowncoder cowtowncoder added this to the 2.22.0 milestone Jan 23, 2026
@cowtowncoder cowtowncoder changed the title Add support for Guava ImmutableIntArray / ImmutableDoubleArray Add support for Guava ImmutableIntArray / ImmutableDoubleArray (2.22) Jan 23, 2026
cowtowncoder added a commit that referenced this pull request Jan 23, 2026
@cowtowncoder
Copy link
Copy Markdown
Member

Ok, I managed to merge this into 3.x as well, although it's bit of hybrid for now.

I think I'll try to follow-up here for long/Long to unify things bit more.

cowtowncoder added a commit that referenced this pull request Jan 23, 2026
cowtowncoder added a commit that referenced this pull request Jan 23, 2026
@kilink
Copy link
Copy Markdown
Contributor Author

kilink commented Jan 24, 2026

@kilink Will merge this --as a follow-up, it would be great to get get long variants too, if you have time & interest (if not that's fine, this is already good!).

EDIT: realized how straight-forward this is so went ahead and did #213 . So no need for extra work for long[].

Thanks! Not sure how I missed the long variant...

@kilink kilink deleted the immutable-int-double-array-support branch January 24, 2026 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.22 cla-received Marker to denote that required CLA received guava

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants