lidavidm commented on a change in pull request #12014:
URL: https://github.com/apache/arrow/pull/12014#discussion_r773897638
##########
File path: cpp/src/arrow/array/validate.cc
##########
@@ -166,6 +166,86 @@ struct ValidateArrayImpl {
return Status::OK();
}
+ Status Visit(const Date64Type& type) {
+ // check that data is divisible by 8.64e7 (= 1000ms * 60s * 60mins * 24hrs)
+ RETURN_NOT_OK(ValidateFixedWidthBuffers());
+
+ using c_type = typename Date64Type::c_type;
+ if (full_validation) {
+ c_type fullDay = 1000 * 60 * 60 * 24;
+ return VisitArrayDataInline<Date64Type>(
+ data,
+ [&](c_type date) {
+ if(date % fullDay != 0) {
+ return Status::Invalid(type, date, "ms does not represent a
whole number of days");
+ }
+ return Status::OK();
+ },
+ []() { return Status::OK(); });
+ }
+ return Status::OK();
+ }
+
+ Status Visit(const Time32Type& type) {
+ // check unit
+ // if unit is s => data must be within [0, 8.64e4)
+ // if unit is ms => data must be within [0, 8.64e7)
+ RETURN_NOT_OK(ValidateFixedWidthBuffers());
+
+ using c_type = typename Time32Type::c_type;
+ if (full_validation) {
+ c_type fullDay_s = 60 * 60 * 24;
+ c_type fullDay_ms = fullDay_s * 1000;
+ return VisitArrayDataInline<Time32Type>(
+ data,
+ [&](c_type time) {
+ if(type.unit() == TimeUnit::SECOND && (time < 0 || time >=
fullDay_s)) {
+ return Status::Invalid(type, " ", time,
+ "s does not fit within the acceptable
range of [0, ",
+ fullDay_s, ") s");
+ }
+ if(type.unit() == TimeUnit::MILLI && (time < 0 || time >=
fullDay_ms)) {
+ return Status::Invalid(type, " ", time,
+ "ms does not fit within the acceptable
range of [0, ",
+ fullDay_ms, ") ms");
+ }
+ return Status::OK();
+ },
+ []() { return Status::OK(); });
+ }
+ return Status::OK();
+ }
+
+ Status Visit(const Time64Type& type) {
+ // check unit
+ // if unit is us => data must be within [0, 8.64e10)
+ // if unit is ns => data must be within [0, 8.64e13)
+ RETURN_NOT_OK(ValidateFixedWidthBuffers());
+
+ using c_type = typename Time64Type::c_type;
+ if (full_validation) {
+ c_type fullDay_us = 1000000ll * 60 * 60 * 24;
Review comment:
nit: use the `L` suffix over `l` so it's a little easier to distinguish
##########
File path: cpp/src/arrow/array/validate.cc
##########
@@ -166,6 +166,86 @@ struct ValidateArrayImpl {
return Status::OK();
}
+ Status Visit(const Date64Type& type) {
+ // check that data is divisible by 8.64e7 (= 1000ms * 60s * 60mins * 24hrs)
+ RETURN_NOT_OK(ValidateFixedWidthBuffers());
+
+ using c_type = typename Date64Type::c_type;
+ if (full_validation) {
+ c_type fullDay = 1000 * 60 * 60 * 24;
+ return VisitArrayDataInline<Date64Type>(
+ data,
+ [&](c_type date) {
+ if(date % fullDay != 0) {
+ return Status::Invalid(type, date, "ms does not represent a
whole number of days");
+ }
+ return Status::OK();
+ },
+ []() { return Status::OK(); });
+ }
+ return Status::OK();
+ }
+
+ Status Visit(const Time32Type& type) {
+ // check unit
+ // if unit is s => data must be within [0, 8.64e4)
+ // if unit is ms => data must be within [0, 8.64e7)
+ RETURN_NOT_OK(ValidateFixedWidthBuffers());
+
+ using c_type = typename Time32Type::c_type;
+ if (full_validation) {
+ c_type fullDay_s = 60 * 60 * 24;
+ c_type fullDay_ms = fullDay_s * 1000;
Review comment:
Note that for regular variable names, `snake_case` is preferred.
##########
File path: cpp/src/arrow/array/validate.cc
##########
@@ -166,6 +166,86 @@ struct ValidateArrayImpl {
return Status::OK();
}
+ Status Visit(const Date64Type& type) {
+ // check that data is divisible by 8.64e7 (= 1000ms * 60s * 60mins * 24hrs)
+ RETURN_NOT_OK(ValidateFixedWidthBuffers());
+
+ using c_type = typename Date64Type::c_type;
+ if (full_validation) {
+ c_type fullDay = 1000 * 60 * 60 * 24;
Review comment:
`constexpr c_type kFullDayMillis`? (Constants in the [Google style
guide](https://arrow.apache.org/docs/developers/cpp/development.html#code-style-linting-and-ci)
use
[`kCamelCase`](https://google.github.io/styleguide/cppguide.html#Constant_Names).)
##########
File path: cpp/src/arrow/array/validate.cc
##########
@@ -166,6 +166,86 @@ struct ValidateArrayImpl {
return Status::OK();
}
+ Status Visit(const Date64Type& type) {
+ // check that data is divisible by 8.64e7 (= 1000ms * 60s * 60mins * 24hrs)
+ RETURN_NOT_OK(ValidateFixedWidthBuffers());
+
+ using c_type = typename Date64Type::c_type;
+ if (full_validation) {
+ c_type fullDay = 1000 * 60 * 60 * 24;
+ return VisitArrayDataInline<Date64Type>(
+ data,
+ [&](c_type date) {
+ if(date % fullDay != 0) {
+ return Status::Invalid(type, date, "ms does not represent a
whole number of days");
+ }
+ return Status::OK();
+ },
+ []() { return Status::OK(); });
+ }
+ return Status::OK();
+ }
+
+ Status Visit(const Time32Type& type) {
+ // check unit
+ // if unit is s => data must be within [0, 8.64e4)
+ // if unit is ms => data must be within [0, 8.64e7)
+ RETURN_NOT_OK(ValidateFixedWidthBuffers());
+
+ using c_type = typename Time32Type::c_type;
+ if (full_validation) {
+ c_type fullDay_s = 60 * 60 * 24;
+ c_type fullDay_ms = fullDay_s * 1000;
Review comment:
Use constexpr here too.
--
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]
