Skip to content
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

[0.18] Fix ClassCastException in document encoding #1159

Merged
merged 3 commits into from
Aug 21, 2023

Conversation

kubukoz
Copy link
Member

@kubukoz kubukoz commented Aug 18, 2023

Closes #1158.

As far as I understand, case x: ArraySeq[Document] isn't actually doing a class check because of type erasure.

The unsafe array inside turns out to actually be an Array[Object], so casting it to Array[Document] throws because arrays aren't erased (unlike ArraySeq which is a normal generic class).

I'm sure this solution is a bit inefficient, another one I had in mind would confirm the identity of the wrapped array:

a.value match {
  case x: ArraySeq[_]
      if x.unsafeArray.getClass == classOf[Array[Document]] =>
    val xs = x.unsafeArray.asInstanceOf[Array[Document]]
    var i = 0
    while (i < xs.length) {
      encodeValue(xs(i), out)
      i += 1
    }
  case xs =>
    xs.foreach(encodeValue(_, out))
}

but I'm not sure it's better as I didn't run any benchmarks. @plokhotnyuk any thoughts on this PR?

Note: the fix for the issue should be backported to 0.17.

EDIT: updated the implementation to be closer to the original, just checking for empty arrays instead. Perhaps it's worth a property-based test of roundtripping arbitrary documents just in case.

@kubukoz kubukoz added this to the 0.18.0 milestone Aug 18, 2023
@kubukoz kubukoz changed the title Fix ClassCastException in document encoding [0.18] Fix ClassCastException in document encoding Aug 18, 2023
@Baccata Baccata merged commit 5d9fdac into series/0.18 Aug 21, 2023
10 checks passed
@Baccata Baccata deleted the fix-cce-document-encode branch August 21, 2023 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scala 3: ClassCastException when encoding empty document arrays
2 participants