You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2022/03/30 11:00:36 UTC
[GitHub] [iotdb] jiazhiren opened a new pull request #5382: [IOTDB-2650] Tablet supports adding String value
jiazhiren opened a new pull request #5382:
URL: https://github.com/apache/iotdb/pull/5382
This PR creates syntactic sugar by supporting String value insertion to Tablet. #[IOTDB-2650]
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] HTHou commented on pull request #5382: [IOTDB-2650] Tablet supports adding String value
Posted by GitBox <gi...@apache.org>.
HTHou commented on pull request #5382:
URL: https://github.com/apache/iotdb/pull/5382#issuecomment-1086463408
Hi, you can execute `mvn spotless:apply` to fix the test issue.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] HTHou commented on a change in pull request #5382: [IOTDB-2650] Tablet supports adding String value
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #5382:
URL: https://github.com/apache/iotdb/pull/5382#discussion_r838445154
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/write/record/Tablet.java
##########
@@ -138,7 +138,7 @@ private void addValueOfDataType(
case TEXT:
{
Binary[] sensor = (Binary[]) values[indexOfSchema];
- sensor[rowIndex] = value != null ? (Binary) value : Binary.EMPTY_VALUE;
+ sensor[rowIndex] = value != null ? new Binary((String) value) : Binary.EMPTY_VALUE;
Review comment:
Thanks for your contribution!
IMO, we should also support adding a Binary value here to keep the compatibility. :)
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] HTHou merged pull request #5382: [IOTDB-2650] Tablet supports adding String value
Posted by GitBox <gi...@apache.org>.
HTHou merged pull request #5382:
URL: https://github.com/apache/iotdb/pull/5382
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] HTHou commented on a change in pull request #5382: [IOTDB-2650] Tablet supports adding String value
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #5382:
URL: https://github.com/apache/iotdb/pull/5382#discussion_r838449157
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/write/record/Tablet.java
##########
@@ -138,7 +138,7 @@ private void addValueOfDataType(
case TEXT:
{
Binary[] sensor = (Binary[]) values[indexOfSchema];
- sensor[rowIndex] = value != null ? (Binary) value : Binary.EMPTY_VALUE;
+ sensor[rowIndex] = value != null ? new Binary((String) value) : Binary.EMPTY_VALUE;
Review comment:
Besides, consider adding some tests in `IoTDBSessionSimpleIT`?
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jiazhiren commented on a change in pull request #5382: [IOTDB-2650] Tablet supports adding String value
Posted by GitBox <gi...@apache.org>.
jiazhiren commented on a change in pull request #5382:
URL: https://github.com/apache/iotdb/pull/5382#discussion_r840160408
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/write/record/Tablet.java
##########
@@ -138,7 +138,7 @@ private void addValueOfDataType(
case TEXT:
{
Binary[] sensor = (Binary[]) values[indexOfSchema];
- sensor[rowIndex] = value != null ? (Binary) value : Binary.EMPTY_VALUE;
+ sensor[rowIndex] = value != null ? new Binary((String) value) : Binary.EMPTY_VALUE;
Review comment:
Thanks for your valuable advice! I will work on them immediately.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jiazhiren commented on pull request #5382: [IOTDB-2650] Tablet supports adding String value
Posted by GitBox <gi...@apache.org>.
jiazhiren commented on pull request #5382:
URL: https://github.com/apache/iotdb/pull/5382#issuecomment-1086480479
>
Thanks for your tip. The code format has been corrected.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org