Skip to content
20 changes: 19 additions & 1 deletion cpp/include/cudf/io/orc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <cudf/types.hpp>
#include <cudf/utilities/export.hpp>
#include <cudf/utilities/memory_resource.hpp>
#include <cudf/utilities/traits.hpp>

#include <memory>
#include <optional>
Expand Down Expand Up @@ -277,9 +278,23 @@ class orc_reader_options {
/**
* @brief Sets timestamp type to which timestamp column will be cast.
*
* Only TIMESTAMP_SECONDS, TIMESTAMP_MILLISECONDS, TIMESTAMP_MICROSECONDS and
* TIMESTAMP_NANOSECONDS are supported as the target type; EMPTY, the default, reads timestamps
* as TIMESTAMP_NANOSECONDS. For a coarser resolution such as TIMESTAMP_DAYS, cast the output
* column instead.
Comment thread
galipremsagar marked this conversation as resolved.
Resolved
*
* @throw cudf::logic_error if `type` is not EMPTY, TIMESTAMP_SECONDS, TIMESTAMP_MILLISECONDS,
* TIMESTAMP_MICROSECONDS or TIMESTAMP_NANOSECONDS
Comment thread
galipremsagar marked this conversation as resolved.
Resolved
*
* @param type Type of timestamp
*/
void set_timestamp_type(data_type type) { _timestamp_type = type; }
void set_timestamp_type(data_type type)
{
CUDF_EXPECTS(type.id() == type_id::EMPTY or
(cudf::is_timestamp(type) and type.id() != type_id::TIMESTAMP_DAYS),
"timestamp_type must be EMPTY or a second-based TIMESTAMP type");
Comment thread
galipremsagar marked this conversation as resolved.
Resolved
_timestamp_type = type;
}

/**
* @brief Set columns that should be read as 128-bit Decimal
Expand Down Expand Up @@ -403,6 +418,9 @@ class orc_reader_options_builder {
/**
* @brief Sets timestamp type to which timestamp column will be cast.
*
* @throw cudf::logic_error if `type` is not EMPTY, TIMESTAMP_SECONDS, TIMESTAMP_MILLISECONDS,
* TIMESTAMP_MICROSECONDS or TIMESTAMP_NANOSECONDS
*
* @param type Type of timestamp
* @return this for chaining
*/
Expand Down
20 changes: 19 additions & 1 deletion cpp/include/cudf/io/parquet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <cudf/types.hpp>
#include <cudf/utilities/export.hpp>
#include <cudf/utilities/memory_resource.hpp>
#include <cudf/utilities/traits.hpp>

#include <memory>
#include <optional>
Expand Down Expand Up @@ -619,9 +620,23 @@ class parquet_reader_options {
/**
* @brief Sets timestamp_type used to cast timestamp columns.
*
* Only TIMESTAMP_SECONDS, TIMESTAMP_MILLISECONDS, TIMESTAMP_MICROSECONDS and
* TIMESTAMP_NANOSECONDS are supported as the target type; EMPTY, the default, keeps each
* column's own resolution. For a coarser resolution such as TIMESTAMP_DAYS, cast the output
* column instead.
*
* @throw cudf::logic_error if `type` is not EMPTY, TIMESTAMP_SECONDS, TIMESTAMP_MILLISECONDS,
* TIMESTAMP_MICROSECONDS or TIMESTAMP_NANOSECONDS
*
* @param type The timestamp data_type to which all timestamp columns need to be cast
*/
void set_timestamp_type(data_type type) { _timestamp_type = type; }
void set_timestamp_type(data_type type)
{
CUDF_EXPECTS(type.id() == type_id::EMPTY or
(cudf::is_timestamp(type) and type.id() != type_id::TIMESTAMP_DAYS),
"timestamp_type must be EMPTY or a second-based TIMESTAMP type");
_timestamp_type = type;
}

/**
* @brief Sets decimal width used to cast decimal columns.
Expand Down Expand Up @@ -900,6 +915,9 @@ class parquet_reader_options_builder {
/**
* @brief timestamp_type used to cast timestamp columns.
*
* @throw cudf::logic_error if `type` is not EMPTY, TIMESTAMP_SECONDS, TIMESTAMP_MILLISECONDS,
* TIMESTAMP_MICROSECONDS or TIMESTAMP_NANOSECONDS
*
* @param type The timestamp data_type to which all timestamp columns need to be cast
* @return this for chaining
*/
Expand Down
25 changes: 25 additions & 0 deletions cpp/tests/io/orc_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1539,6 +1539,31 @@ TEST_F(OrcReaderTest, ZeroColumnsPreservesRowCount)
EXPECT_EQ(result.tbl->view().num_rows(), num_rows);
}

TEST_F(OrcReaderTest, TimestampTypeMustHaveSecondBasedUnit)
{
// The decode kernel converts timestamps to seconds, milliseconds, microseconds or nanoseconds
// and stores 64-bit values, so a request for TIMESTAMP_DAYS would write 8-byte nanosecond
// counts into a column allocated at 4 bytes per row.
auto options = cudf::io::orc_reader_options{};
for (auto const id : {cudf::type_id::EMPTY,
cudf::type_id::TIMESTAMP_SECONDS,
cudf::type_id::TIMESTAMP_MILLISECONDS,
cudf::type_id::TIMESTAMP_MICROSECONDS,
cudf::type_id::TIMESTAMP_NANOSECONDS}) {
EXPECT_NO_THROW(options.set_timestamp_type(cudf::data_type{id}));
EXPECT_EQ(options.get_timestamp_type().id(), id);
}
for (auto const id :
{cudf::type_id::TIMESTAMP_DAYS, cudf::type_id::DURATION_SECONDS, cudf::type_id::INT64}) {
EXPECT_THROW(options.set_timestamp_type(cudf::data_type{id}), cudf::logic_error);
}
// A rejected type leaves the previous one in place.
EXPECT_EQ(options.get_timestamp_type().id(), cudf::type_id::TIMESTAMP_NANOSECONDS);
EXPECT_THROW(cudf::io::orc_reader_options_builder{}.timestamp_type(
cudf::data_type{cudf::type_id::TIMESTAMP_DAYS}),
cudf::logic_error);
Comment thread
galipremsagar marked this conversation as resolved.
Resolved
}

TEST_F(OrcReaderTest, SingleInputs)
{
srand(31533);
Expand Down
57 changes: 57 additions & 0 deletions cpp/tests/io/parquet_reader_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1011,6 +1011,63 @@ TEST_F(ParquetReaderTest, SelectColumnByMissingFieldIds)
}
}

