Skip to content

[SkipRecovery] Restore empty partitioned Parquet write tests [reduced-it] [databricks] - #15949

Open
wjxiz1992 wants to merge 1 commit into
NVIDIA:mainfrom
wjxiz1992:codex/recover-empty-parquet-6453
Open

[SkipRecovery] Restore empty partitioned Parquet write tests [reduced-it] [databricks]#15949
wjxiz1992 wants to merge 1 commit into
NVIDIA:mainfrom
wjxiz1992:codex/recover-empty-parquet-6453

Conversation

@wjxiz1992

Copy link
Copy Markdown
Collaborator

JaCoCo production line coverage: not fully measurable locally — no compatible nightly/runtime bytecode tuple (all five report groups N/A; details below).

Contributes to #6453.

Description

This test-only change restores two skipped checks for empty partitioned Parquet writes. Their original readback expected Spark to infer a schema from output with no data files, which also fails on CPU. The restored tests verify the shared CPU/GPU behavior: schema inference fails, while reading with the declared schema returns no rows. Production behavior is unchanged; these two cases showed no CPU/GPU divergence.

Both the single-writer and concurrent-writer cases compare CPU/GPU outputs and require GpuDataWritingCommandExec. Existing test names are retained. This recovers the two exclusions linked to #6453 without claiming to resolve that issue's broader file-format scope.

AI assistance: The change and PR description were prepared with Codex assistance.

Validation

  • Apache Spark 3.5.0, Scala 2.12, Python 3.10.18, RTX 5880 Ada: focused empty-output tests 53 passed, 335 deselected; full parquet_write_test.py: 387 passed, 1 skipped (385.75 seconds).
  • Exact-main shim-350 integration-test build: BUILD SUCCESS. The original two tests, with only their skips removed, reached the GPU writer and failed at schema inference. CPU probes on Spark 3.3.0 and 3.5.0 produced the same error for writer settings 0 and 10.
  • Three independent local source reviewers reported no actionable findings. Databricks was not run locally.
Commands, CI selection, and coverage provenance

Built from b50f7744723118d5bd63b419fc280a8e52d1a447:

mvn package -DskipTests -pl integration_tests -am -Dbuildver=350 \
  -Dmaven.repo.local=./.mvn-repo \
  -Drapids.test.gpu.allocFraction=0.3 \
  -Drapids.test.gpu.maxAllocFraction=0.3 \
  -Drapids.test.gpu.minAllocFraction=0 \
  -s jenkins/settings.xml -P mirror-apache-to-urm

# SPARK_HOME and both PYSPARK interpreters set to Spark 3.5.0 / Python 3.10.
# local[2], TEST_PARALLEL=0, TEST_TYPE=developer, DATAGEN_SEED=0, TZ=UTC;
# spark.rapids.memory.gpu alloc/maxAlloc/minAllocFraction = 0.3/0.3/0.
TESTS=parquet_write_test.py \
TEST='test_write_empty_data or test_write_empty_parquet_round_trip' \
  integration_tests/run_pyspark_from_build.sh --capture=tee-sys
TESTS=parquet_write_test.py TEST='' \
  integration_tests/run_pyspark_from_build.sh --capture=tee-sys

[reduced-it] retains both recovered cases: neither test has a parametrization decorator, and each collected one node. The full enclosing file was run without reduced selection. Scala unit tests retain the parallel default. [databricks] requests coverage for these shared tests.

JaCoCo execution data was collected for the current runtime. The refreshed nightly is Scala 2.12 / shim 350 / 26.10.0-SNAPSHOT b21 at 7ad4b98727db387b84a5030c34463733da45cf5f. No compatible complete baseline/runtime/classfile tuple was available: the older local runtime candidate has class-ID mismatches in sql-plugin and iceberg, while the current runtime contains newer production bytecode. All five report groups (sql-plugin, iceberg, shuffle-plugin, udf-compiler, and delta-lake) are N/A for marginal contribution; none is reported as measured zero. The shim-350 Delta group is a stub, not real Delta Lake coverage.

Checklists

Documentation

  • Updated for new or modified user-facing features or behaviors
  • No user-facing change

Testing

  • Added or modified tests to cover new code paths
  • Covered by existing tests
    (Please provide the names of the existing tests in the PR description.)
  • Not required

Performance

  • Tests ran and results are added in the PR description
  • Issue filed with a link in the PR description
  • Not required

Signed-off-by: Allen Xu <allxu@nvidia.com>
Copilot AI lite review requested due to automatic review settings September 10, 2026 09:53
@wjxiz1992 wjxiz1992 added the test Only impacts tests label Sep 10, 2026
@greptile-apps

greptile-apps Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge because the restored tests validate the intended empty-output contract and GPU execution without changing production behavior.

Summary

  • Consolidates the single-writer and concurrent-writer scenarios into a shared helper.
  • Verifies that schema inference fails when no data files exist.
  • Verifies that reading with the declared schema produces an empty result.
  • Requires the GPU write plan to contain GpuDataWritingCommandExec.

Reviews (1) · Last reviewed commit: "Restore empty partitioned Parquet write ..."

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new helper always sets spark.sql.maxConcurrentOutputFileWriters even when the value is 0, conflicting with established test semantics in the same file where 0 means “do not set the option,” which can change the single-writer case behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Restores previously skipped integration test coverage for empty, partitioned Parquet writes by asserting the shared Spark CPU/GPU behavior: schema inference fails when output contains no data files, while reading with an explicit schema succeeds and returns no rows.

Changes:

  • Replaces two skipif(True) tests with active assertions that validate the expected AnalysisException on schema inference and a successful explicit-schema readback.
  • Factors common logic into _assert_write_empty_partitioned_data(...) and applies it to both the concurrent-writer and single-writer test cases.
  • Adds @validate_execs_in_gpu_plan("GpuDataWritingCommandExec") to ensure the GPU writer path is exercised.
File summaries
File Description
integration_tests/src/main/python/parquet_write_test.py Restores the empty partitioned Parquet write tests and verifies CPU/GPU parity for schema inference vs explicit-schema readback.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +769 to +774
assert_gpu_and_cpu_writes_are_equal_collect(
lambda spark, path: spark.createDataFrame(spark.sparkContext.parallelize(data), schema)
.write.mode("overwrite").partitionBy('c1', 'c2').parquet(path),
read_empty_data,
data_path,
conf={"spark.sql.maxConcurrentOutputFileWriters": max_concurrent_writers})
@wjxiz1992

Copy link
Copy Markdown
Collaborator Author

build

@thirtiseven thirtiseven left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Only impacts tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants