Support a literal str argument in native lpad/rpad

Abierto
  • ¿Algo no está bien?
#7,525 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 2 días

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
76/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
scala, spark
Área
backend

Línea de trabajo

Start with spark/src/main/scala/org/apache/comet/serde/strings.scala and confirm PR #7450 provides the scalar-string kernel support this change depends on. Remove the literal str rejection in both serdes, update the lpad and rpad notes in docs/source/user-guide/latest/expressions.md, and adjust string_lpad.sql and string_rpad.sql expectations. Done when literal str cases with literal pad use native dispatch, column pad cases still use codegen dispatch, and tests account for Spark constant folding.

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

Descripción

area:expressions enhancement
What is the problem the feature request solves?

CometStringLPad and CometStringRPad (spark/src/main/scala/org/apache/comet/serde/strings.scala) return Unsupported when the str argument is a Literal, so queries such as lpad('hi', len, 'xy') run through codegen dispatch instead of the native kernel.

The check was added in https://github.com/apache/datafusion-comet/pull/2630 because the native read_side_padding kernel only accepted a string array, and a literal str failed at runtime. https://github.com/apache/datafusion-comet/pull/7450 adds a scalar-string arm to the kernel: it handles a scalar str with a scalar or an array length, a NULL str, and a NULL length, without expanding the scalar. So the reason for the check no longer holds.

Raised by @coderfender in the #7450 review.

Describe the potential solution
  • Drop the expr.str.isInstanceOf[Literal] check (and PadReasons.literalStrReason) from both serdes. The pad argument still has to be a literal.
  • Update the lpad/rpad notes in docs/source/user-guide/latest/expressions.md, which say literal strings use codegen dispatch.
  • In string_lpad.sql and string_rpad.sql, switch the queries with a literal str and a literal pad, such as lpad('hi', len, 'xy') and lpad('hi', len), from expect_dispatch to expect_native. Queries whose pad is a column stay on codegen dispatch.
  • An all-literal call is constant-folded by Spark, so the tests need a column length, or ConstantFolding excluded.
Additional context

Depends on https://github.com/apache/datafusion-comet/pull/7450.

Willingness to contribute

I would be willing to contribute a fix for this bug with guidance from the DataFusion community

Lenguaje dominante
Scala
Estrellas
1.3k
Forks
414
Merge medio
3 d 5 h
PR fusionados (30 d)
210

Preparar el entorno

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-comet

Todos los issues de apache/datafusion-comet

Issues similares

Más issues de Scala

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.