Skip to content

fix: Fall back to the serialized body when UDF source misses globals - #6816

Open
Daksha1611 wants to merge 1 commit into
feast-dev:masterfrom
Daksha1611:fix/odfv-udf-globals-rehydrate
Open

fix: Fall back to the serialized body when UDF source misses globals#6816
Daksha1611 wants to merge 1 commit into
feast-dev:masterfrom
Daksha1611:fix/odfv-udf-globals-rehydrate

Conversation

@Daksha1611

Copy link
Copy Markdown

What this PR does / why we need it:

Source-first UDF rehydration execs only the UDF's own source into a namespace seeded
with pandas/numpy. A UDF that reads a helper, constant, or aliased import from its
defining module cannot resolve those names — but the exec still succeeds, because a
function body's free variables are only looked up when the function is called.

resolve_udf therefore treated rehydration as successful and never fell back to the
dill body, even when that body carried the captured globals and would have run. The
failure surfaced later as a NameError during retrieval, so an on-demand feature view
that served correctly before started raising once it was loaded from the registry:

NameError: name 'scaled' is not defined

This validates the rebuilt callable before accepting it: walk its LOAD_GLOBAL
operands, plus those of nested code objects (inner functions, comprehensions), and
return None when a name resolves in neither the exec namespace nor builtins.
resolve_udf then falls back to the serialized body exactly as it did before.

Only LOAD_GLOBAL operands are inspected. co_names also contains attribute names
(df.columns), so checking that instead would flag working UDFs and push them onto the
dill path for no reason — there's a regression test covering that.

Self-contained UDFs are unaffected and still take the source path, so the Spark
segfault and cross-Python-version benefits of source-first rehydration are kept intact.

Which issue(s) this PR fixes:

Fixes #6815

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
  • Manual tests
  • Testing is not required for this change

Four regression tests added to sdk/python/tests/unit/transformation/test_udf_rehydrate.py.
Three of them fail on master and pass here; the fourth (test_attribute_access_is_not_ mistaken_for_a_missing_global) passes both ways by design, guarding the co_names
false-positive trap described above.

Verified end to end as well: an ODFV applied from a script, then read back in a separate
process holding only the registry, returned NameError before this change and the
correct value after.

sdk/python/tests/unit filtered to the transformation / on-demand / feature-view areas:
314 passed, 5 skipped. The 9 errors in that run are MongoDB testcontainers failing to
start locally and are present on master too.

ruff check, ruff format --check and mypy are clean on both changed files.

Misc

The pre-existing test_resolve_udf_prefers_source_over_garbage_dill_body still passes,
which confirms self-contained UDFs continue to prefer source over the body.

Source-first rehydration execs only the UDF's own source into a namespace
seeded with pandas/numpy. A UDF that reads a helper, constant, or aliased
import from its defining module has no way to resolve those names, but the
exec still succeeds, because a function body's free variables are only
looked up when it is called.

resolve_udf therefore treated rehydration as successful and never fell back
to the dill body, even when that body carried the captured globals and would
have run. The failure surfaced later as a NameError during retrieval, so an
on-demand feature view that served correctly before started raising once it
was loaded from the registry.

Validate the rebuilt callable before accepting it: walk its LOAD_GLOBAL
operands, and the operands of nested code objects, and return None when a
name resolves in neither the exec namespace nor builtins. resolve_udf then
falls back as it did before.

Only LOAD_GLOBAL is inspected. co_names also holds attribute names, so
checking it would flag working UDFs and push them onto the dill path for no
reason.

Self-contained UDFs are unaffected and still take the source path, keeping
the Spark and cross-Python-version benefits intact.

Signed-off-by: Daksha1611 <mehtadaksha1611@gmail.com>
@Daksha1611
Daksha1611 requested a review from a team as a code owner September 6, 2026 17:27
@Daksha1611

Copy link
Copy Markdown
Author

/kind bug

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.

On-demand feature view UDFs lose their module globals when rebuilt from the registry, so serving fails with NameError

1 participant