Persist ORC calendar metadata and support legacy DATE rebasing in C++ - #24733

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

No actionable comments were generated in the recent review.

Merge Risk: Minimal · up to 82fec

The change adds opt-in ORC calendar handling, with proleptic Gregorian as the default. The supplied context shows no actionable merge-blocking risk.

Pre-merge checks: 4

Check name	Status	Explanation
Linked Issues check		[#23794] Both writer APIs expose a typed calendar option and write footer field 11. Java and JNI pass the option to the writer. Legacy DATE values are rebased before encoding and statistics; the reade…
Out of Scope Changes check		[#23794] Calendar conversion, metadata, statistics, API changes, and tests support the issue. The shared kernel error change and Parquet include updates support ORC decode error handling. The Parquet …
Title check		The title describes the ORC calendar metadata and DATE rebasing changes. The changes also include Java/JNI support.
Description check		The pull request description directly matches the changeset. It describes ORC calendar metadata, writer and reader options, DATE rebasing, statistics behavior, Java/JNI controls, rejected cases, and t…

Full details: Linked Issues check

Explanation

[#23794] Both writer APIs expose a typed calendar option and write footer field 11. Java and JNI pass the option to the writer. Legacy DATE values are rebased before encoding and statistics; the reader exposes the footer calendar and provides opt-in conversion. The C++ and Java tests cover calendar modes, cutover dates, nested DATE values, statistics, and Apache ORC interoperability. The incremental test diff was unavailable; the whole-PR summary and reviewed-head evidence support this assessment. Tests were not run.

Full details: Out of Scope Changes check

Explanation

[#23794] Calendar conversion, metadata, statistics, API changes, and tests support the issue. The shared kernel error change and Parquet include updates support ORC decode error handling. The Parquet test cleanup is incidental and does not change behavior. No unrelated change is demonstrated.

Full details: Description check

Explanation

The pull request description directly matches the changeset. It describes ORC calendar metadata, writer and reader options, DATE rebasing, statistics behavior, Java/JNI controls, rejected cases, and test coverage.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: NVIDIA/cudf
• Branch: main
• File: .coderabbit.yaml
• Commit: d704d80
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 80870abd-8524-445b-b051-6c720d2c91ee

Commits

Reviewing files that changed from the base of the PR and between f33c38b and 82fec42.

Files selected for processing: 5

• cpp/include/cudf/io/orc.hpp
• cpp/src/io/orc/reader_impl_decode.cu
• cpp/src/io/orc/writer_impl.cu
• cpp/src/io/parquet/hybrid_scan/hybrid_scan_preprocess.cu
• cpp/tests/io/orc_test.cpp

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Historical benchmark results from before this review update. These compare 70eeb18 with db4aedb; they have not been rerun for the new review changes.

Performance

The recorded comparison isolates the earlier review fixes: 70eeb18bb7470868d8e57db9537ff231c2a564a0 versus db4aedbfc29abcde09f87c60d99f9bd41f8390fe, on headless u13 / A100-PCIE-40GB, driver 535.183.01, GCC 14.2.1 / CUDA 12.9.1. Both revisions already contain legacy support and are run with their matching public headers and shared libraries.

The DATE-only microbenchmark measures 1K/1M/10M rows, NONE/SNAPPY, one-shot/4-write-chunk files, both calendars, and write/raw-read/conversion-enabled-read operations. It uses a deterministic non-null DATE column spanning 1001–2024; legacy cutover-gap inputs are mapped to 1582-10-15 outside timing. A 1 GiB RMM pool, reused host sink capacity, GPU warm-up, and CPU affinity 56–59 control allocation/scheduling noise. Each configuration has 3 warmups and 15 measured iterations in each of 3 rotated rounds: 45 samples per cell, 6,480 measurements total. Wall-clock timing includes close and GPU synchronization; data generation, H2D and saving sample files are outside timing.

Median sample coefficient of variation: 1.24%; maximum 19.27%. Positive changes mean added latency. These measurements cover this DATE-only microbenchmark, not every ORC schema.

The highest CV is the baseline legacy 1K-row/NONE/4-chunk writer cell, where retained tail samples raise dispersion. Its median latency changes by +0.50%; a percentile bootstrap over the 45 samples per revision (10,000 resamples, seed 23794) gives a 95% median-ratio interval of -0.16% to +1.06%.

Representative legacy writer results compare the same calendar before and after the review fixes:

Earlier implementation-stage measurements are historical: upstream f45f266 → 3468260 default writers on RTX 5880 had -0.58% to +0.72% median changes across 1,080 measurements; 3468260 → 70eeb18 default reads/writes on A100 had -1.76% to +1.55% across 4,320 total measurements including legacy operations. These are separate baselines, not a direct final-versus-upstream measurement. The separate RTX legacy-stage runs had high dispersion and remain inconclusive.

#include <cudf/column/column_factories.hpp>
#include <cudf/copying.hpp>
#include <cudf/io/orc.hpp>
#include <cudf/table/table.hpp>
#include <cudf/utilities/memory_resource.hpp>
#include <rmm/mr/cuda_memory_resource.hpp>
#include <rmm/mr/pool_memory_resource.hpp>
#include <cuda_runtime_api.h>
#include <chrono>
#include <fstream>
#include <iomanip>
#include <iostream>
#include <stdexcept>
#include <string>
#include <vector>

void check(cudaError_t result) {
  if (result != cudaSuccess) throw std::runtime_error(cudaGetErrorString(result));
}

void warm_gpu();

int main(int argc, char** argv) {
  auto previous_mr = cudf::set_current_device_resource(
      rmm::mr::pool_memory_resource{rmm::mr::cuda_memory_resource{}, size_t{1} << 30});
  std::string label = argv[1], output = argv[2];
  bool legacy = argc > 3 && std::string(argv[3]) == "legacy";
  std::cout << "label,operation,calendar,rows,compression,chunks,sample,elapsed_ms,file_bytes\n";
  for (int n : {1000, 1000000, 10000000}) {
    auto column = cudf::make_timestamp_column(cudf::data_type{cudf::type_id::TIMESTAMP_DAYS}, n);
    std::vector<int32_t> data(n);
    uint32_t rng = 23794;
    for (auto& day : data) {
      rng = rng * 1664525u + 1013904223u;
      day = -353920 + static_cast<int32_t>(rng % 373770);
      if (legacy && day >= -141437 && day < -141427) day = -141427;
    }
    check(cudaMemcpy(column->mutable_view().data<int32_t>(), data.data(),
                     data.size() * sizeof(int32_t), cudaMemcpyHostToDevice));
    cudf::table_view input({column->view()});
    for (auto compression : {cudf::io::compression_type::NONE, cudf::io::compression_type::SNAPPY}) {
      std::string codec = compression == cudf::io::compression_type::NONE ? "NONE" : "SNAPPY";
      for (int chunks : {1, 4}) {
        auto pieces = cudf::slice(input, {0, n/4, n/4, n/2, n/2, n*3/4, n*3/4, n});
        warm_gpu();
        std::vector<char> bytes;
        auto emit = [&](std::string const& op, int sample, auto start) {
          check(cudaDeviceSynchronize());
          double ms = std::chrono::duration<double, std::milli>(
              std::chrono::steady_clock::now() - start).count();
          if (sample >= 0) std::cout << label << ',' << op << ','
              << (legacy ? "legacy" : "proleptic") << ',' << n << ',' << codec << ',' << chunks
              << ',' << sample << ',' << std::setprecision(9) << ms << ',' << bytes.size() << '\n';
        };
        for (int sample = -3; sample < 15; ++sample) {
          bytes.clear();
          check(cudaDeviceSynchronize());
          auto start = std::chrono::steady_clock::now();
          auto calendar = legacy ? cudf::io::orc::CalendarKind::JULIAN_GREGORIAN
                                 : cudf::io::orc::CalendarKind::PROLEPTIC_GREGORIAN;
          if (chunks == 1) {
            cudf::io::write_orc(cudf::io::orc_writer_options::builder(
                cudf::io::sink_info{&bytes}, input).compression(compression).calendar_kind(calendar));
          } else {
            cudf::io::orc_chunked_writer writer(cudf::io::chunked_orc_writer_options::builder(
                cudf::io::sink_info{&bytes}).compression(compression).calendar_kind(calendar));
            for (auto const& piece : pieces) writer.write(piece);
            writer.close();
          }
          emit("write", sample, start);
          if (n == 1000 && sample == 0) {
            std::ofstream file(output + '/' + label + '-' + codec + '-' +
                               std::to_string(chunks) + ".orc", std::ios::binary);
            file.write(bytes.data(), bytes.size());
          }
        }
        for (bool convert : {false, true}) {
#ifdef PHASE1
          if (convert) continue;
#endif
          for (int sample = -3; sample < 15; ++sample) {
            auto opts = cudf::io::orc_reader_options::builder(cudf::io::source_info{
                cudf::host_span<std::byte const>{reinterpret_cast<std::byte const*>(bytes.data()),
                                                 bytes.size()}}).use_np_dtypes(false).build();
#ifndef PHASE1
            opts.set_convert_dates_to_proleptic(convert);
#endif
            check(cudaDeviceSynchronize());
            auto start = std::chrono::steady_clock::now();
            auto result = cudf::io::read_orc(opts);
            emit(convert ? "read-convert" : "read-raw", sample, start);
            if (result.tbl->num_rows() != n) throw std::runtime_error("row count");
          }
        }
      }
    }
  }
  (void)cudf::set_current_device_resource(std::move(previous_mr));
}

#include <cuda_runtime.h>
#include <stdexcept>

__global__ void warm_kernel() {
  auto start = clock64();
  while (clock64() - start < 300000000ULL) {}
}

void warm_gpu() {
  warm_kernel<<<128, 128>>>();
  auto result = cudaDeviceSynchronize();
  if (result != cudaSuccess) throw std::runtime_error(cudaGetErrorString(result));
}

cmake_minimum_required(VERSION 4.0)
project(orc_calendar_review_benchmark LANGUAGES CXX CUDA)
find_package(rmm REQUIRED CONFIG)
find_package(cudf REQUIRED CONFIG)
find_package(CUDAToolkit REQUIRED)
foreach(stage IN ITEMS before after)
  add_executable(benchmark_${stage} benchmark.cpp warm.cu)
  target_compile_features(benchmark_${stage} PRIVATE cxx_std_20)
  target_link_libraries(benchmark_${stage} PRIVATE cudf::cudf CUDA::cudart)
endforeach()
target_include_directories(benchmark_before BEFORE PRIVATE before-include/cpp/include)

Updated the implementation and shortened the description as requested in this review. Historical benchmark results are in this comment.

current PR holds too many changes, which I think is really heavy to review. My plan is to split this PR into 2 parts: 1. c++ and its tests 2. Java API and tests. I'm validating if the split will cause downstream CICD block(cudf-spark premerge).

current PR holds too many changes, which I think is really heavy to review. My plan is to split this PR into 2 parts: 1. c++ and its tests 2. Java API and tests. I'm validating if the split will cause downstream CICD block(cudf-spark premerge).

Different people review C++ and Java changes, so I don't think this should be split this way. I think ~800 lines on C++ is manageable to review.

That said, is the PR ready for a second pass? :)

After several rounds of revisions, all changes are now consolidated in this PR, including Java/JNI. I also refined the implementation for readability. (Commits: c3d622a5f4, 7703f64077; local validation passed.)

Hi @vuule , this is ready for a second pass:)

Suggested review order for the consolidated changes:

1. Public contract and metadata — ORC options, orc_types.hpp, and orc_metadata.hpp: calendar values, defaults, opt-in reader conversion, and unsupported legacy inputs.
2. Calendar conversion — calendar.cuh: the conversion algorithm and lossless-conversion checks.
3. Writer path — writer_impl.cu: one-shot/chunked writes, nested DATE handling, null preservation, footer metadata, and DATE statistics.
4. Reader path — stripe_data.cu and reader_impl_decode.cu: opt-in conversion and error propagation. The shared kernel_error extraction and its Parquet call-site updates can be reviewed separately.
5. Bindings — Python statistics for datetime-range handling, then Java ORCOptions/ORCWriterOptions and TableJni.cpp for option forwarding.
6. Tests — C++ orc_test.cpp and orc_chunked_reader_test.cu, Python test_orc.py, then Java ORCCalendarTest and ORCCalendarInteropTest. These can also be read alongside each corresponding layer.

[MUST FIX] Forward convertDatesToProleptic through both Java chunked-reader JNI routes

The new shared option at ORCOptions.java:115 is silently ignored by both ORCChunkedReader constructors. A caller explicitly requesting conversion receives different dates when switching from Table.readORC to chunked reading.

I reproduced this on an NVIDIA L4 with JDK 17 using a CUDA 12 build of exactly f2595dc648e5a726e213e59a7ed6aebd93cebaba. The input is a valid historical-date compatibility case: 1582-10-04 is a valid date outside the 1582-10-05–1582-10-14 calendar cutover gap. The file is generated through the public writer API, with explicit JULIAN_GREGORIAN metadata. This run covers a flat DATE column with one historical value, the Unix epoch, and a null.

Reproduction steps:

1. Create column d with cuDF type TIMESTAMP_DAYS and values 1582-10-04, 1970-01-01, and null: [-141438, 0, null].
2. Write it using Table.writeORCChunked and ORCWriterOptions.builder().withColumns(true, "d").withCalendarKind(ORCWriterOptions.CalendarKind.JULIAN_GREGORIAN).build(). The writer produces an ORC DATE column with hybrid calendar metadata and raw epoch-day values [-141428, 0, null].
3. Build opts with ORCOptions.builder().withNumPyTypes(false).convertDatesToProleptic(true).build(). Disabling NumPy promotion is a supported option used here to inspect epoch days directly.
4. Read the file with Table.readORC(opts, file): it correctly returns [-141438, 0, null].
5. Load the same file bytes into a HostMemoryBuffer and collect all chunks through each of these constructors:

new ORCChunkedReader(0, 0, opts, buffer, 0, bytes.length);
new ORCChunkedReader(0, 0, 1, opts, buffer, 0, bytes.length);

The first two zeros specify unlimited chunk/pass byte limits. The second overload additionally sets output row sizing granularity to 1; the default-granularity constructor also reproduces the bug. These are the host-buffer constructors, without a custom DataSource.

With conversion enabled:

Both chunked routes return the unconverted hybrid epoch day. Interpreted as a cuDF proleptic date, the first value becomes 1582-10-14 instead of 1582-10-04: a silent 10-day shift. As a control, all three readers correctly return [-141428, 0, null] when conversion is disabled.

The missing propagation is visible in both Java constructor calls and their native declarations. Both JNI adapters reach the shared native builder, which never sets this option, leaving its default false. Ordinary reading already forwards it into the native builder.

Please thread the boolean through both Java constructor calls, both native declarations, both JNI adapters, and the shared native helper, then apply the existing .convert_dates_to_proleptic(flag) builder method. Add a regression covering both chunked constructors with conversion enabled and disabled, checking the independent historical-day anchors and the full three-row result against ordinary reading.

package ai.rapids.cudf;

import java.nio.file.Files;
import java.nio.file.Path;
import java.time.LocalDate;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

public final class ORCChunkedCalendarRepro {
  private static final Integer[] PROLEPTIC = {
      Math.toIntExact(LocalDate.parse("1582-10-04").toEpochDay()), 0, null};
  private static final Integer[] HYBRID = {-141428, 0, null};

  private static Integer[] days(Table table) {
    if (!table.getColumn(0).getType().equals(DType.TIMESTAMP_DAYS)) {
      throw new AssertionError("Expected TIMESTAMP_DAYS, got " + table.getColumn(0).getType());
    }
    try (HostColumnVector host = table.getColumn(0).copyToHost()) {
      Integer[] values = new Integer[(int) table.getRowCount()];
      for (int i = 0; i < values.length; ++i) {
        values[i] = host.isNull(i) ? null : host.getInt(i);
      }
      return values;
    }
  }

  private static void check(String route, Integer[] expected, Integer[] actual) {
    System.out.println(route + " actual=" + Arrays.toString(actual)
        + " expected=" + Arrays.toString(expected));
    if (!Arrays.equals(expected, actual)) {
      throw new AssertionError(route + " mismatch");
    }
  }

  private static Integer[] chunked(boolean granularity, boolean convert,
      HostMemoryBuffer buffer, long length) {
    ORCOptions opts = ORCOptions.builder().withNumPyTypes(false)
        .convertDatesToProleptic(convert).build();
    try (ORCChunkedReader reader = granularity
        ? new ORCChunkedReader(0, 0, 1, opts, buffer, 0, length)
        : new ORCChunkedReader(0, 0, opts, buffer, 0, length)) {
      List<Integer> output = new ArrayList<>();
      while (reader.hasNext()) {
        try (Table chunk = reader.readChunk()) {
          if (chunk != null) output.addAll(Arrays.asList(days(chunk)));
        }
      }
      return output.toArray(new Integer[0]);
    }
  }

  public static void main(String[] args) throws Exception {
    if (args.length != 2 || (!args[1].equals("--expect-bug") && !args[1].equals("--expect-fixed"))) {
      throw new IllegalArgumentException("Usage: <task-tmp-directory> --expect-bug|--expect-fixed");
    }
    boolean expectBug = args[1].equals("--expect-bug");
    Rmm.initialize(RmmAllocationMode.CUDA_DEFAULT, null, 0);
    try {
    Path directory = Path.of(args[0]);
    Files.createDirectories(directory);
    Path file = directory.resolve("chunked-calendar-repro.orc");
    System.out.println("API code source=" + ORCOptions.class.getProtectionDomain().getCodeSource());
    try (ColumnVector dates = ColumnVector.timestampDaysFromBoxedInts(PROLEPTIC);
         Table input = new Table(dates)) {
      ORCWriterOptions options = ORCWriterOptions.builder().withColumns(true, "d")
          .withCalendarKind(ORCWriterOptions.CalendarKind.JULIAN_GREGORIAN).build();
      try (TableWriter writer = Table.writeORCChunked(options, file.toFile())) {
        writer.write(input);
      }
    }
    ORCOptions enabled = ORCOptions.builder().withNumPyTypes(false)
        .convertDatesToProleptic(true).build();
    ORCOptions disabled = ORCOptions.builder().withNumPyTypes(false).build();
    try (Table table = Table.readORC(enabled, file.toFile())) {
      check("nonchunked enabled", PROLEPTIC, days(table));
    }
    try (Table table = Table.readORC(disabled, file.toFile())) {
      check("nonchunked disabled", HYBRID, days(table));
    }
    byte[] bytes = Files.readAllBytes(file);
    try (HostMemoryBuffer buffer = HostMemoryBuffer.allocate(bytes.length)) {
      buffer.setBytes(0, bytes, 0, bytes.length);
      for (boolean granularity : new boolean[]{false, true}) {
        String route = granularity ? "chunked explicit granularity" : "chunked default granularity";
        check(route + " disabled", HYBRID, chunked(granularity, false, buffer, bytes.length));
        check(route + " enabled", expectBug ? HYBRID : PROLEPTIC,
            chunked(granularity, true, buffer, bytes.length));
      }
    }
    System.out.println(expectBug
        ? "REPRODUCED: both chunked constructors silently ignore conversion=true"
        : "PASS: both chunked constructors honor conversion=true");
    } finally {
      Rmm.shutdown();
    }
  }
}

: "${CUDF_JAR:?Set CUDF_JAR to the tested commit's CUDA 12 JAR}"
: "${SLF4J_JARS:?Set SLF4J_JARS to the colon-separated SLF4J JARs}"
mkdir -p classes repro-work
javac -cp "$CUDF_JAR:$SLF4J_JARS" -d classes ORCChunkedCalendarRepro.java
java -ea -cp "classes:$CUDF_JAR:$SLF4J_JARS" \
  ai.rapids.cudf.ORCChunkedCalendarRepro "$PWD/repro-work" --expect-bug

nonchunked enabled actual=[-141438, 0, null] expected=[-141438, 0, null]
nonchunked disabled actual=[-141428, 0, null] expected=[-141428, 0, null]
chunked default granularity disabled actual=[-141428, 0, null] expected=[-141428, 0, null]
chunked default granularity enabled actual=[-141428, 0, null] expected=[-141428, 0, null]
chunked explicit granularity disabled actual=[-141428, 0, null] expected=[-141428, 0, null]
chunked explicit granularity enabled actual=[-141428, 0, null] expected=[-141428, 0, null]
REPRODUCED: both chunked constructors silently ignore conversion=true

Both chunked constructors now forward convertDatesToProleptic through JNI, addressing this finding. Relevant tests added.

/ok to test 4105b7e