You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/05/26 02:26:25 UTC
[GitHub] [arrow] anthonylouisbsb opened a new pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
anthonylouisbsb opened a new pull request #10402:
URL: https://github.com/apache/arrow/pull/10402
Adds the implementation for the **castTIME(int32)** and **castTIME(utf8)** functions and add tests for the **castTIMESTAMP(int64)** function.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#issuecomment-1019747942
Benchmark runs are scheduled for baseline = 54460d96ba1d613e472d8d9a96c072147e736b4d and contender = 8e34b64f60120bdee5991148f765cd4452f0e0d7. 8e34b64f60120bdee5991148f765cd4452f0e0d7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/b6d8753fe17d4b43ad3e125df633e63a...295fe69240dd4a2aaa31897a18bb267c/)
[Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/ad291615ee5340eb8c2d35e66a7f5cf8...4dfa98524cd84f898c5f15fa248a4565/)
[Finished :arrow_down:0.09% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/0e1c912907e541bfbec0fced54f601bd...b89d7218b21848aab559add84da36052/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] anthonylouisbsb commented on a change in pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#discussion_r774520919
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -764,6 +764,82 @@ gdv_date64 castDATE_timestamp(gdv_timestamp timestamp_in_millis) {
return tp.ClearTimeOfDay().MillisSinceEpoch();
}
+/*
+ * Input consists of mandatory and optional fields.
+ * Mandatory fields are hours, minutes.
+ * The seconds and subseconds are optional.
+ * Format is hours:minutes[:seconds.millis]
+ */
+gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length) {
+ using gandiva::TimeFields;
+ using std::chrono::hours;
+ using std::chrono::milliseconds;
+ using std::chrono::minutes;
+ using std::chrono::seconds;
+
+ int32_t time_fields[4] = {0, 0, 0, 0};
Review comment:
Applied
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rkavanap commented on a change in pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
rkavanap commented on a change in pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#discussion_r767160244
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -764,6 +764,82 @@ gdv_date64 castDATE_timestamp(gdv_timestamp timestamp_in_millis) {
return tp.ClearTimeOfDay().MillisSinceEpoch();
}
+/*
+ * Input consists of mandatory and optional fields.
+ * Mandatory fields are hours, minutes.
+ * The seconds and subseconds are optional.
+ * Format is hours:minutes[:seconds.millis]
+ */
+gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length) {
+ using gandiva::TimeFields;
+ using std::chrono::hours;
+ using std::chrono::milliseconds;
+ using std::chrono::minutes;
+ using std::chrono::seconds;
+
+ int32_t time_fields[4] = {0, 0, 0, 0};
+ int32_t sub_seconds_len = 0;
+ int32_t time_field_idx = TimeFields::kHours, index = 0, value = 0;
+ while (time_field_idx < TimeFields::kDisplacementHours && index < length) {
+ if (isdigit(input[index])) {
+ value = (value * 10) + (input[index] - '0');
+
+ if (time_field_idx == TimeFields::kSubSeconds) {
+ sub_seconds_len++;
+ }
+ } else {
+ time_fields[time_field_idx - TimeFields::kHours] = value;
+ value = 0;
+
+ switch (input[index]) {
+ case '.':
+ case ':':
+ time_field_idx++;
+ break;
+ default:
+ break;
+ }
+ }
+
+ index++;
+ }
+
+ // Check if the hours and minutes were defined and store the last value
+ if (time_field_idx < TimeFields::kDisplacementHours) {
+ time_fields[time_field_idx - TimeFields::kHours] = value;
+ }
+
+ // adjust the milliseconds
+ if (sub_seconds_len > 0) {
+ if (sub_seconds_len > 3) {
+ const char* msg = "Invalid millis for time value ";
+ set_error_for_date(length, input, msg, context);
+ return 0;
+ }
+
+ while (sub_seconds_len < 3) {
+ time_fields[TimeFields::kSubSeconds - TimeFields::kHours] *= 10;
+ sub_seconds_len++;
+ }
+ }
+
+ int32_t input_hours = time_fields[0];
Review comment:
should this be kHours instead of 0, just to be consistent
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -764,6 +764,82 @@ gdv_date64 castDATE_timestamp(gdv_timestamp timestamp_in_millis) {
return tp.ClearTimeOfDay().MillisSinceEpoch();
}
+/*
+ * Input consists of mandatory and optional fields.
+ * Mandatory fields are hours, minutes.
+ * The seconds and subseconds are optional.
+ * Format is hours:minutes[:seconds.millis]
+ */
+gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length) {
+ using gandiva::TimeFields;
+ using std::chrono::hours;
+ using std::chrono::milliseconds;
+ using std::chrono::minutes;
+ using std::chrono::seconds;
+
+ int32_t time_fields[4] = {0, 0, 0, 0};
+ int32_t sub_seconds_len = 0;
+ int32_t time_field_idx = TimeFields::kHours, index = 0, value = 0;
+ while (time_field_idx < TimeFields::kDisplacementHours && index < length) {
+ if (isdigit(input[index])) {
+ value = (value * 10) + (input[index] - '0');
+
+ if (time_field_idx == TimeFields::kSubSeconds) {
+ sub_seconds_len++;
+ }
+ } else {
+ time_fields[time_field_idx - TimeFields::kHours] = value;
+ value = 0;
+
+ switch (input[index]) {
+ case '.':
Review comment:
don't we have to be accurate?,, e.g what if someone specifies hours.minutes.seconds instead of hours:minutes:seconds..shouldn't we treat this as an error?
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -764,6 +764,82 @@ gdv_date64 castDATE_timestamp(gdv_timestamp timestamp_in_millis) {
return tp.ClearTimeOfDay().MillisSinceEpoch();
}
+/*
+ * Input consists of mandatory and optional fields.
+ * Mandatory fields are hours, minutes.
+ * The seconds and subseconds are optional.
+ * Format is hours:minutes[:seconds.millis]
+ */
+gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length) {
+ using gandiva::TimeFields;
+ using std::chrono::hours;
+ using std::chrono::milliseconds;
+ using std::chrono::minutes;
+ using std::chrono::seconds;
+
+ int32_t time_fields[4] = {0, 0, 0, 0};
Review comment:
Isn't 4 a hardcoded number..rest of places you are using constants such as kDisplacementHours etc..
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -764,6 +764,82 @@ gdv_date64 castDATE_timestamp(gdv_timestamp timestamp_in_millis) {
return tp.ClearTimeOfDay().MillisSinceEpoch();
}
+/*
+ * Input consists of mandatory and optional fields.
+ * Mandatory fields are hours, minutes.
+ * The seconds and subseconds are optional.
+ * Format is hours:minutes[:seconds.millis]
+ */
+gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length) {
+ using gandiva::TimeFields;
+ using std::chrono::hours;
+ using std::chrono::milliseconds;
+ using std::chrono::minutes;
+ using std::chrono::seconds;
+
+ int32_t time_fields[4] = {0, 0, 0, 0};
+ int32_t sub_seconds_len = 0;
+ int32_t time_field_idx = TimeFields::kHours, index = 0, value = 0;
+ while (time_field_idx < TimeFields::kDisplacementHours && index < length) {
+ if (isdigit(input[index])) {
+ value = (value * 10) + (input[index] - '0');
+
+ if (time_field_idx == TimeFields::kSubSeconds) {
+ sub_seconds_len++;
+ }
+ } else {
+ time_fields[time_field_idx - TimeFields::kHours] = value;
+ value = 0;
+
+ switch (input[index]) {
+ case '.':
+ case ':':
+ time_field_idx++;
+ break;
+ default:
Review comment:
shouldn't any other character result in an error, otherwise if I specify '11HHSB2', it may look legal, no?
##########
File path: cpp/src/gandiva/precompiled/time_test.cc
##########
@@ -132,6 +132,44 @@ TEST(TestTime, TestCastTimestamp) {
context.Reset();
}
+TEST(TestTime, TestCastTimeUtf8) {
+ ExecutionContext context;
+ auto context_ptr = reinterpret_cast<int64_t>(&context);
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30", 7), 35130000);
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30.920", 11), 35130920);
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30.1", 9),
+ castTIME_utf8(context_ptr, "9:45:30", 7) + 100);
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30.10", 10),
+ castTIME_utf8(context_ptr, "9:45:30", 7) + 100);
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30.100", 11),
+ castTIME_utf8(context_ptr, "9:45:30", 7) + 100);
+
+ // error cases
+ EXPECT_EQ(castTIME_utf8(context_ptr, "24:00:00", 8), 0);
+ EXPECT_EQ(context.get_error(), "Not a valid time value 24:00:00");
+ context.Reset();
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "00:60:00", 8), 0);
+ EXPECT_EQ(context.get_error(), "Not a valid time value 00:60:00");
+ context.Reset();
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "00:00:100", 9), 0);
+ EXPECT_EQ(context.get_error(), "Not a valid time value 00:00:100");
+ context.Reset();
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "00:00:00.0001", 13), 0);
+ EXPECT_EQ(context.get_error(), "Invalid millis for time value 00:00:00.0001");
+ context.Reset();
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "00:00:00.1000", 13), 0);
+ EXPECT_EQ(context.get_error(), "Invalid millis for time value 00:00:00.1000");
Review comment:
some error cases such as '00.00.00.765' or '0Ab12:35'
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rkavanap commented on a change in pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
rkavanap commented on a change in pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#discussion_r767160244
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -764,6 +764,82 @@ gdv_date64 castDATE_timestamp(gdv_timestamp timestamp_in_millis) {
return tp.ClearTimeOfDay().MillisSinceEpoch();
}
+/*
+ * Input consists of mandatory and optional fields.
+ * Mandatory fields are hours, minutes.
+ * The seconds and subseconds are optional.
+ * Format is hours:minutes[:seconds.millis]
+ */
+gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length) {
+ using gandiva::TimeFields;
+ using std::chrono::hours;
+ using std::chrono::milliseconds;
+ using std::chrono::minutes;
+ using std::chrono::seconds;
+
+ int32_t time_fields[4] = {0, 0, 0, 0};
+ int32_t sub_seconds_len = 0;
+ int32_t time_field_idx = TimeFields::kHours, index = 0, value = 0;
+ while (time_field_idx < TimeFields::kDisplacementHours && index < length) {
+ if (isdigit(input[index])) {
+ value = (value * 10) + (input[index] - '0');
+
+ if (time_field_idx == TimeFields::kSubSeconds) {
+ sub_seconds_len++;
+ }
+ } else {
+ time_fields[time_field_idx - TimeFields::kHours] = value;
+ value = 0;
+
+ switch (input[index]) {
+ case '.':
+ case ':':
+ time_field_idx++;
+ break;
+ default:
+ break;
+ }
+ }
+
+ index++;
+ }
+
+ // Check if the hours and minutes were defined and store the last value
+ if (time_field_idx < TimeFields::kDisplacementHours) {
+ time_fields[time_field_idx - TimeFields::kHours] = value;
+ }
+
+ // adjust the milliseconds
+ if (sub_seconds_len > 0) {
+ if (sub_seconds_len > 3) {
+ const char* msg = "Invalid millis for time value ";
+ set_error_for_date(length, input, msg, context);
+ return 0;
+ }
+
+ while (sub_seconds_len < 3) {
+ time_fields[TimeFields::kSubSeconds - TimeFields::kHours] *= 10;
+ sub_seconds_len++;
+ }
+ }
+
+ int32_t input_hours = time_fields[0];
Review comment:
should this be kHours instead of 0, just to be consistent
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -764,6 +764,82 @@ gdv_date64 castDATE_timestamp(gdv_timestamp timestamp_in_millis) {
return tp.ClearTimeOfDay().MillisSinceEpoch();
}
+/*
+ * Input consists of mandatory and optional fields.
+ * Mandatory fields are hours, minutes.
+ * The seconds and subseconds are optional.
+ * Format is hours:minutes[:seconds.millis]
+ */
+gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length) {
+ using gandiva::TimeFields;
+ using std::chrono::hours;
+ using std::chrono::milliseconds;
+ using std::chrono::minutes;
+ using std::chrono::seconds;
+
+ int32_t time_fields[4] = {0, 0, 0, 0};
+ int32_t sub_seconds_len = 0;
+ int32_t time_field_idx = TimeFields::kHours, index = 0, value = 0;
+ while (time_field_idx < TimeFields::kDisplacementHours && index < length) {
+ if (isdigit(input[index])) {
+ value = (value * 10) + (input[index] - '0');
+
+ if (time_field_idx == TimeFields::kSubSeconds) {
+ sub_seconds_len++;
+ }
+ } else {
+ time_fields[time_field_idx - TimeFields::kHours] = value;
+ value = 0;
+
+ switch (input[index]) {
+ case '.':
Review comment:
don't we have to be accurate?,, e.g what if someone specifies hours.minutes.seconds instead of hours:minutes:seconds..shouldn't we treat this as an error?
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -764,6 +764,82 @@ gdv_date64 castDATE_timestamp(gdv_timestamp timestamp_in_millis) {
return tp.ClearTimeOfDay().MillisSinceEpoch();
}
+/*
+ * Input consists of mandatory and optional fields.
+ * Mandatory fields are hours, minutes.
+ * The seconds and subseconds are optional.
+ * Format is hours:minutes[:seconds.millis]
+ */
+gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length) {
+ using gandiva::TimeFields;
+ using std::chrono::hours;
+ using std::chrono::milliseconds;
+ using std::chrono::minutes;
+ using std::chrono::seconds;
+
+ int32_t time_fields[4] = {0, 0, 0, 0};
Review comment:
Isn't 4 a hardcoded number..rest of places you are using constants such as kDisplacementHours etc..
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -764,6 +764,82 @@ gdv_date64 castDATE_timestamp(gdv_timestamp timestamp_in_millis) {
return tp.ClearTimeOfDay().MillisSinceEpoch();
}
+/*
+ * Input consists of mandatory and optional fields.
+ * Mandatory fields are hours, minutes.
+ * The seconds and subseconds are optional.
+ * Format is hours:minutes[:seconds.millis]
+ */
+gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length) {
+ using gandiva::TimeFields;
+ using std::chrono::hours;
+ using std::chrono::milliseconds;
+ using std::chrono::minutes;
+ using std::chrono::seconds;
+
+ int32_t time_fields[4] = {0, 0, 0, 0};
+ int32_t sub_seconds_len = 0;
+ int32_t time_field_idx = TimeFields::kHours, index = 0, value = 0;
+ while (time_field_idx < TimeFields::kDisplacementHours && index < length) {
+ if (isdigit(input[index])) {
+ value = (value * 10) + (input[index] - '0');
+
+ if (time_field_idx == TimeFields::kSubSeconds) {
+ sub_seconds_len++;
+ }
+ } else {
+ time_fields[time_field_idx - TimeFields::kHours] = value;
+ value = 0;
+
+ switch (input[index]) {
+ case '.':
+ case ':':
+ time_field_idx++;
+ break;
+ default:
Review comment:
shouldn't any other character result in an error, otherwise if I specify '11HHSB2', it may look legal, no?
##########
File path: cpp/src/gandiva/precompiled/time_test.cc
##########
@@ -132,6 +132,44 @@ TEST(TestTime, TestCastTimestamp) {
context.Reset();
}
+TEST(TestTime, TestCastTimeUtf8) {
+ ExecutionContext context;
+ auto context_ptr = reinterpret_cast<int64_t>(&context);
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30", 7), 35130000);
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30.920", 11), 35130920);
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30.1", 9),
+ castTIME_utf8(context_ptr, "9:45:30", 7) + 100);
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30.10", 10),
+ castTIME_utf8(context_ptr, "9:45:30", 7) + 100);
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30.100", 11),
+ castTIME_utf8(context_ptr, "9:45:30", 7) + 100);
+
+ // error cases
+ EXPECT_EQ(castTIME_utf8(context_ptr, "24:00:00", 8), 0);
+ EXPECT_EQ(context.get_error(), "Not a valid time value 24:00:00");
+ context.Reset();
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "00:60:00", 8), 0);
+ EXPECT_EQ(context.get_error(), "Not a valid time value 00:60:00");
+ context.Reset();
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "00:00:100", 9), 0);
+ EXPECT_EQ(context.get_error(), "Not a valid time value 00:00:100");
+ context.Reset();
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "00:00:00.0001", 13), 0);
+ EXPECT_EQ(context.get_error(), "Invalid millis for time value 00:00:00.0001");
+ context.Reset();
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "00:00:00.1000", 13), 0);
+ EXPECT_EQ(context.get_error(), "Invalid millis for time value 00:00:00.1000");
Review comment:
some error cases such as '00.00.00.765' or '0Ab12:35'
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#issuecomment-1019747942
Benchmark runs are scheduled for baseline = 54460d96ba1d613e472d8d9a96c072147e736b4d and contender = 8e34b64f60120bdee5991148f765cd4452f0e0d7. 8e34b64f60120bdee5991148f765cd4452f0e0d7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/b6d8753fe17d4b43ad3e125df633e63a...295fe69240dd4a2aaa31897a18bb267c/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/ad291615ee5340eb8c2d35e66a7f5cf8...4dfa98524cd84f898c5f15fa248a4565/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/0e1c912907e541bfbec0fced54f601bd...b89d7218b21848aab559add84da36052/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] anthonylouisbsb commented on a change in pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#discussion_r774531908
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -764,6 +764,82 @@ gdv_date64 castDATE_timestamp(gdv_timestamp timestamp_in_millis) {
return tp.ClearTimeOfDay().MillisSinceEpoch();
}
+/*
+ * Input consists of mandatory and optional fields.
+ * Mandatory fields are hours, minutes.
+ * The seconds and subseconds are optional.
+ * Format is hours:minutes[:seconds.millis]
+ */
+gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length) {
+ using gandiva::TimeFields;
+ using std::chrono::hours;
+ using std::chrono::milliseconds;
+ using std::chrono::minutes;
+ using std::chrono::seconds;
+
+ int32_t time_fields[4] = {0, 0, 0, 0};
+ int32_t sub_seconds_len = 0;
+ int32_t time_field_idx = TimeFields::kHours, index = 0, value = 0;
+ while (time_field_idx < TimeFields::kDisplacementHours && index < length) {
+ if (isdigit(input[index])) {
+ value = (value * 10) + (input[index] - '0');
+
+ if (time_field_idx == TimeFields::kSubSeconds) {
+ sub_seconds_len++;
+ }
+ } else {
+ time_fields[time_field_idx - TimeFields::kHours] = value;
+ value = 0;
+
+ switch (input[index]) {
+ case '.':
Review comment:
I used the already implemented **castTimestamp** function as a base and it accepts '.' characters as separators: see here: https://github.com/apache/arrow/blob/419ef49cbc5ec639bb2aefc269f127732b0ef64e/cpp/src/gandiva/precompiled/time.cc#L631
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] anthonylouisbsb commented on pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#issuecomment-983110896
@rkavanap could you review this PR for me, please?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#issuecomment-1019747942
Benchmark runs are scheduled for baseline = 54460d96ba1d613e472d8d9a96c072147e736b4d and contender = 8e34b64f60120bdee5991148f765cd4452f0e0d7. 8e34b64f60120bdee5991148f765cd4452f0e0d7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/b6d8753fe17d4b43ad3e125df633e63a...295fe69240dd4a2aaa31897a18bb267c/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/ad291615ee5340eb8c2d35e66a7f5cf8...4dfa98524cd84f898c5f15fa248a4565/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/0e1c912907e541bfbec0fced54f601bd...b89d7218b21848aab559add84da36052/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] anthonylouisbsb commented on a change in pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#discussion_r774523087
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -764,6 +764,82 @@ gdv_date64 castDATE_timestamp(gdv_timestamp timestamp_in_millis) {
return tp.ClearTimeOfDay().MillisSinceEpoch();
}
+/*
+ * Input consists of mandatory and optional fields.
+ * Mandatory fields are hours, minutes.
+ * The seconds and subseconds are optional.
+ * Format is hours:minutes[:seconds.millis]
+ */
+gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length) {
+ using gandiva::TimeFields;
+ using std::chrono::hours;
+ using std::chrono::milliseconds;
+ using std::chrono::minutes;
+ using std::chrono::seconds;
+
+ int32_t time_fields[4] = {0, 0, 0, 0};
+ int32_t sub_seconds_len = 0;
+ int32_t time_field_idx = TimeFields::kHours, index = 0, value = 0;
+ while (time_field_idx < TimeFields::kDisplacementHours && index < length) {
+ if (isdigit(input[index])) {
+ value = (value * 10) + (input[index] - '0');
+
+ if (time_field_idx == TimeFields::kSubSeconds) {
+ sub_seconds_len++;
+ }
+ } else {
+ time_fields[time_field_idx - TimeFields::kHours] = value;
+ value = 0;
+
+ switch (input[index]) {
+ case '.':
+ case ':':
+ time_field_idx++;
+ break;
+ default:
Review comment:
You are right. I added a check for it and a test.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#issuecomment-1019747942
Benchmark runs are scheduled for baseline = 54460d96ba1d613e472d8d9a96c072147e736b4d and contender = 8e34b64f60120bdee5991148f765cd4452f0e0d7. 8e34b64f60120bdee5991148f765cd4452f0e0d7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/b6d8753fe17d4b43ad3e125df633e63a...295fe69240dd4a2aaa31897a18bb267c/)
[Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/ad291615ee5340eb8c2d35e66a7f5cf8...4dfa98524cd84f898c5f15fa248a4565/)
[Finished :arrow_down:0.09% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/0e1c912907e541bfbec0fced54f601bd...b89d7218b21848aab559add84da36052/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#issuecomment-1019747942
Benchmark runs are scheduled for baseline = 54460d96ba1d613e472d8d9a96c072147e736b4d and contender = 8e34b64f60120bdee5991148f765cd4452f0e0d7. 8e34b64f60120bdee5991148f765cd4452f0e0d7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/b6d8753fe17d4b43ad3e125df633e63a...295fe69240dd4a2aaa31897a18bb267c/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/ad291615ee5340eb8c2d35e66a7f5cf8...4dfa98524cd84f898c5f15fa248a4565/)
[Finished :arrow_down:0.09% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/0e1c912907e541bfbec0fced54f601bd...b89d7218b21848aab559add84da36052/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jvictorhuguenin commented on a change in pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
jvictorhuguenin commented on a change in pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#discussion_r657079865
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -775,6 +851,17 @@ gdv_time32 castTIME_timestamp(gdv_timestamp timestamp_in_millis) {
return static_cast<int32_t>(millis_since_midnight);
}
+// Gets an arbitrary number and return the number of milliseconds since midnight
+int32_t castTIME_int32(int32_t int_val) {
Review comment:
```suggestion
gdv_time32 castTIME_int32(int32_t int_val) {
```
##########
File path: cpp/src/gandiva/precompiled/types.h
##########
@@ -262,7 +262,9 @@ gdv_timestamp castTIMESTAMP_utf8(int64_t execution_context, const char* input,
gdv_timestamp castTIMESTAMP_date64(gdv_date64);
gdv_timestamp castTIMESTAMP_int64(gdv_int64);
gdv_date64 castDATE_timestamp(gdv_timestamp);
+int32_t castTIME_utf8(int64_t context, const char* input, int32_t length);
Review comment:
```suggestion
gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length);
```
##########
File path: cpp/src/gandiva/precompiled/types.h
##########
@@ -262,7 +262,9 @@ gdv_timestamp castTIMESTAMP_utf8(int64_t execution_context, const char* input,
gdv_timestamp castTIMESTAMP_date64(gdv_date64);
gdv_timestamp castTIMESTAMP_int64(gdv_int64);
gdv_date64 castDATE_timestamp(gdv_timestamp);
+int32_t castTIME_utf8(int64_t context, const char* input, int32_t length);
gdv_time32 castTIME_timestamp(gdv_timestamp timestamp_in_millis);
+int32_t castTIME_int32(int32_t int_val);
Review comment:
```suggestion
gdv_time32 castTIME_int32(int32_t int_val);
```
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -764,6 +764,82 @@ gdv_date64 castDATE_timestamp(gdv_timestamp timestamp_in_millis) {
return tp.ClearTimeOfDay().MillisSinceEpoch();
}
+/*
+ * Input consists of mandatory and optional fields.
+ * Mandatory fields are hours, minutes.
+ * The seconds and subseconds are optional.
+ * Format is hours:minutes[:seconds.millis]
+ */
+int32_t castTIME_utf8(int64_t context, const char* input, int32_t length) {
Review comment:
```suggestion
gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length) {
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#issuecomment-848407639
https://issues.apache.org/jira/browse/ARROW-12880
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] anthonylouisbsb commented on a change in pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#discussion_r774526153
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -764,6 +764,82 @@ gdv_date64 castDATE_timestamp(gdv_timestamp timestamp_in_millis) {
return tp.ClearTimeOfDay().MillisSinceEpoch();
}
+/*
+ * Input consists of mandatory and optional fields.
+ * Mandatory fields are hours, minutes.
+ * The seconds and subseconds are optional.
+ * Format is hours:minutes[:seconds.millis]
+ */
+gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length) {
+ using gandiva::TimeFields;
+ using std::chrono::hours;
+ using std::chrono::milliseconds;
+ using std::chrono::minutes;
+ using std::chrono::seconds;
+
+ int32_t time_fields[4] = {0, 0, 0, 0};
+ int32_t sub_seconds_len = 0;
+ int32_t time_field_idx = TimeFields::kHours, index = 0, value = 0;
+ while (time_field_idx < TimeFields::kDisplacementHours && index < length) {
+ if (isdigit(input[index])) {
+ value = (value * 10) + (input[index] - '0');
+
+ if (time_field_idx == TimeFields::kSubSeconds) {
+ sub_seconds_len++;
+ }
+ } else {
+ time_fields[time_field_idx - TimeFields::kHours] = value;
+ value = 0;
+
+ switch (input[index]) {
+ case '.':
+ case ':':
+ time_field_idx++;
+ break;
+ default:
+ break;
+ }
+ }
+
+ index++;
+ }
+
+ // Check if the hours and minutes were defined and store the last value
+ if (time_field_idx < TimeFields::kDisplacementHours) {
+ time_fields[time_field_idx - TimeFields::kHours] = value;
+ }
+
+ // adjust the milliseconds
+ if (sub_seconds_len > 0) {
+ if (sub_seconds_len > 3) {
+ const char* msg = "Invalid millis for time value ";
+ set_error_for_date(length, input, msg, context);
+ return 0;
+ }
+
+ while (sub_seconds_len < 3) {
+ time_fields[TimeFields::kSubSeconds - TimeFields::kHours] *= 10;
+ sub_seconds_len++;
+ }
+ }
+
+ int32_t input_hours = time_fields[0];
Review comment:
Applied
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] anthonylouisbsb commented on a change in pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #10402:
URL: https://github.com/apache/arrow/pull/10402#discussion_r774530554
##########
File path: cpp/src/gandiva/precompiled/time_test.cc
##########
@@ -132,6 +132,44 @@ TEST(TestTime, TestCastTimestamp) {
context.Reset();
}
+TEST(TestTime, TestCastTimeUtf8) {
+ ExecutionContext context;
+ auto context_ptr = reinterpret_cast<int64_t>(&context);
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30", 7), 35130000);
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30.920", 11), 35130920);
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30.1", 9),
+ castTIME_utf8(context_ptr, "9:45:30", 7) + 100);
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30.10", 10),
+ castTIME_utf8(context_ptr, "9:45:30", 7) + 100);
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "9:45:30.100", 11),
+ castTIME_utf8(context_ptr, "9:45:30", 7) + 100);
+
+ // error cases
+ EXPECT_EQ(castTIME_utf8(context_ptr, "24:00:00", 8), 0);
+ EXPECT_EQ(context.get_error(), "Not a valid time value 24:00:00");
+ context.Reset();
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "00:60:00", 8), 0);
+ EXPECT_EQ(context.get_error(), "Not a valid time value 00:60:00");
+ context.Reset();
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "00:00:100", 9), 0);
+ EXPECT_EQ(context.get_error(), "Not a valid time value 00:00:100");
+ context.Reset();
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "00:00:00.0001", 13), 0);
+ EXPECT_EQ(context.get_error(), "Invalid millis for time value 00:00:00.0001");
+ context.Reset();
+
+ EXPECT_EQ(castTIME_utf8(context_ptr, "00:00:00.1000", 13), 0);
+ EXPECT_EQ(context.get_error(), "Invalid millis for time value 00:00:00.1000");
Review comment:
I add error for invalid character
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pravindra closed pull request #10402: ARROW-12880: [C++][Gandiva] Add castTIME(int32), castTIMESTAMP(int64) and castTIME(utf8) functions
Posted by GitBox <gi...@apache.org>.
pravindra closed pull request #10402:
URL: https://github.com/apache/arrow/pull/10402
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org