wgtmac commented on code in PR #1761:
URL: https://github.com/apache/orc/pull/1761#discussion_r1468510283
##########
c++/src/ConvertColumnReader.cc:
##########
@@ -593,6 +593,105 @@ namespace orc {
int32_t toScale;
};
+ template <typename FileTypeBatch>
+ class DecimalToTimestampColumnReader : public ConvertToTimestampColumnReader
{
+ public:
+ DecimalToTimestampColumnReader(const Type& _readType, const Type& fileType,
+ StripeStreams& stripe, bool
_throwOnOverflow)
+ : ConvertToTimestampColumnReader(_readType, fileType, stripe,
_throwOnOverflow),
+ precision(static_cast<int32_t>(fileType.getPrecision())),
+ scale(static_cast<int32_t>(fileType.getScale())) {}
+
+ void next(ColumnVectorBatch& rowBatch, uint64_t numValues, char* notNull)
override {
+ ConvertColumnReader::next(rowBatch, numValues, notNull);
+ const auto& srcBatch = *SafeCastBatchTo<const
FileTypeBatch*>(data.get());
+ auto& dstBatch = *SafeCastBatchTo<TimestampVectorBatch*>(&rowBatch);
+ for (uint64_t i = 0; i < rowBatch.numElements; ++i) {
+ if (!rowBatch.hasNulls || rowBatch.notNull[i]) {
+ convertDecimalToTimestamp(dstBatch, i, srcBatch);
+ }
+ }
+ }
+
+ private:
+ void convertDecimalToTimestamp(TimestampVectorBatch& dstBatch, uint64_t
idx,
+ const FileTypeBatch& srcBatch) {
+ constexpr int SecondToNanoFactor = 9;
+ // '-1000000000-01-01T00:00Z'
Review Comment:
Where are the min/max values defined? It would be good to add a comment for
record.
##########
c++/src/ConvertColumnReader.cc:
##########
@@ -593,6 +593,105 @@ namespace orc {
int32_t toScale;
};
+ template <typename FileTypeBatch>
+ class DecimalToTimestampColumnReader : public ConvertToTimestampColumnReader
{
+ public:
+ DecimalToTimestampColumnReader(const Type& _readType, const Type& fileType,
+ StripeStreams& stripe, bool
_throwOnOverflow)
+ : ConvertToTimestampColumnReader(_readType, fileType, stripe,
_throwOnOverflow),
+ precision(static_cast<int32_t>(fileType.getPrecision())),
+ scale(static_cast<int32_t>(fileType.getScale())) {}
+
+ void next(ColumnVectorBatch& rowBatch, uint64_t numValues, char* notNull)
override {
+ ConvertColumnReader::next(rowBatch, numValues, notNull);
+ const auto& srcBatch = *SafeCastBatchTo<const
FileTypeBatch*>(data.get());
+ auto& dstBatch = *SafeCastBatchTo<TimestampVectorBatch*>(&rowBatch);
+ for (uint64_t i = 0; i < rowBatch.numElements; ++i) {
+ if (!rowBatch.hasNulls || rowBatch.notNull[i]) {
+ convertDecimalToTimestamp(dstBatch, i, srcBatch);
+ }
+ }
+ }
+
+ private:
+ void convertDecimalToTimestamp(TimestampVectorBatch& dstBatch, uint64_t
idx,
+ const FileTypeBatch& srcBatch) {
+ constexpr int SecondToNanoFactor = 9;
+ // '-1000000000-01-01T00:00Z'
+ constexpr int64_t MIN_EPOCH_SECONDS = -31557014167219200L;
+ // '1000000000-12-31T23:59:59.999999999Z'
+ constexpr int64_t MAX_EPOCH_SECONDS = 31556889864403199L;
+ // dummy variable, there's no risk of overflow
+
+ Int128 i128(srcBatch.values[idx]);
+ bool overflow = false;
+ Int128 integerPortion = scaleDownInt128ByPowerOfTen(i128, scale);
+ if (integerPortion < MIN_EPOCH_SECONDS || integerPortion >
MAX_EPOCH_SECONDS) {
+ handleOverflow<Decimal, int64_t>(dstBatch, idx, throwOnOverflow);
+ return;
+ }
+ i128 -= scaleUpInt128ByPowerOfTen(integerPortion, scale, overflow);
+ Int128 fractionPortion = std::move(i128);
+ if (scale < SecondToNanoFactor) {
+ fractionPortion =
+ scaleUpInt128ByPowerOfTen(fractionPortion, SecondToNanoFactor -
scale, overflow);
+ } else {
+ fractionPortion = scaleDownInt128ByPowerOfTen(fractionPortion, scale -
SecondToNanoFactor);
+ }
+ if (fractionPortion < 0) {
+ fractionPortion += 1e9;
+ integerPortion -= 1;
+ }
+ dstBatch.data[idx] = integerPortion.toLong();
+ dstBatch.nanoseconds[idx] = fractionPortion.toLong();
+
+ if (needConvertTimezone) {
+ dstBatch.data[idx] = readerTimezone.convertFromUTC(dstBatch.data[idx]);
+ }
+ }
+
+ const int32_t precision;
+ const int32_t scale;
+ };
+
+ template <typename FileTypeBatch>
+ class DecimalToStringVariantColumnReader : public
ConvertToStringVariantColumnReader {
+ public:
+ DecimalToStringVariantColumnReader(const Type& _readType, const Type&
fileType,
+ StripeStreams& stripe, bool
_throwOnOverflow)
+ : ConvertToStringVariantColumnReader(_readType, fileType, stripe,
_throwOnOverflow),
Review Comment:
Unrelated to this PR: we have many weird `_xxx` variable names appearing in
the function signature. I'm thinking to do something like apache/arrow, which
only use `xxx_` as the name of private class member variables. In this way, we
can get rid of the weird case like this.
##########
c++/src/ConvertColumnReader.cc:
##########
@@ -593,6 +593,105 @@ namespace orc {
int32_t toScale;
};
+ template <typename FileTypeBatch>
+ class DecimalToTimestampColumnReader : public ConvertToTimestampColumnReader
{
+ public:
+ DecimalToTimestampColumnReader(const Type& _readType, const Type& fileType,
+ StripeStreams& stripe, bool
_throwOnOverflow)
+ : ConvertToTimestampColumnReader(_readType, fileType, stripe,
_throwOnOverflow),
+ precision(static_cast<int32_t>(fileType.getPrecision())),
+ scale(static_cast<int32_t>(fileType.getScale())) {}
+
+ void next(ColumnVectorBatch& rowBatch, uint64_t numValues, char* notNull)
override {
+ ConvertColumnReader::next(rowBatch, numValues, notNull);
+ const auto& srcBatch = *SafeCastBatchTo<const
FileTypeBatch*>(data.get());
+ auto& dstBatch = *SafeCastBatchTo<TimestampVectorBatch*>(&rowBatch);
+ for (uint64_t i = 0; i < rowBatch.numElements; ++i) {
+ if (!rowBatch.hasNulls || rowBatch.notNull[i]) {
+ convertDecimalToTimestamp(dstBatch, i, srcBatch);
+ }
+ }
+ }
+
+ private:
+ void convertDecimalToTimestamp(TimestampVectorBatch& dstBatch, uint64_t
idx,
+ const FileTypeBatch& srcBatch) {
+ constexpr int SecondToNanoFactor = 9;
+ // '-1000000000-01-01T00:00Z'
+ constexpr int64_t MIN_EPOCH_SECONDS = -31557014167219200L;
+ // '1000000000-12-31T23:59:59.999999999Z'
+ constexpr int64_t MAX_EPOCH_SECONDS = 31556889864403199L;
+ // dummy variable, there's no risk of overflow
+
+ Int128 i128(srcBatch.values[idx]);
+ bool overflow = false;
+ Int128 integerPortion = scaleDownInt128ByPowerOfTen(i128, scale);
+ if (integerPortion < MIN_EPOCH_SECONDS || integerPortion >
MAX_EPOCH_SECONDS) {
+ handleOverflow<Decimal, int64_t>(dstBatch, idx, throwOnOverflow);
+ return;
+ }
+ i128 -= scaleUpInt128ByPowerOfTen(integerPortion, scale, overflow);
+ Int128 fractionPortion = std::move(i128);
+ if (scale < SecondToNanoFactor) {
+ fractionPortion =
+ scaleUpInt128ByPowerOfTen(fractionPortion, SecondToNanoFactor -
scale, overflow);
+ } else {
+ fractionPortion = scaleDownInt128ByPowerOfTen(fractionPortion, scale -
SecondToNanoFactor);
+ }
+ if (fractionPortion < 0) {
+ fractionPortion += 1e9;
+ integerPortion -= 1;
+ }
+ dstBatch.data[idx] = integerPortion.toLong();
Review Comment:
line 630 has guaranteed `toLong()` will not overflow?
##########
c++/src/ConvertColumnReader.cc:
##########
@@ -593,6 +593,105 @@ namespace orc {
int32_t toScale;
};
+ template <typename FileTypeBatch>
+ class DecimalToTimestampColumnReader : public ConvertToTimestampColumnReader
{
+ public:
+ DecimalToTimestampColumnReader(const Type& _readType, const Type& fileType,
+ StripeStreams& stripe, bool
_throwOnOverflow)
+ : ConvertToTimestampColumnReader(_readType, fileType, stripe,
_throwOnOverflow),
+ precision(static_cast<int32_t>(fileType.getPrecision())),
+ scale(static_cast<int32_t>(fileType.getScale())) {}
+
+ void next(ColumnVectorBatch& rowBatch, uint64_t numValues, char* notNull)
override {
+ ConvertColumnReader::next(rowBatch, numValues, notNull);
+ const auto& srcBatch = *SafeCastBatchTo<const
FileTypeBatch*>(data.get());
+ auto& dstBatch = *SafeCastBatchTo<TimestampVectorBatch*>(&rowBatch);
+ for (uint64_t i = 0; i < rowBatch.numElements; ++i) {
+ if (!rowBatch.hasNulls || rowBatch.notNull[i]) {
+ convertDecimalToTimestamp(dstBatch, i, srcBatch);
+ }
+ }
+ }
+
+ private:
+ void convertDecimalToTimestamp(TimestampVectorBatch& dstBatch, uint64_t
idx,
+ const FileTypeBatch& srcBatch) {
+ constexpr int SecondToNanoFactor = 9;
+ // '-1000000000-01-01T00:00Z'
+ constexpr int64_t MIN_EPOCH_SECONDS = -31557014167219200L;
+ // '1000000000-12-31T23:59:59.999999999Z'
+ constexpr int64_t MAX_EPOCH_SECONDS = 31556889864403199L;
+ // dummy variable, there's no risk of overflow
Review Comment:
Where is it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
