Skip to content

Commit 623a6b1

Browse files
committed
feat(dwio): Add DATE to TIMESTAMP widening coercion in DWIO readers
Reading a DATE column as TIMESTAMP previously failed at schema-compatibility validation because DateType inherits from IntegerType (kind() == INTEGER), and integer→timestamp is not a numeric widening. DWIO readers now accept the coercion and emit Timestamp (days × 86400 seconds, 0 nanos) at read time. The widening is wired in three places. TypeUtils.cpp short-circuits isCompatible() when the file type is DATE and the requested kind is TIMESTAMP. SelectiveColumnReader gains convertDateToTimestampValues(), dispatched from getIntValues() when the requested kind is TIMESTAMP and the file type is DATE — it follows the same sourceRows iteration pattern as upcastScalarValues but applies the days × 86400 conversion and stores Timestamp structs. ParquetReader.cpp's convertType() for ConvertedType::DATE now also accepts TIMESTAMP as a valid requested type, in addition to DATE itself. Test plan: - velox/dwio/parquet/tests/reader/ParquetReaderTest.cpp: - readDateColumnAsTimestamp - readDateColumnAsTimestampWithNulls - readRealColumnAsDouble, readRealColumnAsDoubleWithNulls — coverage for pre-existing FLOAT→DOUBLE widening, no production change in this commit. - velox/dwio/dwrf/test/ReaderTest.cpp: - readDateColumnAsTimestamp - readDateColumnAsTimestampWithNulls
1 parent 4513605 commit 623a6b1

8 files changed

Lines changed: 907 additions & 2 deletions

File tree

velox/dwio/common/SelectiveColumnReader.cpp

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,25 @@ void SelectiveColumnReader::getIntValues(
303303
VELOX_FAIL("Unsupported value size: {}", valueSize_);
304304
}
305305
break;
306+
case TypeKind::TIMESTAMP:
307+
// DateType inherits from IntegerType (kind() == INTEGER), and DWRF
308+
// erases DATE to plain INT during write -- so the genuine-DATE case
309+
// and the post-DWRF-roundtrip case both present as kind() == INTEGER
310+
// here. Conversion (days * 86400) is the same in both.
311+
VELOX_CHECK_EQ(
312+
fileType_->type()->kind(),
313+
TypeKind::INTEGER,
314+
"TIMESTAMP output requires an INTEGER-kind file type (DATE or INTEGER), got: {}",
315+
fileType_->type()->toString());
316+
if (valueSize_ == sizeof(Timestamp)) {
317+
// A prior convertDateToTimestampValues call already bulk-transmuted
318+
// the int32 days buffer to Timestamps; further extractions are plain
319+
// Timestamp compactions.
320+
getFlatValues<Timestamp, Timestamp>(rows, result, TIMESTAMP());
321+
} else {
322+
convertDateToTimestampValues(rows, result);
323+
}
324+
break;
306325
default:
307326
VELOX_FAIL(
308327
"Not a valid type for integer reader: {}", requestedType->toString());
@@ -452,6 +471,108 @@ void SelectiveColumnReader::compactScalarValues<bool, bool>(
452471
values_->setSize(bits::nbytes(numValues_));
453472
}
454473

