Skip to content

[fix](regression) Bundle jieba for Python UDF NLP test#65541

Draft
shuke987 wants to merge 1 commit into
apache:masterfrom
shuke987:codex/fix-pythonudf-nlp-jieba-master
Draft

[fix](regression) Bundle jieba for Python UDF NLP test#65541
shuke987 wants to merge 1 commit into
apache:masterfrom
shuke987:codex/fix-pythonudf-nlp-jieba-master

Conversation

@shuke987

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: N/A

Related PR: #60499, #63387

Problem Summary:

test_python_udf_nlp_chinese imports jieba, but py_udf_complex.zip only contained the five project Python modules. The UDF runs on BE Python runtimes, so installing jieba on the regression runner does not satisfy the dependency. Every recent master and branch-4.1 P0 occurrence therefore returned the case-authored "jieba package not installed" diagnostic at the first segmentation assertion.

This PR makes the test artifact self-contained:

  • bundles the CPython runtime subset of jieba==0.42.1 required for segmentation, TF-IDF, and POS tagging
  • includes jieba's MIT license
  • adds a deterministic builder that verifies the upstream source archive SHA-256 and normalizes zip metadata
  • preserves the five existing UDF modules byte-for-byte and does not change the Groovy case or golden output

The optional Paddle lac_small model and Jython-only .p models are excluded, reducing the generated artifact from approximately 18 MB for the full package to approximately 5 MB.

Release note

None

Check List (For Author)

  • Test

    • Regression test
      • test_python_udf_nlp_chinese passed twice with parallel=1, suiteParallel=1, and actionParallel=1 on the authorized build-matched debug environment using CPython 3.10.12.
      • Both runs exercised all ten existing NLP query tags with no golden changes.
    • Manual test
      • Rebuilt the zip twice from the checksum-pinned source archive and verified byte-identical output.
      • Verified unzip -t, required directory entries, extracted import with user site packages disabled, source-file identity, segmentation output, and builder syntax.
  • Behavior changed:

    • No.
  • Does this need documentation?

    • No.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

### What problem does this PR solve?

Issue Number: None

Related PR: apache#60499, apache#63387

Problem Summary: The Python UDF NLP case imports jieba, but its zip only contains the project modules. Bundle a checksum-pinned CPython subset of jieba and add a deterministic builder so every BE receives the required dictionaries and modules.

### Release note

None

### Check List (For Author)

- Test: Regression test
    - test_python_udf_nlp_chinese passed twice with parallelism set to 1
    - deterministic rebuild, extracted import, zip integrity, and golden spot checks passed
- Behavior changed: No
- Does this need documentation: No
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@shuke987

Copy link
Copy Markdown
Collaborator Author

/review

@shuke987

Copy link
Copy Markdown
Collaborator Author

run buildall

@github-actions

Copy link
Copy Markdown
Contributor

Possible file(s) that should be tracked in LFS detected: 🚨

The following file(s) exceeds the file size limit: 1048576 bytes, as set in the .yml configuration files:

  • regression-test/suites/pythonudf_complex_p0/py_udf_complex_scripts/py_udf_complex.zip

Consider using git-lfs to manage large files.

@github-actions github-actions Bot added the lfs-detected! Warning Label for use when LFS is detected in the commits of a Pull Request label Jul 13, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed for PR 65541 at head 333642f.

I found one issue that should be fixed before merging: the fix makes the shared complex Python UDF archive carry the large jieba payload, so all non-NLP complex UDF suites now pay that cost through Doris' per-function-id Python UDF cache/extract path.

Checkpoint conclusions:

  • Goal/test proof: bundling jieba makes the NLP UDF artifact self-contained, and the committed zip is byte-for-byte reproducible from the pinned source archive. Extracted-directory import checks match Doris' Python UDF runtime shape. The implementation is too broad because it changes the common archive used by unrelated suites.
  • Scope: builder and artifact changes are small and deterministic, but the dependency should be split into an NLP-specific archive instead of the shared one.
  • Concurrency/lifecycle: no product concurrency change, but BE cache lifecycle is relevant: Python UDF libraries are cached and unzipped per function id.
  • Compatibility/persistence/protocol: no FE/BE protocol, storage, transaction, or persistence format change found.
  • Performance: one runtime regression found and commented inline.
  • License/observability: jieba's MIT text is present next to the builder and inside the generated archive; no new observability need for this regression artifact.
  • Tests/validation: no builds or Doris regression tests were run in this review environment per instructions. Static review, artifact rebuild, zip inspection, and extracted Python import checks were performed.
  • User focus: no additional user-provided review focus was present.

Subagent convergence completed in two rounds. After M1 was accepted, both Round 2 subagents returned NO_NEW_VALUABLE_FINDINGS.

).read_bytes()

archive_prefix = PurePosixPath(f"jieba-{JIEBA_VERSION}") / "jieba"
with tarfile.open(jieba_archive, "r:gz") as archive:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes the existing shared complex-UDF archive much heavier for suites that do not use jieba. The old zip at the PR base is only 18 KB compressed / 73 KB uncompressed, while this one is 5.25 MB compressed / 18.6 MB uncompressed because of jieba/dict.txt, jieba/analyse/idf.txt, and the POS/finalseg tables. That would be fine if only the NLP suite loaded it, but test_python_udf_business_logic, test_python_udf_external_api, test_python_udaf_complex, and test_python_udtf_complex all still point at the same py_udf_complex.zip.

On BE this is paid per function id, not once per identical zip: scalar UDF, UDAF, and UDTF open paths pass _fn.id/_t_fn.id into UserFunctionCache::get_pypath, _get_cache_entry keys _entry_map by fid, _make_lib_file includes that id in the cached filename, and _load_cache_entry unzips each PY_ZIP. So the 45 non-NLP functions in these suites now copy/extract the 18.6 MB jieba payload even though none of them imports it, which is hundreds of MB of avoidable per-BE IO/disk churn and leaves much larger cache directories if a suite aborts before cleanup. Please split out an NLP-specific archive (for example nlp_chinese.py + jieba + its license) and have only test_python_udf_nlp_chinese.groovy use that, leaving the common archive small for the other complex UDF cases.

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

Labels

lfs-detected! Warning Label for use when LFS is detected in the commits of a Pull Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants