Visitar URL original
fix: Map narrow and unsigned Arrow integer types to Feast value types by raashish1601 · Pull Request #6960 · feast-dev/feast · GitHub
Skip to content

fix: Map narrow and unsigned Arrow integer types to Feast value types - #6960

Open
raashish1601 wants to merge 2 commits into
feast-dev:masterfrom
raashish1601:fix/pa-small-int-types
Open

fix: Map narrow and unsigned Arrow integer types to Feast value types#6960
raashish1601 wants to merge 2 commits into
feast-dev:masterfrom
raashish1601:fix/pa-small-int-types

Conversation

@raashish1601

@raashish1601 raashish1601 commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

pa_to_feast_value_type only knows int32 and int64, so a Parquet file with a narrower or unsigned integer column breaks schema inference for a FileSource (and the other sources that use this decoder):

pd.DataFrame({"id": [1, 2], "rating": np.array([3, 4], dtype=np.int16), "ts": ...}).to_parquet(path)
src = FileSource(path=path, timestamp_field="ts")
src.source_datatype_to_feast_value_type()("int16")
# KeyError: 'int16'

python_type_to_feast_value_type already maps these numpy dtypes (int8, int16, uint8, uint16 to INT32), so the same data works from pandas but not from its Arrow schema.

This adds the Arrow names to the map: int8, int16, uint8 and uint16 decode to INT32, and uint32 and uint64 to INT64 (for uint32 I picked INT64 so every value fits). Lists of them decode to the matching _LIST type through the existing list handling.

Which issue(s) this PR fixes:

No issue, found while checking Arrow type decoding.

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests
  • Integration tests

Added test_pa_to_feast_value_type_small_and_unsigned_ints in sdk/python/tests/unit/test_type_map.py (8 cases, all fail on master). pytest tests/unit/test_type_map.py passes apart from the TestSparkNativeTypeValidation cases, which fail the same way on master here because pyspark isn't installed. ruff check and ruff format --check are clean on the changed files.

@raashish1601
raashish1601 requested a review from a team as a code owner October 7, 2026 07:38
@codecov-commenter

codecov-commenter commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.32%. Comparing base (9d42729) to head (e6c9c93).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6960   +/-   ##
=======================================
  Coverage   49.32%   49.32%           
=======================================
  Files         443      443           
  Lines       55094    55094           
  Branches     8017     8017           
=======================================
  Hits        27176    27176           
  Misses      26035    26035           
  Partials     1883     1883
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 50.71% <ø> (ø)
Files with missing lines Coverage Δ
sdk/python/feast/type_map.py 64.35% <ø> (ø)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d42729...e6c9c93. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ntkathole
ntkathole force-pushed the fix/pa-small-int-types branch from f437d04 to 5d31929 Compare October 7, 2026 14:37
@ntkathole

ntkathole commented Oct 7, 2026

Copy link
Copy Markdown
Member

One thing worth resolving before merge: uint32 now silently diverges from the pandas path.

python_type_to_feast_value_type maps uint32 -> INT32:

https://github.com/feast-dev/feast/blob/master/sdk/python/feast/type_map.py#L397

but this PR maps uint32 -> INT64. So the same uint32 column infers a different Feast value type depending on whether schema inference goes through the pandas dtype or the Arrow schema — which cuts against the parity motivation in the PR description ("the same data works from pandas but not from its Arrow schema").

To be clear, I think the INT64 choice here is the more correct one (uint32 max 4,294,967,295 doesn't fit in INT32), so the cleaner fix is probably to also update python_type_to_feast_value_type's uint32 -> INT32 to INT64 so both paths agree. If you'd rather keep them different on purpose, could you add a short code comment next to the Arrow mapping noting the intentional divergence? Otherwise a future reader comparing the two maps will see them disagree with no explanation.

@raashish1601

raashish1601 commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Agreed, thanks. I changed python_type_to_feast_value_type to map uint32 to INT64 as well (ec11d88), and added a test that checks the pandas dtype and Arrow paths infer the same type for each narrow and unsigned int.

@ntkathole

ntkathole commented Oct 7, 2026

Copy link
Copy Markdown
Member

Please resolve conflicts

@raashish1601
fix: Map narrow and unsigned Arrow integer types to Feast value types
16e72f2 
Signed-off-by: Raashish Aggarwal <94279692+raashish1601@users.noreply.github.com>
@raashish1601
fix: Map uint32 to INT64 in pandas dtype inference too
e6c9c93 
Signed-off-by: Raashish Aggarwal <94279692+raashish1601@users.noreply.github.com>
@raashish1601

raashish1601 commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on master and resolved the conflict in test_type_map.py (kept both the new Spark test and this one).

@raashish1601
raashish1601 force-pushed the fix/pa-small-int-types branch from ec11d88 to e6c9c93 Compare October 7, 2026 15:54

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@raashish1601 @codecov-commenter @ntkathole