Skip to content

vortex-array constructor inconsistencies #4420

Closed

Description

@connortsui20

So I read through the other canonical arrays to check for new_unchecked methods, and they are wildly inconsistent:

  • bool: No new_unchecked even though there is a try_new
  • chunked: pub unsafe fn new_unchecked
  • datetime: Panicking new (and no try_new; maybe this is okay since it is just checking the time unit but this is still inconsistent)
  • decimal: No new_unchecked even though there is a try_new
  • list: No new_unchecked even though there is a try_new
  • primitive: No new_unchecked NOR a try_new, which is terrible because it only has a new that panics if the inputs are invalid
  • struct: pub(crate) fn new_unchecked (this isn't unsafe! A function being private does not make it safe!)
  • varbin: No new_unchecked even though there is a try_new
  • varbinview: pub unsafe fn new_unchecked

There are only a few that "make sense" to me:

  • constant: No new_unchecked (this makes sense since it is infallible and there is no try_new)
  • extension: No new_unchecked (this makes sense since it (seems to be) infallible and there is no try_new)
  • null: No new_unchecked (this makes sense since it is infallible and there is no try_new)

I'm going to address all of these in a followup PR because this is not great.

Originally posted by @connortsui20 in #4405 (comment)

Activity

  1. self-assigned this
    on Aug 29, 2025
  2. a10y commented on Aug 29, 2025

    @a10y
    Contributor

    Yep there are definitely some inconsistencies here! The Rust API has a few warts, we should definitely make these better :)

  3. connortsui20 commented on Sep 4, 2025

    @connortsui20
    MemberAuthor

    I'm also realizing that it would be good to update the encodings/ arrays to be consistent as well, but I'll leave that for a separate issue since most of them are good enough

  4. added a commit that references this issue on Sep 8, 2025
    4c7589c
  5. connortsui20 commented on Sep 8, 2025

    @connortsui20
    MemberAuthor

    Reopening this since a lot more nedds to be cleaned up (namely encodings/ and also the varbinview builder needs to be made more consistent with everything else as well).

  6. robert3005 commented on Apr 15, 2026

    @robert3005
    Contributor

    I think we have refactored things enough by now

  7. pinned this issue on Apr 15, 2026
  8. unpinned this issue on Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions