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 2020/10/02 11:12:00 UTC
[GitHub] [iotdb] jack870131 opened a new pull request #1791: [IOTDB-912] Implement wasNull method in AbstractIoTDBJDBCResultSet
jack870131 opened a new pull request #1791:
URL: https://github.com/apache/iotdb/pull/1791
The wasNull method had been implemented and getXXX method should be modified according to the JDBC specification while there is null.
----------------------------------------------------------------
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] [iotdb] jack870131 commented on a change in pull request #1791: [IOTDB-912] Implement wasNull method in AbstractIoTDBJDBCResultSet
Posted by GitBox <gi...@apache.org>.
jack870131 commented on a change in pull request #1791:
URL: https://github.com/apache/iotdb/pull/1791#discussion_r500106698
##########
File path: service-rpc/src/main/java/org/apache/iotdb/rpc/IoTDBRpcDataSet.java
##########
@@ -312,9 +317,23 @@ public float getFloat(String columnName) throws StatementExecutionException {
checkRecord();
int index = columnOrdinalMap.get(columnName) - START_INDEX;
if (!isNull(index, rowsIndex - 1)) {
+ lastReadWasNull = false;
return BytesUtils.bytesToFloat(values[index]);
} else {
- throw new StatementExecutionException(String.format(VALUE_IS_NULL, columnName));
+ lastReadWasNull = true;
+ return 0;
+ }
+ }
+
+ public short getShort(String columnName) throws StatementExecutionException {
+ checkRecord();
Review comment:
I think so, too. I felt weird that why it was implemented. How about just throwing exception while it has been used?
----------------------------------------------------------------
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] [iotdb] HTHou commented on a change in pull request #1791: [IOTDB-912] Implement wasNull method in AbstractIoTDBJDBCResultSet
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #1791:
URL: https://github.com/apache/iotdb/pull/1791#discussion_r499988152
##########
File path: service-rpc/src/main/java/org/apache/iotdb/rpc/IoTDBRpcDataSet.java
##########
@@ -312,9 +317,23 @@ public float getFloat(String columnName) throws StatementExecutionException {
checkRecord();
int index = columnOrdinalMap.get(columnName) - START_INDEX;
if (!isNull(index, rowsIndex - 1)) {
+ lastReadWasNull = false;
return BytesUtils.bytesToFloat(values[index]);
} else {
- throw new StatementExecutionException(String.format(VALUE_IS_NULL, columnName));
+ lastReadWasNull = true;
+ return 0;
+ }
+ }
+
+ public short getShort(String columnName) throws StatementExecutionException {
+ checkRecord();
Review comment:
Actually, we are not supporting short type of data yet...
----------------------------------------------------------------
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] [iotdb] qiaojialin commented on a change in pull request #1791: [IOTDB-912] Implement wasNull method in AbstractIoTDBJDBCResultSet
Posted by GitBox <gi...@apache.org>.
qiaojialin commented on a change in pull request #1791:
URL: https://github.com/apache/iotdb/pull/1791#discussion_r500833080
##########
File path: service-rpc/src/main/java/org/apache/iotdb/rpc/IoTDBRpcDataSet.java
##########
@@ -343,9 +352,11 @@ public long getLong(String columnName) throws StatementExecutionException {
}
int index = columnOrdinalMap.get(columnName) - START_INDEX;
if (!isNull(index, rowsIndex - 1)) {
+ lastReadWasNull = false;
return BytesUtils.bytesToLong(values[index]);
} else {
- throw new StatementExecutionException(String.format(VALUE_IS_NULL, columnName));
Review comment:
Maybe throw an exception is better? which can help users find bugs in client
----------------------------------------------------------------
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] [iotdb] qiaojialin commented on a change in pull request #1791: [IOTDB-912] Implement wasNull method in AbstractIoTDBJDBCResultSet
Posted by GitBox <gi...@apache.org>.
qiaojialin commented on a change in pull request #1791:
URL: https://github.com/apache/iotdb/pull/1791#discussion_r509258525
##########
File path: service-rpc/src/main/java/org/apache/iotdb/rpc/IoTDBRpcDataSet.java
##########
@@ -343,9 +352,11 @@ public long getLong(String columnName) throws StatementExecutionException {
}
int index = columnOrdinalMap.get(columnName) - START_INDEX;
if (!isNull(index, rowsIndex - 1)) {
+ lastReadWasNull = false;
return BytesUtils.bytesToLong(values[index]);
} else {
- throw new StatementExecutionException(String.format(VALUE_IS_NULL, columnName));
Review comment:
ok 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] HTHou commented on a change in pull request #1791: [IOTDB-912] Implement wasNull method in AbstractIoTDBJDBCResultSet
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #1791:
URL: https://github.com/apache/iotdb/pull/1791#discussion_r499988152
##########
File path: service-rpc/src/main/java/org/apache/iotdb/rpc/IoTDBRpcDataSet.java
##########
@@ -312,9 +317,23 @@ public float getFloat(String columnName) throws StatementExecutionException {
checkRecord();
int index = columnOrdinalMap.get(columnName) - START_INDEX;
if (!isNull(index, rowsIndex - 1)) {
+ lastReadWasNull = false;
return BytesUtils.bytesToFloat(values[index]);
} else {
- throw new StatementExecutionException(String.format(VALUE_IS_NULL, columnName));
+ lastReadWasNull = true;
+ return 0;
+ }
+ }
+
+ public short getShort(String columnName) throws StatementExecutionException {
+ checkRecord();
Review comment:
Actually, we don't support short type of data yet...
----------------------------------------------------------------
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] [iotdb] jack870131 commented on a change in pull request #1791: [IOTDB-912] Implement wasNull method in AbstractIoTDBJDBCResultSet
Posted by GitBox <gi...@apache.org>.
jack870131 commented on a change in pull request #1791:
URL: https://github.com/apache/iotdb/pull/1791#discussion_r500106698
##########
File path: service-rpc/src/main/java/org/apache/iotdb/rpc/IoTDBRpcDataSet.java
##########
@@ -312,9 +317,23 @@ public float getFloat(String columnName) throws StatementExecutionException {
checkRecord();
int index = columnOrdinalMap.get(columnName) - START_INDEX;
if (!isNull(index, rowsIndex - 1)) {
+ lastReadWasNull = false;
return BytesUtils.bytesToFloat(values[index]);
} else {
- throw new StatementExecutionException(String.format(VALUE_IS_NULL, columnName));
+ lastReadWasNull = true;
+ return 0;
+ }
+ }
+
+ public short getShort(String columnName) throws StatementExecutionException {
+ checkRecord();
Review comment:
I think so, too. I felt weird that why it was implemented before. How about just throwing exception while it has been used?
----------------------------------------------------------------
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] [iotdb] HTHou commented on a change in pull request #1791: [IOTDB-912] Implement wasNull method in AbstractIoTDBJDBCResultSet
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #1791:
URL: https://github.com/apache/iotdb/pull/1791#discussion_r500133158
##########
File path: service-rpc/src/main/java/org/apache/iotdb/rpc/IoTDBRpcDataSet.java
##########
@@ -312,9 +317,23 @@ public float getFloat(String columnName) throws StatementExecutionException {
checkRecord();
int index = columnOrdinalMap.get(columnName) - START_INDEX;
if (!isNull(index, rowsIndex - 1)) {
+ lastReadWasNull = false;
return BytesUtils.bytesToFloat(values[index]);
} else {
- throw new StatementExecutionException(String.format(VALUE_IS_NULL, columnName));
+ lastReadWasNull = true;
+ return 0;
+ }
+ }
+
+ public short getShort(String columnName) throws StatementExecutionException {
+ checkRecord();
Review comment:
Yes, look good to 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jack870131 commented on a change in pull request #1791: [IOTDB-912] Implement wasNull method in AbstractIoTDBJDBCResultSet
Posted by GitBox <gi...@apache.org>.
jack870131 commented on a change in pull request #1791:
URL: https://github.com/apache/iotdb/pull/1791#discussion_r502801510
##########
File path: service-rpc/src/main/java/org/apache/iotdb/rpc/IoTDBRpcDataSet.java
##########
@@ -343,9 +352,11 @@ public long getLong(String columnName) throws StatementExecutionException {
}
int index = columnOrdinalMap.get(columnName) - START_INDEX;
if (!isNull(index, rowsIndex - 1)) {
+ lastReadWasNull = false;
return BytesUtils.bytesToLong(values[index]);
} else {
- throw new StatementExecutionException(String.format(VALUE_IS_NULL, columnName));
Review comment:
@qiaojialin The reason why I remove the exception here is that it was not consistent with JDBC specification as we discussed in [IOTDB-912](https://issues.apache.org/jira/projects/IOTDB/issues/IOTDB-912?filter=allopenissues).
----------------------------------------------------------------
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] [iotdb] HTHou merged pull request #1791: [IOTDB-912] Implement wasNull method in AbstractIoTDBJDBCResultSet
Posted by GitBox <gi...@apache.org>.
HTHou merged pull request #1791:
URL: https://github.com/apache/iotdb/pull/1791
----------------------------------------------------------------
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] [iotdb] jack870131 commented on a change in pull request #1791: [IOTDB-912] Implement wasNull method in AbstractIoTDBJDBCResultSet
Posted by GitBox <gi...@apache.org>.
jack870131 commented on a change in pull request #1791:
URL: https://github.com/apache/iotdb/pull/1791#discussion_r502801510
##########
File path: service-rpc/src/main/java/org/apache/iotdb/rpc/IoTDBRpcDataSet.java
##########
@@ -343,9 +352,11 @@ public long getLong(String columnName) throws StatementExecutionException {
}
int index = columnOrdinalMap.get(columnName) - START_INDEX;
if (!isNull(index, rowsIndex - 1)) {
+ lastReadWasNull = false;
return BytesUtils.bytesToLong(values[index]);
} else {
- throw new StatementExecutionException(String.format(VALUE_IS_NULL, columnName));
Review comment:
The reason why I remove the exception here is that it was not consistent with JDBC specification as we discussed in [IOTDB-912](https://issues.apache.org/jira/projects/IOTDB/issues/IOTDB-912?filter=allopenissues).
----------------------------------------------------------------
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