physical-plan: decoding an Unnest or Explain node with an unset field panics

Abierto Apto para principiantes
#26,999 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
2/5
Tiempo estimado
1-3 horas
Aptitud para principiantes
75/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
rust
Área
backend

Línea de trabajo

Empieza en datafusion/physical-plan/src/unnest.rs, en UnnestExec::try_from_proto, donde se hace unwrap de recursion.input_column y recursion.output_column. Luego revisa datafusion/physical-plan/src/explain.rs, en stringified_plan_from_proto y ExplainExec::try_from_proto, donde se hace unwrap de plan_type y plan_type_enum. Está terminado cuando ambas rutas devuelven un internal_err que nombra el campo faltante, con una prueba de decodificación para cada caso.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

Describe the bug

Two physical plan decoders in datafusion-physical-plan panic on a field the wire leaves unset. Their neighbours return an error in the same situation.

UnnestExec::try_from_proto unwraps both columns of each recursion option:

https://github.com/apache/datafusion/blob/934691e9af8f11bf4b629f21bb17795abfb84d93/datafusion/physical-plan/src/unnest.rs#L468-L469

input_column: recursion.input_column.as_ref().unwrap().into(),
output_column: recursion.output_column.as_ref().unwrap().into(),

RecursionUnnestOption.input_column and .output_column are optional messages, so a peer can leave either one unset. A few lines earlier, the same function refuses a missing options with internal_datafusion_err!:

https://github.com/apache/datafusion/blob/934691e9af8f11bf4b629f21bb17795abfb84d93/datafusion/physical-plan/src/unnest.rs#L443-L447

ExplainExec::try_from_proto maps each stringified plan through stringified_plan_from_proto. That function panics when plan_type, or its plan_type_enum, is unset:

https://github.com/apache/datafusion/blob/934691e9af8f11bf4b629f21bb17795abfb84d93/datafusion/physical-plan/src/explain.rs#L357-L365

ExplainExec::try_from_proto itself refuses a missing schema with an error:

https://github.com/apache/datafusion/blob/934691e9af8f11bf4b629f21bb17795abfb84d93/datafusion/physical-plan/src/explain.rs#L253-L257

The same code is in 55.1.0, at src/unnest.rs lines 473-474 and src/explain.rs lines 355-363.

To Reproduce

Decode a PhysicalPlanNode with datafusion_proto::bytes::physical_plan_from_bytes, where the node is one of:

  • an Explain node with a schema and one StringifiedPlan::default() in stringified_plans, so plan_type is None;
  • an Unnest node with a decodable input and schema, and options.recursions holding one RecursionUnnestOption::default(), so input_column is None.

Both reach the panic without any earlier check failing. I found these by reading the decode path and have not run these exact inputs here yet. I will add the panic output once a decode test on our side has run.

Expected behavior

Return internal_err! naming the missing field, as the sibling checks do. The caller then gets Err and keeps running.

Additional context

This matters wherever a process decodes plans it did not encode itself, such as a distributed worker that receives plan stages from peers. A malformed node panics the decoding task, and under panic = "abort" it stops the whole process. A related case in datafusion-proto is #26998.

The fix is two ok_or_else calls plus a decode test for each. I can open the PR if that is welcome.

Lenguaje dominante
Rust
Estrellas
9.6k
Forks
2.6k
Merge medio
4 d 11 h
PR fusionados (30 d)
322

Preparar el entorno

Abrir en Codespaces

Inicia el contenedor de desarrollo del proyecto en tu navegador, con tu propia cuenta de GitHub.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de apache/datafusion

Todos los issues de apache/datafusion

Issues similares

Más issues de Rust

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.