TEST_F(ParquetReaderTest, TimestampTypeMustHaveSecondBasedUnit)
Comment thread
galipremsagar marked this conversation as resolved.
Resolved
{
// The reader rescales timestamps by a clock rate in ticks per second, which TIMESTAMP_DAYS
// cannot express: a request for it used to decode seconds as 8-byte values into a column
// allocated at 4 bytes per row.
auto options = cudf::io::parquet_reader_options{};
for (auto const id : {cudf::type_id::EMPTY,
cudf::type_id::TIMESTAMP_SECONDS,
cudf::type_id::TIMESTAMP_MILLISECONDS,
cudf::type_id::TIMESTAMP_MICROSECONDS,
cudf::type_id::TIMESTAMP_NANOSECONDS}) {
EXPECT_NO_THROW(options.set_timestamp_type(cudf::data_type{id}));
EXPECT_EQ(options.get_timestamp_type().id(), id);
}
for (auto const id :
{cudf::type_id::TIMESTAMP_DAYS, cudf::type_id::DURATION_SECONDS, cudf::type_id::INT64}) {
EXPECT_THROW(options.set_timestamp_type(cudf::data_type{id}), cudf::logic_error);
}
// A rejected type leaves the previous one in place.
EXPECT_EQ(options.get_timestamp_type().id(), cudf::type_id::TIMESTAMP_NANOSECONDS);
EXPECT_THROW(cudf::io::parquet_reader_options_builder{}.timestamp_type(
cudf::data_type{cudf::type_id::TIMESTAMP_DAYS}),
cudf::logic_error);
Comment thread
galipremsagar marked this conversation as resolved.
Resolved
}

