Quarantine the greedy ForeignExecutionPlan claim so it cannot be copied by accident

オープン
#1,725 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

評価

難易度
3/5
見積もり時間
1〜2日
初心者へのやさしさ
72/100
issue の種類
リファクタリング
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
rust

調査の方向性

examples/datafusion-ffi-example/src/physical_extension_codec.rs:125-150 と、extension-guide/checklist.md の関連するガイダンスから始めます。MyPhysicalExtensionCodec が使用する statics と helpers を追跡し、指定されたモジュール・ドキュメントを付けて、src/foreign_plan_workaround.rs の claims()、park()、take() の背後に workaround を切り出します。grep が examples/ 内で “NOT A PATTERN” をちょうど 1 件見つけ、cargo clippy --no-deps --all-targets がクリーンに完了すれば完了です。

索引モデルが issue の本文から書いたものです。

説明

documentation rust

examples/datafusion-ffi-example/src/physical_extension_codec.rs:150 claims node.is::<DataSourceExec>() || node.is::<ForeignExecutionPlan>(). The second arm takes every other library's nodes, which extension-guide/checklist.md tells authors never to do.

This is not a bug to fix. It is G2 in #1719: the arm is load-bearing for upstream defect apache/datafusion#25152, and narrowing it to DataSourceExec alone makes 31 of the 51 tests in datafusion-ffi-query-planner-example fail. The object registry in the same file is downstream of it — once you claim a node you cannot introspect, there is nothing to write down about it, so parking it is all that is left.

The goal is to make it impossible to copy by accident while it stays. Proposal: move the statics and helpers into a new src/foreign_plan_workaround.rs behind three deliberately blunt functions — claims(), park(), take(). Nobody reads foreign_plan_workaround::park(node, buf) and thinks they are looking at serialization. The 25-line explanation currently buried inside a function body at lines 125-149 becomes the module //! doc, with three greppable fields:

//! # NOT A PATTERN
//! **Blocked on:** <https://github.com/apache/datafusion/issues/25152>
//! **Delete when:** `FFI_PlanProperties` carries `scheduling_type`, or
//!   `ForeignExecutionPlan` gains a reachable `try_to_proto`.
//! **Copying this will:** claim every other library's plan nodes, and produce
//!   payloads that decode only in the writing process, exactly once each.

One honesty note belongs in that doc: the DataSourceExec arm could be durable and is not, because the registry has to exist for the ForeignExecutionPlan arm regardless. Splitting the two arms across two wire formats costs real code and removes nothing. Saying so lets the guide claim the registry cannot be removed without also claiming every byte of it is forced.

Do not rename MyPhysicalExtensionCodec — 21 call sites plus two doc references, and the module doc already carries the label.

Done when: grep -rn "NOT A PATTERN" examples/ returns exactly one site, and cargo clippy --no-deps --all-targets is clean. No test changes expected; this is code motion plus documentation.

主要言語
Python
スター
605
フォーク
176
平均マージ
1日 23時間
マージ済み PR(30日)
8

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

apache/datafusion-python のほかの issue

apache/datafusion-python の issue をすべて見る

似ている issue

Python の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。