fix: Fall back to the serialized body when UDF source misses globals - #6816
Open
Daksha1611 wants to merge 1 commit into
Open
fix: Fall back to the serialized body when UDF source misses globals#6816Daksha1611 wants to merge 1 commit into
Daksha1611 wants to merge 1 commit into
Conversation
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>
Author
|
/kind bug |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 itsdefining module cannot resolve those names — but the
execstill succeeds, because afunction body's free variables are only looked up when the function is called.
resolve_udftherefore treated rehydration as successful and never fell back to thedill body, even when that body carried the captured globals and would have run. The
failure surfaced later as a
NameErrorduring retrieval, so an on-demand feature viewthat served correctly before started raising once it was loaded from the registry:
This validates the rebuilt callable before accepting it: walk its
LOAD_GLOBALoperands, plus those of nested code objects (inner functions, comprehensions), and
return
Nonewhen a name resolves in neither the exec namespace nor builtins.resolve_udfthen falls back to the serialized body exactly as it did before.Only
LOAD_GLOBALoperands are inspected.co_namesalso contains attribute names(
df.columns), so checking that instead would flag working UDFs and push them onto thedill 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
git commit -s)Testing Strategy
Four regression tests added to
sdk/python/tests/unit/transformation/test_udf_rehydrate.py.Three of them fail on
masterand pass here; the fourth (test_attribute_access_is_not_ mistaken_for_a_missing_global) passes both ways by design, guarding theco_namesfalse-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
NameErrorbefore this change and thecorrect value after.
sdk/python/tests/unitfiltered 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
mastertoo.ruff check,ruff format --checkandmypyare clean on both changed files.Misc
The pre-existing
test_resolve_udf_prefers_source_over_garbage_dill_bodystill passes,which confirms self-contained UDFs continue to prefer source over the body.