You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by GitBox <gi...@apache.org> on 2021/04/13 15:48:25 UTC

[GitHub] [orc] coderex2522 opened a new pull request #683: ORC-784:[C++] Read/Write timestamp column with the specified timezone option

coderex2522 opened a new pull request #683:
URL: https://github.com/apache/orc/pull/683


   ### How was this patch tested?
   this patch is tested by the ut of writeTimestampWithTimezone.
   


-- 
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] [orc] wgtmac commented on a change in pull request #683: ORC-784:[C++] Read/Write timestamp column with the specified timezone option

Posted by GitBox <gi...@apache.org>.
wgtmac commented on a change in pull request #683:
URL: https://github.com/apache/orc/pull/683#discussion_r614517380



##########
File path: c++/src/Writer.cc
##########
@@ -56,6 +57,7 @@ namespace orc {
       enableIndex = true;
       bloomFilterFalsePositiveProb = 0.05;
       bloomFilterVersion = UTF8;
+      timezone = "GMT";

Review comment:
       The Java ORC writer uses local time zone by default. However, the C++ ORC writer uses GMT by default in this [fix](https://github.com/apache/orc/commit/8ea3592f6aeddb5e6930798e4ff95065c6177e00). I hard-coded it to GMT at that time to work around the insufficient support of timezone conversion on the C++ side. I'd suggest to leave some comments to explain this history.

##########
File path: c++/test/TestWriter.cc
##########
@@ -699,6 +704,98 @@ namespace orc {
     }
   }
 
+#ifndef _MSC_VER

Review comment:
       leave some comments to explain why you skip windows here.




-- 
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] [orc] coderex2522 commented on pull request #683: ORC-784:[C++] Read/Write timestamp column with the specified timezone option

Posted by GitBox <gi...@apache.org>.
coderex2522 commented on pull request #683:
URL: https://github.com/apache/orc/pull/683#issuecomment-818866913


   Hey @wgtmac, the UT of this patch don't support on windows system. There are several problems here 
   1) Windows don't support acquiring the structure timezone.
   2) The timezones of Windows cannot match exactly linux system. 
   3) the function calls about timezone in Windows need to acquire process token. 


-- 
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] [orc] wgtmac merged pull request #683: ORC-784:[C++] Read/Write timestamp column with the specified timezone option

Posted by GitBox <gi...@apache.org>.
wgtmac merged pull request #683:
URL: https://github.com/apache/orc/pull/683


   


-- 
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] [orc] coderex2522 commented on a change in pull request #683: ORC-784:[C++] Read/Write timestamp column with the specified timezone option

Posted by GitBox <gi...@apache.org>.
coderex2522 commented on a change in pull request #683:
URL: https://github.com/apache/orc/pull/683#discussion_r615201392



##########
File path: c++/src/Writer.cc
##########
@@ -56,6 +57,7 @@ namespace orc {
       enableIndex = true;
       bloomFilterFalsePositiveProb = 0.05;
       bloomFilterVersion = UTF8;
+      timezone = "GMT";

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] [orc] coderex2522 commented on a change in pull request #683: ORC-784:[C++] Read/Write timestamp column with the specified timezone option

Posted by GitBox <gi...@apache.org>.
coderex2522 commented on a change in pull request #683:
URL: https://github.com/apache/orc/pull/683#discussion_r615201850



##########
File path: c++/test/TestWriter.cc
##########
@@ -699,6 +704,98 @@ namespace orc {
     }
   }
 
+#ifndef _MSC_VER

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