474+
void SelectiveColumnReader::convertDateToTimestampValues(
475+
const RowSet& rows,
476+
VectorPtr* result,
477+
bool isFinal) {
478+
VELOX_CHECK_EQ(valueSize_, sizeof(int32_t));
479+
VELOX_CHECK(mayGetValues_);
480+
481+
if (allNull_) {
482+
*result = std::make_shared<ConstantVector<Timestamp>>(
483+
pool_, rows.size(), true, TIMESTAMP(), Timestamp());
484+
if (isFinal) {
485+
mayGetValues_ = false;
486+
}
487+
return;
488+
}
489+
VELOX_CHECK_NOT_NULL(values_);
490+
491+
static constexpr int64_t kSecondsPerDay{86'400};
492+
493+
// Bulk path: this single call asks for at least half of the read values.
494+
// Transmute all numValues_ days to Timestamps in place (amortizing the
495+
// conversion), transition to State B, then emit the requested slice via
496+
// standard Timestamp extraction. Iterates backwards so the wider
497+
// Timestamp writes don't clobber int32 source bytes we haven't read yet.
498+
if (rows.size() >= numValues_ / 2) {
499+
ensureValuesCapacity<Timestamp>(numValues_, true);
500+
const auto* rawDays = reinterpret_cast<const int32_t*>(rawValues_);
501+
auto* timestamps = reinterpret_cast<Timestamp*>(rawValues_);
502+
for (auto i = numValues_; i-- > 0;) {
503+
timestamps[i] =
504+
Timestamp(kSecondsPerDay * static_cast<int64_t>(rawDays[i]), 0);
505+
}
506+
valueSize_ = sizeof(Timestamp);
507+
values_->setSize(numValues_ * sizeof(Timestamp));
508+
getFlatValues<Timestamp, Timestamp>(rows, result, TIMESTAMP(), isFinal);
509+
return;
510+
}
511+
512+
// Small-subset path: leave the int32 days buffer intact and emit a
513+
// freshly-allocated Timestamp vector for just rows.size(). Multi-call
514+
// safe because values_, valueSize_, valueRows_ are untouched.
515+
const auto sourceRows = valueRows_.empty()
516+
? (outputRows_.empty() ? RowSet(inputRows_) : RowSet(outputRows_))
517+
: RowSet(valueRows_);
518+
const auto* rawDays = reinterpret_cast<const int32_t*>(rawValues_);
519+
520+
auto timestampValues = AlignedBuffer::allocate<Timestamp>(
521+
rows.size() + simd::kPadding / sizeof(Timestamp), pool_);
522+
auto* timestamps = timestampValues->asMutable<Timestamp>();
523+
524+
const auto* moveNullsFrom = shouldMoveNulls(rows);
525+
BufferPtr localNullsBuffer;
526+
uint64_t* localNulls = nullptr;
527+
if (moveNullsFrom) {
528+
localNullsBuffer = AlignedBuffer::allocate<bool>(rows.size(), pool_);
529+
localNulls = localNullsBuffer->asMutable<uint64_t>();
530+
}
531+
532+
size_t sourceIndex{0};
533+
if (moveNullsFrom) {
534+
for (vector_size_t rowIndex = 0; rowIndex < rows.size();) {
535+
const auto begin = rowIndex;
536+
const auto end = std::min<vector_size_t>(begin + 64, rows.size());
537+
uint64_t nulls = 0;
538+
for (; rowIndex < end; ++rowIndex) {
539+
while (sourceRows[sourceIndex] < rows[rowIndex]) {
540+
++sourceIndex;
541+
}
542+
VELOX_DCHECK_EQ(sourceRows[sourceIndex], rows[rowIndex]);
543+
timestamps[rowIndex] = Timestamp(
544+
kSecondsPerDay * static_cast<int64_t>(rawDays[sourceIndex]), 0);
545+
nulls |=
546+
static_cast<uint64_t>(bits::isBitSet(moveNullsFrom, sourceIndex))
547+
<< (rowIndex - begin);
548+
++sourceIndex;
549+
}
550+
localNulls[begin / 64] = nulls;
551+
}
552+
} else {
553+
for (vector_size_t rowIndex = 0; rowIndex < rows.size(); ++rowIndex) {
554+
while (sourceRows[sourceIndex] < rows[rowIndex]) {
555+
++sourceIndex;
556+
}
557+
VELOX_DCHECK_EQ(sourceRows[sourceIndex], rows[rowIndex]);
558+
timestamps[rowIndex] = Timestamp(
559+
kSecondsPerDay * static_cast<int64_t>(rawDays[sourceIndex++]), 0);
560+
}
561+
}
562+
563+
*result = std::make_shared<FlatVector<Timestamp>>(
564+
pool_,
565+
TIMESTAMP(),
566+
moveNullsFrom ? localNullsBuffer : resultNulls(),
567+
rows.size(),
568+
std::move(timestampValues),
569+
std::vector<BufferPtr>{});
570+
571+
if (isFinal) {
572+
mayGetValues_ = false;
573+
}
574+
}
575+
455576
char* SelectiveColumnReader::copyStringValue(std::string_view value) {
456577
uint64_t size = value.size();
457578
if (stringBuffers_.empty() || rawStringUsed_ + size > rawStringSize_) {

velox/dwio/common/SelectiveColumnReader.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,23 @@ class SelectiveColumnReader {
595595
template <typename T, typename TVector>
596596
void upcastScalarValues(const RowSet& rows);
597597

598+
/// Reads DATE (int32 days-since-epoch) values and converts them to
599+
/// Timestamp (seconds = days × 86400, nanoseconds = 0).
600+
/// Emits Timestamps for 'rows' from a buffer of int32 days-since-epoch read
601+
/// by prepareRead<int32_t>. Reentrant: can be called multiple times for
602+
/// disjoint subsets of 'rows' from the same read, mirroring getFlatValues.
603+
/// If rows.size() >= numValues_/2 the int32 buffer is transmuted to
604+
/// Timestamps in place (amortizing the conversion) and valueSize_ becomes
605+
/// sizeof(Timestamp); subsequent extractions should use
606+
/// getFlatValues<Timestamp, Timestamp> directly. Otherwise emits a fresh
607+
/// Timestamp vector and leaves the source buffer intact. Caller (getIntValues)
608+
/// dispatches on valueSize_ before calling. Pass isFinal=true on the last
609+
/// call to release the mayGetValues_ lock.
610+
void convertDateToTimestampValues(
611+
const RowSet& rows,
612+
VectorPtr* result,
613+
bool isFinal = false);
614+
598615
// For complex type column, we need to compact only nulls if the rows are
599616
// shrinked. Child fields are handled recursively in their own column
600617
// readers.

velox/dwio/common/TypeUtils.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,18 @@ void checkTypeCompatibility(
130130
const FKind& kind,
131131
const FShouldRead& shouldRead,
132132
const std::function<std::string()>& exceptionMessageCreator) {
133-
if (shouldRead(to) && !isCompatible(from.kind(), kind(to))) {
133+
// Special case: DATE (stored as int32 days-since-epoch) may be widened to
134+
// TIMESTAMP (seconds-since-epoch). The semantic conversion (days × 86400)
135+
// is performed at read time by convertDateToTimestampValues(). DWRF erases
136+
// DATE to plain INT in the proto schema during write, so on the read side
137+
// the file-side kind() is INTEGER in both the genuine-DATE case (DateType
138+
// inherits from IntegerType, kind() == INTEGER) and the post-roundtrip
139+
// case. Match on kind() == INTEGER so both shapes resolve.
140+
const bool isIntToTimestamp = from.kind() == TypeKind::INTEGER &&
141+
kind(to) == TypeKind::TIMESTAMP;
142+
143+
if (shouldRead(to) && !isIntToTimestamp &&
144+
!isCompatible(from.kind(), kind(to))) {
134145
VELOX_SCHEMA_MISMATCH_ERROR(
135146
fmt::format(
136147
"{}, From Kind: {}, To Kind: {}",

0 commit comments

Comments
 (0)