test: cover lpad and rpad routing configurations

[rich7420]

Which issue does this PR close?

Part of #4616.

Rationale for this change

The padding SQL tests compare results without distinguishing native execution from JVM codegen dispatch. Separate Scala tests duplicate the routing coverage.

What changes are included in this PR?

Add expression-specific routing assertions to the four existing lpad/rpad SQL fixtures and remove the duplicate Scala routing tests. Preserve the original NULL, empty-padding and literal/column cases with the dispatcher enabled and disabled.

How are these changes tested?

All four SQL fixtures pass locally on Spark 3.5.9 and 4.1.3. The five retained padding tests also pass on Spark 4.1.3. A native-to-dispatch regression passes the old SQL fixtures but fails the new native assertions for both expressions.

[andygrove]

Thanks @rich7420. Maybe we could just delete these tests and re-implement in the Comet SQL suite?

[sunchao]

Correctness

Summary

Reviewed 81fa972a498695fbe05015f5eabc3222dd576845 against f69c4c81b9429e327ea95658530ae4ed4ed19635. I found no actionable correctness issue. This changes one test file and leaves production routing and padding semantics unchanged.

The existing checks exercised unsupported shapes only with the dispatcher enabled and relied on global activity counters. The revised tests cover both dispatcher settings and name lpad or rpad explicitly. The shared assertion requires the expression to appear in its expected implementation set and be absent from the other. With dispatch disabled, the tests compare Spark results and require the expression-specific disabled-dispatcher reason.

The cases match the current serde contract: a column string with a literal pad stays native, while literal-string and nonliteral-pad shapes use dispatch or Spark fallback. Removing the allowIncompatible loop is consistent with these shapes being Unsupported, whose routing does not consult that setting. Disabling constant folding keeps the all-literal case meaningful.

I compared the tests with the maintained Spark 3.5 and 4.0 implementations and padding tests. They preserve Spark's null-intolerant behavior, default space padding, truncation, and literal/column argument handling. Existing tests retain negative/zero lengths, Unicode, null lengths, and binary coverage.

Validation

Current CI is green. I verified that the Spark 4.1 and Spark 4.0 expression jobs each passed all four changed routing tests. Their checkout was 2c4768268cb92c2286c3d51c3afe3413d8166f6e, whose parents are the reviewed base and head and whose tree equals the head. The four tests cover 20 query/configuration combinations.

The Spark 4.1 shard reports 1,386 passed with no failures or cancellations. Spark 4.0 reports 1,383 passed, no failures, and three cancellations outside these four tests. I did not run a local Spark build or independently repeat the author's injected-regression probes. Maintained Spark 3.4/4.1 source was unavailable, so CI execution is distinct from the canonical source comparison.

Performance

There is no production execution cost in this test-only change. Replacing global counter checks with per-plan expression assertions makes the routing checks independent of unrelated dispatcher activity. The matrix grows from 16 to 20 query/configuration combinations across tiny fixtures. No performance claim or microbenchmark is needed for this scope.

Design

Reusing checkSparkAnswerAndImpl and the existing fallback helper keeps result comparison and routing checks together. The existing SQL-suite suggestion is feasible: the SQL runner already supports expect_native, expect_dispatch, and expect_fallback through the same helpers. Moving the fixtures there can preserve these assertions. I have no additional design finding beyond that discussion.

Abstraction & complexity

The two small loops share coverage between left and right padding and between dispatcher settings. They add no helper abstraction or production indirection. Removing the expression-class-name mapping and global stats handling makes each test's expected route easier to read.

[rich7420]

Thanks @rich7420. Maybe we could just delete these tests and re-implement in the Comet SQL suite?

sure, I'll try to do that

[sunchao]

Re-reviewed c1615090. The SQL migration addresses the earlier suggestion: all 20 query/configuration combinations from the removed Scala routing tests are preserved, including native execution with dispatch disabled. The fixtures use the same expression-specific implementation and fallback assertions, keep the previous input cases, and disable constant folding. I found no new or remaining P1/P2 findings in this update.

I verified all four SQL fixtures and the five retained padding tests passing in the current CI logs for Spark 3.5, 4.0, and 4.1. CI executed merge 44ac055f, which includes newer base changes. All five reviewed files match that merge, and the relevant assertion helpers are equivalent. This is CI execution evidence. I did not run Spark/JNI locally or independently repeat the author’s mutation probe.

[andygrove]

Thanks. I checked that CometStringLPad/RPad.getSupportLevel only ever returns Compatible or Unsupported, so the allowIncompatible loop in the deleted Scala test was not exercising anything and the fixtures are a strict upgrade.