You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "caicancai (via GitHub)" <gi...@apache.org> on 2025/01/02 14:56:09 UTC
[PR] [CALCITE-6760] Support time data type in Arrow adapter [calcite]
caicancai opened a new pull request, #4120:
URL: https://github.com/apache/calcite/pull/4120
https://issues.apache.org/jira/browse/CALCITE-6760
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6760] Support time data type in Arrow adapter [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4120:
URL: https://github.com/apache/calcite/pull/4120#discussion_r1901430486
##########
arrow/src/test/java/org/apache/calcite/adapter/arrow/ArrowDataTest.java:
##########
@@ -411,4 +420,14 @@ private void dateField(FieldVector fieldVector, int rowCount) {
}
fieldVector.setValueCount(rowCount);
}
+
+ private void timeField(FieldVector fieldVector, int rowCount) {
+ TimeSecVector timeVector = (TimeSecVector) fieldVector;
+ timeVector.setInitialCapacity(rowCount);
+ timeVector.allocateNew();
+ for (int i = 0; i < rowCount; i++) {
+ timeVector.set(i, i * 1000);
Review Comment:
I am adapting and discussing with the Arrow community. Currently, the Arrow community only has the SQL standard for timestamp, but no SQL standard for time type.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6760] Support time data type in Arrow adapter [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4120:
URL: https://github.com/apache/calcite/pull/4120#issuecomment-2567943204
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4120) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4120&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4120&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4120&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4120&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4120&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4120)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6760] Support time data type in Arrow adapter [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4120:
URL: https://github.com/apache/calcite/pull/4120#discussion_r1901430486
##########
arrow/src/test/java/org/apache/calcite/adapter/arrow/ArrowDataTest.java:
##########
@@ -411,4 +420,14 @@ private void dateField(FieldVector fieldVector, int rowCount) {
}
fieldVector.setValueCount(rowCount);
}
+
+ private void timeField(FieldVector fieldVector, int rowCount) {
+ TimeSecVector timeVector = (TimeSecVector) fieldVector;
+ timeVector.setInitialCapacity(rowCount);
+ timeVector.allocateNew();
+ for (int i = 0; i < rowCount; i++) {
+ timeVector.set(i, i * 1000);
Review Comment:
I am adapting and discussing with the Arrow community. Currently, the Arrow community only has the SQL standard for timestamp, but no SQL standard for time type.
I will fix this problem over the weekend
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6760] Support time data type in Arrow adapter [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4120:
URL: https://github.com/apache/calcite/pull/4120#discussion_r1903096850
##########
arrow/src/test/java/org/apache/calcite/adapter/arrow/ArrowDataTest.java:
##########
@@ -411,4 +420,14 @@ private void dateField(FieldVector fieldVector, int rowCount) {
}
fieldVector.setValueCount(rowCount);
}
+
+ private void timeField(FieldVector fieldVector, int rowCount) {
+ TimeSecVector timeVector = (TimeSecVector) fieldVector;
+ timeVector.setInitialCapacity(rowCount);
+ timeVector.allocateNew();
+ for (int i = 0; i < rowCount; i++) {
+ timeVector.set(i, i * 1000);
Review Comment:
@ILuffZhe After testing, no matter which timevector is chosen, it is in milliseconds.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6760] Support time data type in Arrow adapter [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4120:
URL: https://github.com/apache/calcite/pull/4120#discussion_r1901430486
##########
arrow/src/test/java/org/apache/calcite/adapter/arrow/ArrowDataTest.java:
##########
@@ -411,4 +420,14 @@ private void dateField(FieldVector fieldVector, int rowCount) {
}
fieldVector.setValueCount(rowCount);
}
+
+ private void timeField(FieldVector fieldVector, int rowCount) {
+ TimeSecVector timeVector = (TimeSecVector) fieldVector;
+ timeVector.setInitialCapacity(rowCount);
+ timeVector.allocateNew();
+ for (int i = 0; i < rowCount; i++) {
+ timeVector.set(i, i * 1000);
Review Comment:
I am adapting and discussing with the Arrow community. Currently, the Arrow community only has the SQL standard for timestamp, but no SQL standard for time type.
I will fix this problem over the weekend
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6760] Support time data type in Arrow adapter [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4120:
URL: https://github.com/apache/calcite/pull/4120#discussion_r1901427017
##########
arrow/src/test/java/org/apache/calcite/adapter/arrow/ArrowDataTest.java:
##########
@@ -411,4 +420,14 @@ private void dateField(FieldVector fieldVector, int rowCount) {
}
fieldVector.setValueCount(rowCount);
}
+
+ private void timeField(FieldVector fieldVector, int rowCount) {
+ TimeSecVector timeVector = (TimeSecVector) fieldVector;
+ timeVector.setInitialCapacity(rowCount);
+ timeVector.allocateNew();
+ for (int i = 0; i < rowCount; i++) {
+ timeVector.set(i, i * 1000);
Review Comment:
It depends on arrow-java's adaptation of the time type. Like most Java libraries, the basic unit is milliseconds, so 1000ms == 1s
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6760] Support time data type in Arrow adapter [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on code in PR #4120:
URL: https://github.com/apache/calcite/pull/4120#discussion_r1901428834
##########
arrow/src/test/java/org/apache/calcite/adapter/arrow/ArrowDataTest.java:
##########
@@ -411,4 +420,14 @@ private void dateField(FieldVector fieldVector, int rowCount) {
}
fieldVector.setValueCount(rowCount);
}
+
+ private void timeField(FieldVector fieldVector, int rowCount) {
+ TimeSecVector timeVector = (TimeSecVector) fieldVector;
+ timeVector.setInitialCapacity(rowCount);
+ timeVector.allocateNew();
+ for (int i = 0; i < rowCount; i++) {
+ timeVector.set(i, i * 1000);
Review Comment:
You mean there exist unit that is not milliseconds? How do we handle this.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6760] Support time data type in Arrow adapter [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4120:
URL: https://github.com/apache/calcite/pull/4120#discussion_r1903096850
##########
arrow/src/test/java/org/apache/calcite/adapter/arrow/ArrowDataTest.java:
##########
@@ -411,4 +420,14 @@ private void dateField(FieldVector fieldVector, int rowCount) {
}
fieldVector.setValueCount(rowCount);
}
+
+ private void timeField(FieldVector fieldVector, int rowCount) {
+ TimeSecVector timeVector = (TimeSecVector) fieldVector;
+ timeVector.setInitialCapacity(rowCount);
+ timeVector.allocateNew();
+ for (int i = 0; i < rowCount; i++) {
+ timeVector.set(i, i * 1000);
Review Comment:
@ILuffZhe After testing, milliseconds are the basic unit of the time type in arrow
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6760] Support time data type in Arrow adapter [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #4120:
URL: https://github.com/apache/calcite/pull/4120#issuecomment-2567970555
I encountered some problems adapting the timestmap type, so this pr first adapts to the time type
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6760] Support time data type in Arrow adapter [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on code in PR #4120:
URL: https://github.com/apache/calcite/pull/4120#discussion_r1901393893
##########
arrow/src/test/java/org/apache/calcite/adapter/arrow/ArrowDataTest.java:
##########
@@ -411,4 +420,14 @@ private void dateField(FieldVector fieldVector, int rowCount) {
}
fieldVector.setValueCount(rowCount);
}
+
+ private void timeField(FieldVector fieldVector, int rowCount) {
+ TimeSecVector timeVector = (TimeSecVector) fieldVector;
+ timeVector.setInitialCapacity(rowCount);
+ timeVector.allocateNew();
+ for (int i = 0; i < rowCount; i++) {
+ timeVector.set(i, i * 1000);
Review Comment:
Why multiply `i` by 1000?
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6760] Support time data type in Arrow adapter [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe merged PR #4120:
URL: https://github.com/apache/calcite/pull/4120
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6760] Support time data type in Arrow adapter [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on code in PR #4120:
URL: https://github.com/apache/calcite/pull/4120#discussion_r1903097714
##########
arrow/src/test/java/org/apache/calcite/adapter/arrow/ArrowDataTest.java:
##########
@@ -411,4 +420,14 @@ private void dateField(FieldVector fieldVector, int rowCount) {
}
fieldVector.setValueCount(rowCount);
}
+
+ private void timeField(FieldVector fieldVector, int rowCount) {
+ TimeSecVector timeVector = (TimeSecVector) fieldVector;
+ timeVector.setInitialCapacity(rowCount);
+ timeVector.allocateNew();
+ for (int i = 0; i < rowCount; i++) {
+ timeVector.set(i, i * 1000);
Review Comment:
Thanks for the clarify.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org