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/04/14 19:18:36 UTC
[GitHub] [arrow] frank400 opened a new pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
frank400 opened a new pull request #10033:
URL: https://github.com/apache/arrow/pull/10033
--
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] projjal commented on pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#issuecomment-855818299
> @projjal can you please confirm the errors on the builds is not related.
Build failures are unrelated
--
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] praveenbingo closed pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
praveenbingo closed pull request #10033:
URL: https://github.com/apache/arrow/pull/10033
--
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] kou commented on pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#issuecomment-893779610
Thanks!
--
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] kou commented on pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#issuecomment-892953607
@praveenbingo Could you use the latest `dev/merge_arrow_pr.py`? The latest `dev/merge_arrow_pr.py` doesn't list squashed commits.
--
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] github-actions[bot] commented on pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#issuecomment-819893948
https://issues.apache.org/jira/browse/ARROW-12388
--
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] jvictorhuguenin commented on a change in pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
jvictorhuguenin commented on a change in pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#discussion_r631933396
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -300,12 +299,17 @@ char* gdv_fn_dec_to_string(int64_t context, int64_t x_high, uint64_t x_low,
return val; \
}
-CAST_NUMERIC_FROM_STRING(int32_t, arrow::Int32Type, INT)
-CAST_NUMERIC_FROM_STRING(int64_t, arrow::Int64Type, BIGINT)
-CAST_NUMERIC_FROM_STRING(float, arrow::FloatType, FLOAT4)
-CAST_NUMERIC_FROM_STRING(double, arrow::DoubleType, FLOAT8)
+CAST_NUMERIC_STRING(int32_t, arrow::Int32Type, INT, utf8)
Review comment:
done
##########
File path: cpp/src/gandiva/gdv_function_stubs_test.cc
##########
@@ -290,4 +290,140 @@ TEST(TestGdvFnStubs, TestCastVARCHARFromDouble) {
EXPECT_FALSE(ctx.has_error());
}
+TEST(TestGdvFnStubs, TestCastVarbinaryINT) {
+ gandiva::ExecutionContext ctx;
+
+ int64_t ctx_ptr = reinterpret_cast<int64_t>(&ctx);
+
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, "-45", 3), -45);
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, "0", 1), 0);
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, "2147483647", 10), 2147483647);
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, "02147483647", 11), 2147483647);
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, "-2147483648", 11), -2147483648LL);
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, "-02147483648", 12), -2147483648LL);
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, " 12 ", 4), 12);
+
+ gdv_fn_castINT_varbinary(ctx_ptr, "2147483648", 10);
+ EXPECT_THAT(ctx.get_error(),
+ ::testing::HasSubstr("Failed to cast the string 2147483648 to int32"));
+ ctx.Reset();
+
+ gdv_fn_castINT_varbinary(ctx_ptr, "-2147483649", 11);
+ EXPECT_THAT(ctx.get_error(),
+ ::testing::HasSubstr("Failed to cast the string -2147483649 to int32"));
+ ctx.Reset();
+
+ gdv_fn_castINT_varbinary(ctx_ptr, "12.34", 5);
+ EXPECT_THAT(ctx.get_error(),
+ ::testing::HasSubstr("Failed to cast the string 12.34 to int32"));
+ ctx.Reset();
+
+ gdv_fn_castINT_varbinary(ctx_ptr, "abc", 3);
+ EXPECT_THAT(ctx.get_error(),
+ ::testing::HasSubstr("Failed to cast the string abc to int32"));
+ ctx.Reset();
+
+ gdv_fn_castINT_varbinary(ctx_ptr, "", 0);
+ EXPECT_THAT(ctx.get_error(),
+ ::testing::HasSubstr("Failed to cast the string to int32"));
+ ctx.Reset();
+
+ gdv_fn_castINT_varbinary(ctx_ptr, "-", 1);
+ EXPECT_THAT(ctx.get_error(),
+ ::testing::HasSubstr("Failed to cast the string - to int32"));
+ ctx.Reset();
+}
+
+TEST(TestGdvFnStubs, TestCastVarbinaryBIGINT) {
+ gandiva::ExecutionContext ctx;
+
+ int64_t ctx_ptr = reinterpret_cast<int64_t>(&ctx);
+
+ EXPECT_EQ(gdv_fn_castBIGINT_varbinary(ctx_ptr, "-45", 3), -45);
+ EXPECT_EQ(gdv_fn_castBIGINT_varbinary(ctx_ptr, "0", 1), 0);
+ EXPECT_EQ(gdv_fn_castBIGINT_varbinary(ctx_ptr, "9223372036854775807", 19),
Review comment:
done
--
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] jvictorhuguenin closed pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
jvictorhuguenin closed pull request #10033:
URL: https://github.com/apache/arrow/pull/10033
--
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] projjal commented on a change in pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#discussion_r623628042
##########
File path: cpp/src/gandiva/gdv_function_stubs_test.cc
##########
@@ -290,4 +290,140 @@ TEST(TestGdvFnStubs, TestCastVARCHARFromDouble) {
EXPECT_FALSE(ctx.has_error());
}
+TEST(TestGdvFnStubs, TestCastVarbinaryINT) {
+ gandiva::ExecutionContext ctx;
+
+ int64_t ctx_ptr = reinterpret_cast<int64_t>(&ctx);
+
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, "-45", 3), -45);
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, "0", 1), 0);
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, "2147483647", 10), 2147483647);
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, "02147483647", 11), 2147483647);
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, "-2147483648", 11), -2147483648LL);
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, "-02147483648", 12), -2147483648LL);
+ EXPECT_EQ(gdv_fn_castINT_varbinary(ctx_ptr, " 12 ", 4), 12);
+
+ gdv_fn_castINT_varbinary(ctx_ptr, "2147483648", 10);
+ EXPECT_THAT(ctx.get_error(),
+ ::testing::HasSubstr("Failed to cast the string 2147483648 to int32"));
+ ctx.Reset();
+
+ gdv_fn_castINT_varbinary(ctx_ptr, "-2147483649", 11);
+ EXPECT_THAT(ctx.get_error(),
+ ::testing::HasSubstr("Failed to cast the string -2147483649 to int32"));
+ ctx.Reset();
+
+ gdv_fn_castINT_varbinary(ctx_ptr, "12.34", 5);
+ EXPECT_THAT(ctx.get_error(),
+ ::testing::HasSubstr("Failed to cast the string 12.34 to int32"));
+ ctx.Reset();
+
+ gdv_fn_castINT_varbinary(ctx_ptr, "abc", 3);
+ EXPECT_THAT(ctx.get_error(),
+ ::testing::HasSubstr("Failed to cast the string abc to int32"));
+ ctx.Reset();
+
+ gdv_fn_castINT_varbinary(ctx_ptr, "", 0);
+ EXPECT_THAT(ctx.get_error(),
+ ::testing::HasSubstr("Failed to cast the string to int32"));
+ ctx.Reset();
+
+ gdv_fn_castINT_varbinary(ctx_ptr, "-", 1);
+ EXPECT_THAT(ctx.get_error(),
+ ::testing::HasSubstr("Failed to cast the string - to int32"));
+ ctx.Reset();
+}
+
+TEST(TestGdvFnStubs, TestCastVarbinaryBIGINT) {
+ gandiva::ExecutionContext ctx;
+
+ int64_t ctx_ptr = reinterpret_cast<int64_t>(&ctx);
+
+ EXPECT_EQ(gdv_fn_castBIGINT_varbinary(ctx_ptr, "-45", 3), -45);
+ EXPECT_EQ(gdv_fn_castBIGINT_varbinary(ctx_ptr, "0", 1), 0);
+ EXPECT_EQ(gdv_fn_castBIGINT_varbinary(ctx_ptr, "9223372036854775807", 19),
Review comment:
Can you add some tests with hex string containing valid numbers
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -300,12 +299,17 @@ char* gdv_fn_dec_to_string(int64_t context, int64_t x_high, uint64_t x_low,
return val; \
}
-CAST_NUMERIC_FROM_STRING(int32_t, arrow::Int32Type, INT)
-CAST_NUMERIC_FROM_STRING(int64_t, arrow::Int64Type, BIGINT)
-CAST_NUMERIC_FROM_STRING(float, arrow::FloatType, FLOAT4)
-CAST_NUMERIC_FROM_STRING(double, arrow::DoubleType, FLOAT8)
+CAST_NUMERIC_STRING(int32_t, arrow::Int32Type, INT, utf8)
Review comment:
Can you change the macros to CAST_NUMERIC_FROM_VARLEN_TYPES, another two macros CAST_NUMERIC_FROM_STRING and CAST_NUMERIC_FROM_BINARY which calls the above macro with utf8 or binary. Call these two macros with numberic types
--
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] projjal commented on pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#issuecomment-828937350
Can you look at the failing tests
--
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] projjal commented on a change in pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#discussion_r617519065
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -307,6 +306,37 @@ CAST_NUMERIC_FROM_STRING(double, arrow::DoubleType, FLOAT8)
#undef CAST_NUMERIC_FROM_STRING
+#define CAST_NUMERIC_FROM_VARBINARY(OUT_TYPE, ARROW_TYPE, TYPE_NAME) \
+ GANDIVA_EXPORT \
+ OUT_TYPE gdv_fn_cast##TYPE_NAME##_varbinary(int64_t context, const char* data, \
Review comment:
Use a macro to use the same definition for both string and varbinary inputs
--
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] kou commented on pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#issuecomment-892953607
@praveenbingo Could you use the latest `dev/merge_arrow_pr.py`? The latest `dev/merge_arrow_pr.py` doesn't list squashed commits.
--
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] praveenbingo commented on pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
praveenbingo commented on pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#issuecomment-869181283
@projjal Needs a rebase
--
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] praveenbingo commented on pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
praveenbingo commented on pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#issuecomment-855809523
@projjal can you please confirm the errors on the builds is not related.
--
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] projjal commented on pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#issuecomment-881266263
@jvictorhuguenin Can you rebase. There is merge conflict.
--
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] praveenbingo closed pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
praveenbingo closed pull request #10033:
URL: https://github.com/apache/arrow/pull/10033
--
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] praveenbingo commented on pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
praveenbingo commented on pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#issuecomment-893390301
@kou Will pull the latest in. Thanks - sorry if my merge caused any trouble.
--
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] projjal commented on a change in pull request #10033: ARROW-12388: [C++][Gandiva] Implement cast numbers from varbinary functions in gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10033:
URL: https://github.com/apache/arrow/pull/10033#discussion_r617511000
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -307,6 +306,37 @@ CAST_NUMERIC_FROM_STRING(double, arrow::DoubleType, FLOAT8)
#undef CAST_NUMERIC_FROM_STRING
+#define CAST_NUMERIC_FROM_VARBINARY(OUT_TYPE, ARROW_TYPE, TYPE_NAME) \
+ GANDIVA_EXPORT \
+ OUT_TYPE gdv_fn_cast##TYPE_NAME##_varbinary(int64_t context, const char* data, \
Review comment:
Is it any different from CAST_NUMERIC_FROM_STRING functions. Can't you just call them
--
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