Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty ResultSet And ResultSetLog output Total: 1 #1917

Closed
10brothers opened this issue May 6, 2020 · 10 comments
Closed

Empty ResultSet And ResultSetLog output Total: 1 #1917

10brothers opened this issue May 6, 2020 · 10 comments

Comments

@10brothers
Copy link

it 's very confused,the log tells me that there has one record Total: 1, actually , nothing, the ResultSet is Empty
So i debugged, found this

image

and this

image

that's make me crazy, i am read the source code , hope to find the reason, solve my problem.
finally, i am fixed my problem, it was caused by MySQL high level Driver need specify a serverTimezone
But i wasted too much time on read source code, and noting help to solve problem
we should fix this, make the log clearly and correctly

@harawata
Copy link
Member

harawata commented May 6, 2020

Hello @kilowang ,

Please fill out the bug report template.
We need these information to investigate the problem.

## MyBatis version
3.x.x

## Database vendor and version

## Test case or example project

## Steps to reproduce

## Expected result

## Actual result

And please use text instead of images.

@10brothers
Copy link
Author

sorry for that, but i am already figure out what happened, there is no need to provide any Test case or example project, just do that like below

  • prepare a MySQL DB Server (my MySQL Server version is 5.7.29 and 5.6.36 )
  • create table , empty table is alright
CREATE TABLE `TestTable` (
  `id` int(11) NOT NULL DEFAULT '0',
  `column1` int(11) DEFAULT '0',
  `column2` int(11) DEFAULT '0'
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci
  • set debug level : DEBUG
  • and execute query ,
@Select("SELECT SUM(column1),SUM(column2) FROM TestTable WHERE id = #{id}")
Test sum(@Param("id") int id);

and will find below in log file

==> Parameters: 10(Integer)
<==      Total: 1

but the method sum return a null Object
the result is true, very correctly, both are correct.
if you execute this query on mysql client directly, will got this

+---------------+----------------+
| SUM(column1)  |  SUM(column2)  |
+---------------+----------------+
|    NULL       |     NULL       |
+---------------+----------------+

yes, we queried one record , so when we enable debug, we got log like this Total: 1, that's true.

private boolean applyAutomaticMappings(ResultSetWrapper rsw, ResultMap resultMap, MetaObject metaObject, String columnPrefix) throws SQLException {
  List<UnMappedColumnAutoMapping> autoMapping = createAutomaticMappings(rsw, resultMap, metaObject, columnPrefix);
  boolean foundValues = false;
  if (!autoMapping.isEmpty()) {
    for (UnMappedColumnAutoMapping mapping : autoMapping) {
      final Object value = mapping.typeHandler.getResult(rsw.getResultSet(), mapping.column);
     //  look here plz
     //  all value is null , so `foundValues` alway be false 
     //  look here plz
      if (value != null) {
        foundValues = true;
      }
      if (value != null || (configuration.isCallSettersOnNulls() && !mapping.primitive)) {
        // gcode issue #377, call setter on nulls (value is not 'found')
        metaObject.setValue(mapping.property, value);
      }
    }
  }
 // return false;
  return foundValues;
}

private Object getRowValue(ResultSetWrapper rsw, ResultMap resultMap, String columnPrefix) throws SQLException {
  final ResultLoaderMap lazyLoader = new ResultLoaderMap();
  Object rowValue = createResultObject(rsw, resultMap, lazyLoader, columnPrefix);
  if (rowValue != null && !hasTypeHandlerForResultObject(rsw, resultMap.getType())) {
    final MetaObject metaObject = configuration.newMetaObject(rowValue);
    boolean foundValues = this.useConstructorMappings;
    if (shouldApplyAutomaticMappings(resultMap, false)) {
      foundValues = applyAutomaticMappings(rsw, resultMap, metaObject, columnPrefix) || foundValues;
    }
    foundValues = applyPropertyMappings(rsw, resultMap, metaObject, lazyLoader, columnPrefix) || foundValues;
    foundValues = lazyLoader.size() > 0 || foundValues;
    // look here plz
    // rowValue will be null, and finally return a null 
    // look here plz
    rowValue = foundValues || configuration.isReturnInstanceForEmptyRow() ? rowValue : null;
  }
  return rowValue;
}

@harawata
Copy link
Member

harawata commented May 7, 2020

I agree that 'Total: 1' seems to be the right output.
So, there is no bug to fix and we can close this issue, right?

@10brothers
Copy link
Author

yeah, it's very hard to say that is a bug and the scenario is rare, i am not sure if you guys can do something make it clearly - keep ResultSetLog output and Mapper Method returnObj consistent, if Total: 1, return object not null, otherwise null
i know it's not a big deal, if we do nothing about that , that's ok !
tell me what's your decision, please

@harawata
Copy link
Member

harawata commented May 8, 2020

I think you should try enabling returnInstanceForEmptyRow.
The documentation is here:
https://mybatis.org/mybatis-3/configuration.html

Then you will receive an instance of Test instead of null when all columns in the result set are NULL.
This is the behavior you expect, isn't it?

@10brothers
Copy link
Author

Ok, i will try enable returnInstanceForEmptyRow , and close this issue. But i still thought that is difference, enable this configure is a little trick :-)

