Repository navigation
Rebuild Python UDF with typed revisions and Arrow Flight execution - #29152
iamlinjunhong wants to merge 226 commits into
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
mo-bot review · Commit: Review completed. Read the assessment. |
XuPeng-SH
left a comment
There was a problem hiding this comment.
mo-bot review · mo-bot-local · review
Commit: f1eb492c02b2bef9778dd5775cdd01cc12a60148
Assessment: request_changes
The mandatory r13 design-approval gate remains open. Keeping catalog publication and expression evaluation with existing owners is coherent in direction, but implementation acceptance requires the outstanding independent decisions.
Model: gpt-6-astra / xhigh
Session: 01a13dcf-1517-7f90-b3f9-c5f9f9e7031b · slot 0 · generation 1
- P1
docs/design/python_udf_current_stage.md:8: Accepting the implementation while independent Architecture and SQL/Planner approval of python-udf-current-stage-r13-2026-10-15 remains explicitly pending.
Impact: The required design gate is unmet for shared catalog persistence, SQL compatibility, protocol and lifecycle changes. Python opt-in does not isolate the catalog migration. Record decisions identifying the current design artifact, reviewers, scope and outcome, or an authorized policy-owner exception.
Evidence: Line 8 and python_udf_current_stage_approval.md:232–233 explicitly retain pending status. Frozen comment 5809313638 approves historical r1; no supplied decision closes r13 approval. pkg/bootstrap/upgrade.go:79 independently registers 4.0.15, whose tenant handler replays shared-function migrations after protocol-114 admission. This is an unmet design requirement, not a reproduced runtime defect.
Validation:
-
Verified frozen HEAD, merge-base, clean source and complete 248-file inventory; confirmed upstream catalog 4.0.14 remains unchanged.
-
Reviewed governing design documents, supplied approval discussion and catalog activation owners. evidence.json.local_validation is empty.
-
Static audit passed: checks/1792047083242085559-2.json matches design-gate-audit.log. Command, source fingerprint and artifact hashes are recorded in receipt-reconciliation.json under the assigned diagnostic directory.
-
Frozen CI reports successful required, UT, native build, SCA, coverage and active multi-CN BVT checks; upgrade-compatibility checks were skipped. Execution logs were not independently reconciled.
Limitations:
-
The applicable mo-self-review skill states that missing design approval “short-circuits full implementation polishing.” Exhaustive implementation, lifecycle, unhappy-path, test-quality and performance review remains outstanding.
-
No local runtime tests or performance measurements were executed. Issue #28132's body was absent from the frozen evidence.
-
The accepted opt-in, unisolated development/test scope is not an additional blocker.
What type of PR is this?
Which issue(s) this PR fixes:
Related to #28132. This delivers the external unisolated adapter stage; it does not close the issue's broader sandbox/production-isolation requirements.
What this PR does / why we need it:
Replace the demo's per-value protobuf transport with typed, revision-bound Python UDF execution over Arrow Flight. CN remains MO-vector-native and owns guarded expression evaluation; user code runs in external handler processes.
Implementation and rollout contract: docs/design/python_udf.md. Namespace validation: docs/design/routine_namespace_validation.md. Local usage and cases: Python BVT README.
Current CI repair validation
The CI repair fixes the Arrow import ownership checks, preserves existing UDF SQL error codes/messages while adopting repository error wrapping, installs timezone data in both CI runtime image variants, and aligns the Python local launch FileServices with TN. Catalog result fixtures now assert the added revision table and six identity columns, including cross-account snapshot restore; Python error/result oracles remain unchanged. Real-worker test cleanup owns the process before readiness and records completion before repeated cleanup.
make err-check; CI actionlint; Python 3.12 coverage helper tests 6 passed.Existing host UT and coverage workflows use matrixorigin/CI#457 to install the checked-out worker requirements in a Python 3.12 venv and expose that interpreter to Go tests. CI #457 merged into
matrixorigin/CImain atc1a30c45dfce0352d1d4fb9573dda2a0feedb4d9; this changes existing jobs only. At the pre-rebase headc560dc168944c6ed158c8db36284173c38d255b1, MatrixOne ALL CI run 35810950106 had not reached a terminal result at the last recorded check. It does not validate the rebased head. Per user request, hosted CI was not awaited; this PR update will trigger a fresh run, and no hosted result is claimed.Latest-main rebase and current local validation
The branch was rebased onto
mainatca71f51e2f962c0fbe0a92d9d01bb071bb1bb9c9after nine new main commits made the previous PR head conflict. The merged MySQL grammar was regenerated with the repository goyacc target and its full parser package passed. Current-head validation also passed: Python UDF race, protocol race, full frontend, CN service and colexec packages;go vet; incrementalgolangci-lint(0 issues); andgit diff --check. The official CI coverage parser, using the latest-main PR diff plus CI UT/BVT profiles and current local package profiles, reports 2,727/3,626 = 75.21%, strictly above the 75% gate. Hosted CI was not awaited, as requested.Prior Linux integration evidence
Validated on Linux amd64 with the repository CI tester image, Go 1.26.4, Python 3.13.5 and PyArrow 24.0.0. Source included main merge
550ca2830dand repair37889f08b7; these results predate the CI repair above and are retained as earlier deployment/semantic evidence.-race.nometamode.GPT-6 medium design and integrated mo-self-review covered the branch and corrected exact-DROP, partial-initialization ownership and orphan-reaping findings. Hosted CI is pending; local/remote test passes are not reported as hosted CI results.
Compatibility and remaining scope
Python is disabled in generic launch and requires explicit unisolated opt-in. Legacy demo definitions/plans are rejected and must be explicitly recreated; existing SQL UDFs remain supported. Worker/Gateway capability and tzdata/environment digests must match before execution. Operator integrations must use an init-capable worker image and preserve its init command.
This does not provide sandbox tenant isolation or authenticated/TLS Flight, imported dependency environments, W>1/cumulative ACK, or full production Operator rollout acceptance. Native Linux dual-CN tests and image tests do not claim all deployment topologies were exercised. Prior optional strict-metadata SQL checking found six header differences; those expectations were not changed to hide them, and this PR reports the existing CI comparison mode explicitly.