TEST_F(ParquetReaderTest, ReadTimestampsAsSeconds)
{
// SupportedTimestampTypes (parquet_common.hpp) has no seconds member, so this is the read
// coverage for the TIMESTAMP_SECONDS target: each finer unit is floored to whole seconds,
// negative values included.
auto const check = [&]<typename Timestamp>() {
constexpr int64_t per_second = Timestamp::period::den;
std::vector<int64_t> const ticks{0,
per_second - 1,
per_second,
1'767'314'700 * per_second + per_second / 2,
-1,
-per_second,
-per_second - 1};
auto const input = column_wrapper<Timestamp, int64_t>(ticks.begin(), ticks.end());
auto const expected =
column_wrapper<cudf::timestamp_s, cudf::timestamp_s::rep>{0, 0, 1, 1'767'314'700, -1, -1, -2};
auto const filepath =
write_parquet_temp_file(cudf::table_view{{input}},
"ReadTimestampsAsSeconds_" + std::to_string(per_second) + ".parquet");

auto const result = cudf::io::read_parquet(
cudf::io::parquet_reader_options::builder(cudf::io::source_info{filepath})
.timestamp_type(cudf::data_type{cudf::type_id::TIMESTAMP_SECONDS})
.build());
CUDF_TEST_EXPECT_COLUMNS_EQUAL(result.tbl->view().column(0), expected);
};
check.template operator()<cudf::timestamp_ms>();
check.template operator()<cudf::timestamp_us>();
check.template operator()<cudf::timestamp_ns>();
}

TEST_F(ParquetReaderTest, ColumnSelectionModesAreExclusive)
{
auto selected_by_name = cudf::io::parquet_reader_options{};
Expand Down
11 changes: 9 additions & 2 deletions java/src/main/java/ai/rapids/cudf/ORCOptions.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
*
* SPDX-FileCopyrightText: Copyright (c) 2019-2025, NVIDIA CORPORATION.
* SPDX-FileCopyrightText: Copyright (c) 2019-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*
*/
Expand Down Expand Up @@ -73,11 +73,18 @@ public Builder withNumPyTypes(boolean useNumPyTypes) {

/**
* Specify the time unit to use when returning timestamps.
* DType.EMPTY (the default), TIMESTAMP_SECONDS, TIMESTAMP_MILLISECONDS,
* TIMESTAMP_MICROSECONDS and TIMESTAMP_NANOSECONDS are valid; for TIMESTAMP_DAYS,
* cast the returned column instead.
* @param unit default unit of time specified by the user
* @return builder for chaining
* @throws IllegalArgumentException if {@code unit} is not one of those types
*/
public ORCOptions.Builder withTimeUnit(DType unit) {
assert unit.isTimestampType();
if (unit == null || (unit != DType.EMPTY && !unit.hasTimeResolution())) {
throw new IllegalArgumentException(
"Time unit must be EMPTY or a second-based timestamp type, got " + unit);
}
Comment thread
galipremsagar marked this conversation as resolved.
Resolved
this.unit = unit;
return this;
}
Expand Down
11 changes: 9 additions & 2 deletions java/src/main/java/ai/rapids/cudf/ParquetOptions.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
*
* SPDX-FileCopyrightText: Copyright (c) 2019, NVIDIA CORPORATION.
* SPDX-FileCopyrightText: Copyright (c) 2019-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*
*/
Expand Down Expand Up @@ -48,11 +48,18 @@ public static class Builder extends ColumnFilterOptions.Builder<Builder> {

/**
* Specify the time unit to use when returning timestamps.
* DType.EMPTY (the default), TIMESTAMP_SECONDS, TIMESTAMP_MILLISECONDS,
* TIMESTAMP_MICROSECONDS and TIMESTAMP_NANOSECONDS are valid; for TIMESTAMP_DAYS,
* cast the returned column instead.
Comment thread
galipremsagar marked this conversation as resolved.
Resolved
* @param unit default unit of time specified by the user
* @return builder for chaining
* @throws IllegalArgumentException if {@code unit} is not one of those types
*/
public Builder withTimeUnit(DType unit) {
assert unit.isTimestampType();
if (unit == null || (unit != DType.EMPTY && !unit.hasTimeResolution())) {
throw new IllegalArgumentException(
"Time unit must be EMPTY or a second-based timestamp type, got " + unit);
}
this.unit = unit;
return this;
}
Expand Down
15 changes: 15 additions & 0 deletions java/src/test/java/ai/rapids/cudf/TableTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1936,6 +1936,21 @@ void testReadORCTimeUnit() {
}
}

@Test
void testReaderTimeUnitRejectsTimestampDays() {
// Both readers convert timestamps to a second-based unit only, so TIMESTAMP_DAYS is
// rejected up front; the four second-based units and the default are accepted.
assertThrows(IllegalArgumentException.class,
() -> ParquetOptions.builder().withTimeUnit(DType.TIMESTAMP_DAYS));
assertThrows(IllegalArgumentException.class,
() -> ORCOptions.builder().withTimeUnit(DType.TIMESTAMP_DAYS));
for (DType unit : new DType[]{DType.EMPTY, DType.TIMESTAMP_SECONDS,
DType.TIMESTAMP_MILLISECONDS, DType.TIMESTAMP_MICROSECONDS, DType.TIMESTAMP_NANOSECONDS}) {
assertDoesNotThrow(() -> ParquetOptions.builder().withTimeUnit(unit));
assertDoesNotThrow(() -> ORCOptions.builder().withTimeUnit(unit));
}
}
Comment thread
galipremsagar marked this conversation as resolved.
Resolved

@Test
void testORCChunkedReader() throws IOException {
byte[] buffer = Files.readAllBytes(TEST_ORC_FILE_CHUNKED_READ.toPath());
Expand Down
Loading