You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2022/11/07 13:12:40 UTC
[GitHub] [shardingsphere] sandynz opened a new pull request, #21992: Improve AbstractMemoryQueryResult.wasNull
sandynz opened a new pull request, #21992:
URL: https://github.com/apache/shardingsphere/pull/21992
Changes proposed in this pull request:
- Improve AbstractMemoryQueryResult.wasNull
---
Before committing this PR, I'm sure that I have checked the following options:
- [ ] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
- [ ] I have self-reviewed the commit code.
- [ ] I have (or in comment I request) added corresponding labels for the pull request.
- [ ] I have passed maven check locally : `mvn clean install -B -T2C -DskipTests -Dmaven.javadoc.skip=true -e`.
- [ ] I have made corresponding changes to the documentation.
- [ ] I have added corresponding unit tests for my changes.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] sandynz commented on pull request #21992: Improve AbstractMemoryQueryResult.wasNull
Posted by GitBox <gi...@apache.org>.
sandynz commented on PR #21992:
URL: https://github.com/apache/shardingsphere/pull/21992#issuecomment-1325906486
OK, I'll update PR description and labels
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] sandynz commented on pull request #21992: Improve AbstractMemoryQueryResult.wasNull
Posted by GitBox <gi...@apache.org>.
sandynz commented on PR #21992:
URL: https://github.com/apache/shardingsphere/pull/21992#issuecomment-1325896338
OK, thanks. I've updated some unit test cases in `JDBCMemoryQueryResultTest` before, could you help to double check it? It might need more test cases
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] FlyingZC commented on pull request #21992: Improve AbstractMemoryQueryResult.wasNull
Posted by GitBox <gi...@apache.org>.
FlyingZC commented on PR #21992:
URL: https://github.com/apache/shardingsphere/pull/21992#issuecomment-1325827249
When call `next` method,
```java
public final boolean next() {
if (rows.hasNext()) {
currentRow = rows.next();
rowCount--;
return true;
}
// when rows.hasNext() is false, this line will set currentRow = null and `wasNull()` method will return true before,but return false now.
currentRow = null;
return false;
}
```
when rows.hasNext() is false, this line will set currentRow = null and `wasNull()` method will return true before,but return false now.
wasNull method before:
```java
public final boolean wasNull() {
// return true;
return null == currentRow;
}
```
wasNull method now:
```java
public final boolean wasNull() {
// return false;
return wasNull;
}
```
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] terrymanu merged pull request #21992: Improve AbstractMemoryQueryResult.wasNull
Posted by GitBox <gi...@apache.org>.
terrymanu merged PR #21992:
URL: https://github.com/apache/shardingsphere/pull/21992
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] FlyingZC commented on pull request #21992: Improve AbstractMemoryQueryResult.wasNull
Posted by GitBox <gi...@apache.org>.
FlyingZC commented on PR #21992:
URL: https://github.com/apache/shardingsphere/pull/21992#issuecomment-1325885575
```java
Connection connection = getConnection();
String query = "select * from t_order;";
ResultSet resultSet = connection.createStatement().executeQuery(query);
while (resultSet.next()) {
// null value
String status = resultSet.getString(3);
// will be true
boolean wasNull = resultSet.wasNull();
if (wasNull) {
System.out.print("status: " + status);
}
}
```
By my test in mysql JDBC, when `resultSet.getXxx()` return null, `resultSet.wasNull()` will be true.It's the same with your change.But maybe we should check the usage of these methods,because the logic has changed.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] sandynz commented on pull request #21992: Improve AbstractMemoryQueryResult.wasNull
Posted by GitBox <gi...@apache.org>.
sandynz commented on PR #21992:
URL: https://github.com/apache/shardingsphere/pull/21992#issuecomment-1325861721
> When call `next` method,
>
> ```java
> public final boolean next() {
> if (rows.hasNext()) {
> currentRow = rows.next();
> rowCount--;
> return true;
> }
> // when rows.hasNext() is false, this line will set currentRow = null and `wasNull()` method will return true before,but return false now.
> currentRow = null;
> return false;
> }
> ```
>
> when rows.hasNext() is false, this line will set currentRow = null and `wasNull()` method will return true before,but return false now.
>
> wasNull method before:
>
> ```java
> public final boolean wasNull() {
> // return true;
> return null == currentRow;
> }
> ```
>
> wasNull method now:
>
> ```java
> public final boolean wasNull() {
> // return false;
> return wasNull;
> }
> ```
Hi @FlyingZC , some investigation as following.
The definition in JDBC specification of `wasNull`:
```
/**
* Reports whether
* the last column read had a value of SQL <code>NULL</code>.
* Note that you must first call one of the getter methods
* on a column to try to read its value and then call
* the method <code>wasNull</code> to see if the value read was
* SQL <code>NULL</code>.
*
* @return <code>true</code> if the last column value read was SQL
* <code>NULL</code> and <code>false</code> otherwise
* @exception SQLException if a database access error occurs or this method is
* called on a closed result set
*/
boolean wasNull() throws SQLException;
```
MySQL JDBC implementation of `wasNull` (mysql-connector-java:5.1.47):
ResultSetImpl.java
```
/** Did the previous value retrieval find a NULL? */
protected boolean wasNullFlag = false;
public boolean wasNull() throws SQLException {
return this.wasNullFlag;
}
```
The default value of `wasNullFlag` is `false`.
PostgreSQL JDBC implementation of `wasNull` (postgresql:42.4.1):
PgResultSet.java
```
/**
* True if the last obtained column value was SQL NULL as specified by {@link #wasNull}. The value
* is always updated by the {@link #getRawValue} method.
*/
protected boolean wasNullFlag = false;
public boolean wasNull() throws SQLException {
checkClosed();
return wasNullFlag;
}
```
The default value of `wasNullFlag` is `false`.
So when `ResultSet.hasNext()` return `false` (Invoke ResultSet.getXxx() method will throw exception, `wasNullFlag` won't be set with value), `wasNull` should not be invoked, else it might return old value of `wasNullFlag` or default value `false` (if there's no any record in ResultSet).
But I didn't do real test on JDBC driver.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] FlyingZC commented on pull request #21992: Improve AbstractMemoryQueryResult.wasNull
Posted by GitBox <gi...@apache.org>.
FlyingZC commented on PR #21992:
URL: https://github.com/apache/shardingsphere/pull/21992#issuecomment-1325902215
Maybe next time we could first raise an issue to describe the issue, or describe the reason for the repair in the PR. This pr is easy to make others think it is a simple refactoring...
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org