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 2022/11/21 21:47:25 UTC

[GitHub] [arrow-datafusion] waitingkuo commented on a diff in pull request #4312: Support type coercion for timestamp and utf8

waitingkuo commented on code in PR #4312:
URL: https://github.com/apache/arrow-datafusion/pull/4312#discussion_r1028536829


##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -830,10 +844,12 @@ message ScalarValue{
         ScalarDictionaryValue dictionary_value = 27;
         bytes binary_value = 28;
         bytes large_binary_value = 29;
+
         ScalarTime64Value time64_value = 30;
         IntervalMonthDayNanoValue interval_month_day_nano = 31;
         StructValue struct_value = 32;
         ScalarFixedSizeBinary fixed_size_binary_value = 34;
+>>>>>>> upstream/master

Review Comment:
   ```suggestion
   ```



##########
datafusion/core/tests/sql/select.rs:
##########
@@ -814,6 +814,8 @@ async fn query_on_string_dictionary() -> Result<()> {
     ];
     assert_batches_eq!(expected, &actual);
 
+    // filtering with Time32 and Time64 types
+

Review Comment:
   not quite sure what it is, i guess we need to delete ths



##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -770,6 +770,20 @@ message ScalarTimestampValue {
   string timezone = 5;
 }
 
+message ScalarTime32Value {
+  oneof value {
+    int32 time32_second_value = 1;
+    int32 time32_millisecond_value = 2;
+  };
+}
+
+message ScalarTime64Value {
+  oneof value {
+    int64 time64_microsecond_value = 1;
+    int64 time64_nanosecond_value = 2;
+  };
+}
+

Review Comment:
   i think you should rebase master, the pr for time32/64 is merged



##########
datafusion/core/tests/sql/timestamp.rs:
##########
@@ -1555,7 +1555,7 @@ async fn cast_timestamp_to_timestamptz() -> Result<()> {
 #[tokio::test]
 async fn test_cast_to_time() -> Result<()> {
     let ctx = SessionContext::new();
-    let sql = "SELECT 0::TIME";
+    let sql = "SELECT 0::TIME64";

Review Comment:
   i'm not sure why do we have this. but `select 0::time64` doesn't work for me



-- 
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