lpad/rpad do not use withNullShortCircuit, so ANSI errors are raised on rows Spark skips

オープン 初心者向け
  • 何か問題がありますか？
#7,583 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 2 日以内に返信

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

評価

難易度
2/5
見積もり時間
1〜3時間
初心者へのやさしさ
72/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
scala, spark
領域
data-engineering

調査の方向性

lpad/rpad のシリアライザは spark/src/main/scala/org/apache/comet/serde/strings.scala（568〜620行目付近）の CometStringLPad と CometStringRPad です。QueryPlanSerde.scala の withNullShortCircuit と、arrays.scala でそれがどう使われているかを読み、同じ方法で両方のプロトを包んでください。再現コードの NULL 入力に ANSI キャストを適用したとき、lpad と rpad が NULL を返し、そのケースを対象とした SQL ファイルテストがあれば完了です。

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

説明

requires-triage
Describe the bug

CometStringLPad and CometStringRPad build their native expression with scalarFunctionExprToProto and do not wrap it in withNullShortCircuit:

In Spark, StringLPad / StringRPad are TernaryExpressions with nullIntolerant = true. TernaryExpression.eval evaluates the len and pad children only when str is non-null. Comet evaluates all children for every row, so when a child can fail (for example an ANSI cast), Comet can raise an error on a row where str is NULL and Spark would have returned NULL without evaluating that child.

This came up in review of https://github.com/apache/datafusion-comet/pull/7450: https://github.com/apache/datafusion-comet/pull/7450#discussion_r4511335598

Steps to reproduce

Found by reading the code; I have not run this repro yet.

SET spark.sql.ansi.enabled=true;
CREATE TABLE tbl(s STRING, t STRING) USING parquet;
INSERT INTO tbl VALUES (NULL, 'abc');
SELECT lpad(s, CAST(t AS INT)) FROM tbl;
SELECT rpad(s, CAST(t AS INT)) FROM tbl;
Expected behavior

Both queries return NULL, as Spark does, because s is NULL and Spark never evaluates CAST(t AS INT) for that row. Comet is expected to raise a cast error instead.

Additional context

Other serdes already use withNullShortCircuit for this (for example in arrays.scala, maps.scala and datetime.scala). The fix is probably to wrap the lpad/rpad proto in withNullShortCircuit and add a SQL file test for the case above.

Willingness to contribute

I can contribute a fix for this bug independently

主要言語
Scala
スター
1.3k
フォーク
414
平均マージ
3日 5時間
マージ済み PR（30日）
210

環境構築

はじめの一歩

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

apache/datafusion-comet のほかの issue

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

似ている issue

Scala の issue をもっと見る

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

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