Skip to content

BUG: length-aware string hashtable keys, fixing embedded-NUL collapse in unique/factorize/groupby - #66512

Draft
jbrockmendel wants to merge 1 commit into
pandas-dev:mainfrom
jbrockmendel:bug-nul-khash
Draft

BUG: length-aware string hashtable keys, fixing embedded-NUL collapse in unique/factorize/groupby#66512
jbrockmendel wants to merge 1 commit into
pandas-dev:mainfrom
jbrockmendel:bug-nul-khash

Conversation

@jbrockmendel

@jbrockmendel jbrockmendel commented Jul 28, 2026

Copy link
Copy Markdown
Member

pandas keys several khash tables on kh_cstr_t, a bare const char *, hashed with __ac_X31_hash_string (which stops at the first NUL) and compared with strcmp. Any value containing an embedded NUL is therefore truncated for both hashing and comparison, so distinct values sharing a prefix up to their first NUL collapse into one.

arr = np.array(["", "\x00"], dtype=object)
pd.unique(arr)        # before: ['']            after: ['', '\x00']
pd.factorize(arr)[0]  # before: [0, 0]          after: [0, 1]

df = pd.DataFrame({"c": pd.Series(np.array(["", "\x00", "", "\x00"], dtype=object))})
df.groupby("c").ngroups   # before: 1           after: 2

Keyed on an explicit {const char *ptr; size_t len} pair instead. The hash is X31 over exactly len bytes -- bit-identical to __ac_X31_hash_string for NUL-free input, so bucket distribution is unchanged -- and equality is a length check plus a byte comparison. PyUnicode_AsUTF8AndSize and _token_len supply the length, so no path pays an extra strlen.

Fixes unique, factorize, Series.unique, Index.unique and groupby on object dtype, plus read_csv's dtype=object interning and dtype="category" conversion (both of which silently boxed two distinct fields to one value).

Note on the issue's snippet

GH-34551 is titled for DataFrame.drop_duplicates, but that method routes through PyObjectHashTable and already returns the right answer -- the issue's own snippet passes on main. The confusion it reports is real, but it lives in unique/factorize/groupby. Worth knowing before comparing this diff against the issue title.

Performance

The byte-by-byte equality is deliberate and load-bearing: memcmp with a runtime length does not inline, and one libc call per token cost -9.2% on a bool column, where a hashset lookup hits on nearly every token. Full 123-case read_csv benchmark against this branch's merge-base: median 1.0081, 55 significant wins, 2 regressions (bool @12 0.974, mixed @12 0.978 -- 12-thread only; the same fixtures are +7.6% and +2.4% serial). Long keys were the obvious risk for a byte loop and did not materialise (long fields (~150B) +3.6%), so no length threshold is needed.

Scope

One of three PRs splitting the embedded-NUL work apart so the piece needing a semantics discussion doesn't block the rest:

Draft until the set can be reviewed together.

pandas keys several khash tables on a bare `const char *`, which hashes to
the first NUL and compares with strcmp, so any string containing an embedded
NUL is truncated for both hashing and comparison. Distinct values that share
a prefix up to their first NUL therefore collapse into one.

Key those tables on an explicit (pointer, length) pair instead. The hash is
X31 over exactly `len` bytes -- bit-identical to `__ac_X31_hash_string` for
NUL-free input, so bucket distribution is unchanged -- and equality is a
length check plus a byte comparison. `PyUnicode_AsUTF8AndSize` and
`_token_len` supply the length, so no path pays an extra strlen.

This fixes `unique`, `factorize`, `Series.unique`, `Index.unique` and
`groupby` on object dtype, plus `read_csv`'s `dtype=object` interning and
`dtype="category"` conversion.

The na_values/true_values/false_values hashsets change key type but still
derive their length with strlen, so NA handling is unchanged here; giving
them an explicit length changes user-visible NA semantics and is left to a
follow-up.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug hashing hash_pandas_object IO CSV read_csv, to_csv Strings String extension data type and string data

Projects

None yet

1 participant