@Alice52
Copy link

Alice52 commented Sep 21, 2022

@harawata I want to know why returnInstanceForEmptyRow's default value is false?

I think it's strange for some scenario, such as list contains null element, which will lead to npe probably.

@harawata
Copy link
Member

@Alice52

returnInstanceForEmptyRow was added recently (version 3.4.2) and we wanted to keep backward compatibility.
Besides, MyBatis does not add null to a list even if returnInstanceForEmptyRow is not enabled.

@Alice52
Copy link

Alice52 commented Sep 22, 2022

@harawata

returnInstanceForEmptyRow was added recently (version 3.4.2) and we wanted to keep backward compatibility.

Yep, I have great understand about default value, thx.

Besides, MyBatis does not add null to a list even if returnInstanceForEmptyRow is not enabled.

It's do will put null value to list in below scenario: all selected column are null value, then the rowvalue in mybatis will be assigned to null after handled

  • data in database

    id a_column others columns
    1 NULL ...
  • this is raw SQL:

    -- and the response is Entity list, a_column is null value
    select a_column from table_a where id=1
  • then I get the response list, which size is 1 and just have null element in list. [It's more prosible for npe]

  • I had to search about it by debugging, and got the core logic about handle rowvalue

    private Object getRowValue(ResultSetWrapper rsw, ResultMap resultMap, String columnPrefix) throws SQLException {
        final ResultLoaderMap lazyLoader = new ResultLoaderMap();
        Object rowValue = createResultObject(rsw, resultMap, lazyLoader, columnPrefix);
        if (rowValue != null && !hasTypeHandlerForResultObject(rsw, resultMap.getType())) {
            final MetaObject metaObject = configuration.newMetaObject(rowValue);
            boolean foundValues = this.useConstructorMappings;
            if (shouldApplyAutomaticMappings(resultMap, false)) {
                foundValues = applyAutomaticMappings(rsw, resultMap, metaObject, columnPrefix) || foundValues;
            }
            foundValues = applyPropertyMappings(rsw, resultMap, metaObject, lazyLoader, columnPrefix) || foundValues;
            foundValues = lazyLoader.size() > 0 || foundValues;
            ////////////////////////////
            // rowValue will be assigned to null here in this scenario`[all selected values are null value]`
            // , then this null will be put to list in calller method.
            ////////////////////////////
            rowValue = foundValues || configuration.isReturnInstanceForEmptyRow() ? rowValue : null;
        }
        return rowValue;
    }

so, I have a proposal to change returnInstanceForEmptyRow default value to true, should we do this?

  1. It will have side impact on aggregate function, such as select a_column, sum(a_column) as sum from table_a;

@harawata
Copy link
Member

@Alice52 ,
Ah, you're right. In that case, null will be added to the list.

If we change the default value, existing applications that expect null element in the list will stop working.
So, we will not change the default value.

Adding comments on closing issue is not appropriate.
If you have something to discuss, please use the mailing list.
If you have technical question, ask it on the mailing list or Stack Overflow with the details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants