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.

Sign up for GitHub

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

Jump to bottom

NumberFormatException is thrown if a column label contains brackets and result type is Map #3062

Closed
scf18857887860 opened this issue Jan 11, 2024 · 23 comments
Closed

NumberFormatException is thrown if a column label contains brackets and result type is Map #3062

scf18857887860 opened this issue Jan 11, 2024 · 23 comments
Assignees
@harawata
Labels
enhancement Improve a feature or add a new feature
Milestone
3.5.16

Comments

@scf18857887860
Copy link

MyBatis version

3.5.15

Database vendor and version

5.7.42

Test case or example project

xml sql is
"select (case when subject_name='aaa[张三]' then subject_name else null end) as subject_name from als_core_p"

Steps to reproduce

To execute directly through MyBatis, it means executing database operations through MyBatis without any intermediate processes.

Expected result

have result

Actual result

org.mybatis.spring.MyBatisSystemException: nested exception is org.apache.ibatis.exceptions.PersistenceException:

Error querying database. Cause: java.lang.NumberFormatException: For input string: "张三"
@harawata
Copy link
Member

Hello @scf18857887860 ,

Please post the full stack trace (enclose it in ```).

@scf18857887860
Copy link
Author

scf18857887860 commented Jan 11, 2024 via email

@harawata
Copy link
Member

Um..that seems like the top part of the stack. Is that all?

@scf18857887860
Copy link
Author

scf18857887860 commented Jan 11, 2024 via email •
edited
Loading

@harawata
Copy link
Member

I guess you are trying to set the results to a Map.
In that case, you cannot use [] in the column label, basically.
[] is used to access list/array item and causes a problem like that.

I don't know any good solution other than changing the column alias in the SQL, unfortunately.

@scf18857887860
Copy link
Author

I guess you are trying to set the results to a Map. In that case, you cannot use [] in the column label, basically. [] is used to access list/array item and causes a problem like that.

I don't know any good solution other than changing the column alias in the SQL, unfortunately.

unfortunately,i can't use Map,because this string is custome data, I can't avoid，I hope there is a way to avoid MyBatis parsing these special characters.

@harawata
Copy link
Member

You are not using Map?
Then I'm not sure what's wrong.

Please create a demo project and share it on your GitHub repository.
We need to reproduce the same exception to investigate.

Here are some project templates and examples :
https://github.com/harawata/mybatis-issues

And please ....

  • simplify the project (i.e. use minimum set of tables/columns/classes).
  • do not include mybatis-plus because it's not our product.

@scf18857887860
Copy link
Author

You are not using Map? Then I'm not sure what's wrong.

Please create a demo project and share it on your GitHub repository. We need to reproduce the same exception to investigate.

Here are some project templates and examples : https://github.com/harawata/mybatis-issues

And please ....

  • simplify the project (i.e. use minimum set of tables/columns/classes).
  • do not include mybatis-plus because it's not our product.

I do it,but I have no Permission and can't push,please give me enough Permission

@harawata
Copy link
Member

Here is what you need to do.

  1. Create the demo project and verify the same error reproduces.
  2. Create a new repository of your own. Make it 'public' so that we can see the files.
    https://docs.github.com/zh/repositories/creating-and-managing-repositories/creating-a-new-repository
  3. Then upload the files in your demo project to that repository.
    https://docs.github.com/zh/repositories/working-with-files/managing-files/adding-a-file-to-a-repository
  4. Let us know the repository URL.

@scf18857887860
Copy link
Author

scf18857887860 commented Jan 18, 2024 •
edited
Loading

Here is what you need to do.

  1. Create the demo project and verify the same error reproduces.
  2. Create a new repository of your own. Make it 'public' so that we can see the files.
    https://docs.github.com/zh/repositories/creating-and-managing-repositories/creating-a-new-repository
  3. Then upload the files in your demo project to that repository.
    https://docs.github.com/zh/repositories/working-with-files/managing-files/adding-a-file-to-a-repository
  4. Let us know the repository URL.

Hello, I've been waiting for a long time ，this is my repository url :[email protected]:scf18857887860/gh-3060.git, i write a test case name is “selectError” ，But I don't know if it's because of the database, the error here is not the same as mine.

@scf18857887860
Copy link
Author

I encountered a problem when I used MyBatis. When I used a map to receive the result set and the entire result set was empty, an empty map was returned, and there were no specified column names. However, I hope to return my column names. Is there a good solution? But I cannot use resultMap because it is a dynamic result set.

@scf18857887860
Copy link
Author

Hello, I have found the cause of this bug. Below is the specific code and description. However, there is currently no good solution. I hope you can notify me promptly after the subsequent version modification.
`
DefaultResultSetHand#applyAutomaticMappings(){
List 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);
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 foundValues;
}

MetaObject#setValue(){
PropertyTokenizer prop = new PropertyTokenizer(name);
if (prop.hasNext()) {
MetaObject metaValue = metaObjectForProperty(prop.getIndexedName());
if (metaValue == SystemMetaObject.NULL_META_OBJECT) {
if (value == null) {
// don't instantiate child path if value is null
return;
}
metaValue = objectWrapper.instantiatePropertyValue(name, prop, objectFactory);
}
metaValue.setValue(prop.getChildren(), value);
} else {
objectWrapper.set(prop, value);
}
}

public PropertyTokenizer(String fullname) {
int delim = fullname.indexOf('.');
if (delim > -1) {
name = fullname.substring(0, delim);
children = fullname.substring(delim + 1);
} else {
name = fullname;
children = null;
}
indexedName = name;
delim = name.indexOf('[');
if (delim > -1) {
index = name.substring(delim + 1, name.length() - 1);
name = name.substring(0, delim);
}
}

BaseWrapper#setCollectionValue(){
if (collection instanceof Map) {
((Map) collection).put(prop.getIndex(), value);
} else {
int i = Integer.parseInt(prop.getIndex());
if (collection instanceof List) {
((List) collection).set(i, value);
} else if (collection instanceof Object[]) {
((Object[]) collection)[i] = value;
} else if (collection instanceof char[]) {
((char[]) collection)[i] = (Character) value;
} else if (collection instanceof boolean[]) {
((boolean[]) collection)[i] = (Boolean) value;
} else if (collection instanceof byte[]) {
((byte[]) collection)[i] = (Byte) value;
} else if (collection instanceof double[]) {
((double[]) collection)[i] = (Double) value;
} else if (collection instanceof float[]) {
((float[]) collection)[i] = (Float) value;
} else if (collection instanceof int[]) {
((int[]) collection)[i] = (Integer) value;
} else if (collection instanceof long[]) {
((long[]) collection)[i] = (Long) value;
} else if (collection instanceof short[]) {
((short[]) collection)[i] = (Short) value;
} else {
throw new ReflectionException(
"The '" + prop.getName() + "' property of " + collection + " is not a List or Array.");
}
}
}
`

The above is the specific code. The bold part is the entry point, because in this （PropertyTokenizer） class, my alias name is split using the special character '[]', causing the index in PropertyTokenizer to become a string, resulting in an error when subsequently converting the index to an integer.

@harawata
Copy link
Member

Well, that is basically what I explained in this comment.

A possible workaround is to write a custom ObjectWrapper.
I sent a pull request to your repository.
The test selectError will pass if you merge the PR.

@scf18857887860
Copy link
Author

Well, that is basically what I explained in this comment.

A possible workaround is to write a custom ObjectWrapper. I sent a pull request to your repository. The test selectError will pass if you merge the PR.

I think it would be possible to add some kind of escape character to skip the indexof for the bracket, as production environments are complex and it is likely that other special characters will be encountered.
For example, \

@harawata
Copy link
Member

It's really difficult to change the core code because it could break existing applications.
I hope you understand.

@scf18857887860
Copy link
Author

scf18857887860 commented Jan 18, 2024 •
edited
Loading

It's really difficult to change the core code because it could break existing applications. I hope you understand.
For example,

public PropertyTokenizer(String fullname){
      if(fullname.startsWith("\\")){
            name=fullname.replace("\\","");
            return;
        }
int delim = fullname.indexOf('.');
    if (delim > -1) {
      name = fullname.substring(0, delim);
      children = fullname.substring(delim + 1);
    } else {
      name = fullname;
      children = null;
    }
    indexedName = name;
    delim = name.indexOf('[');
    if (delim > -1) {
      index = name.substring(delim + 1, name.length() - 1);
      name = name.substring(0, delim);
    }

}

@harawata
Copy link
Member

harawata commented Jan 18, 2024 •
edited
Loading

Thank you for the idea, but escape syntax does not help users who cannot control the column label.
And if you can control the column label, you should be able to use a label that does not contain [.

ObjectWrapper is provided to support a minor usage without affecting majority of the users, so it is exactly what you should use.

I'll take a deeper look to see if there is room for improvement.
Changing the visibility of MapWrapper.map field may make it easier to write a custom wrapper, for example.

diff --git a/src/main/java/org/apache/ibatis/reflection/wrapper/MapWrapper.java b/src/main/java/org/apache/ibatis/reflection/wrapper/MapWrapper.java
index 374b03e65b..e41df78459 100644
--- a/src/main/java/org/apache/ibatis/reflection/wrapper/MapWrapper.java
+++ b/src/main/java/org/apache/ibatis/reflection/wrapper/MapWrapper.java
@@ -29,7 +29,7 @@ import org.apache.ibatis.reflection.property.PropertyTokenizer;
  */
 public class MapWrapper extends BaseWrapper {
 
-  private final Map<String, Object> map;
+  protected final Map<String, Object> map;
 
   public MapWrapper(MetaObject metaObject, Map<String, Object> map) {
     super(metaObject);

I'll close this as the behavior is by design.

@scf18857887860
Copy link
Author

Thank you for the idea, but escape syntax does not help users who cannot control the column label. And if you can control the column label, you should be able to use a label that does not contain [.

ObjectWrapper is provided to support a minor usage without affecting majority of the users, so it is exactly what you should use.

I'll take a deeper look to see if there is room for improvement. Changing the visibility of MapWrapper.map field may make it easier to write a custom wrapper, for example.

diff --git a/src/main/java/org/apache/ibatis/reflection/wrapper/MapWrapper.java b/src/main/java/org/apache/ibatis/reflection/wrapper/MapWrapper.java
index 374b03e65b..e41df78459 100644
--- a/src/main/java/org/apache/ibatis/reflection/wrapper/MapWrapper.java
+++ b/src/main/java/org/apache/ibatis/reflection/wrapper/MapWrapper.java
@@ -29,7 +29,7 @@ import org.apache.ibatis.reflection.property.PropertyTokenizer;
  */
 public class MapWrapper extends BaseWrapper {
 
-  private final Map<String, Object> map;
+  protected final Map<String, Object> map;
 
   public MapWrapper(MetaObject metaObject, Map<String, Object> map) {
     super(metaObject);

I'll close this as the behavior is by design.

Ok, will this be released in the next version?

@scf18857887860
Copy link
Author

scf18857887860 commented Jan 18, 2024 •
edited
Loading

Thank you for the idea, but escape syntax does not help users who cannot control the column label. And if you can control the column label, you should be able to use a label that does not contain [.

ObjectWrapper is provided to support a minor usage without affecting majority of the users, so it is exactly what you should use.

I'll take a deeper look to see if there is room for improvement. Changing the visibility of MapWrapper.map field may make it easier to write a custom wrapper, for example.

diff --git a/src/main/java/org/apache/ibatis/reflection/wrapper/MapWrapper.java b/src/main/java/org/apache/ibatis/reflection/wrapper/MapWrapper.java
index 374b03e65b..e41df78459 100644
--- a/src/main/java/org/apache/ibatis/reflection/wrapper/MapWrapper.java
+++ b/src/main/java/org/apache/ibatis/reflection/wrapper/MapWrapper.java
@@ -29,7 +29,7 @@ import org.apache.ibatis.reflection.property.PropertyTokenizer;
  */
 public class MapWrapper extends BaseWrapper {
 
-  private final Map<String, Object> map;
+  protected final Map<String, Object> map;
 
   public MapWrapper(MetaObject metaObject, Map<String, Object> map) {
     super(metaObject);

I'll close this as the behavior is by design.

I have reviewed the source code and found that this modification does not seem to prevent the problem from occurring.


MapWrapper#set(PropertyTokenizer prop, Object value) {
if (prop.getIndex() != null) {
      Object collection = resolveCollection(prop, map);
      setCollectionValue(prop, collection, value);
    } else {
      map.put(prop.getName(), value);
    }
}

@harawata
Copy link
Member

You still need to write a custom ObjectWrapper and ObjectWrapperFactory.
With the change in my comment, you can extend the built-in MapWrapper and override a few methods.

@scf18857887860
Copy link
Author

scf18857887860 commented Jan 18, 2024 •
edited
Loading

You still need to write a custom ObjectWrapper and ObjectWrapperFactory. With the change in my comment, you can extend the built-in MapWrapper and override a few methods.

MapWrapper#set(PropertyTokenizer prop, Object value) {
   String regex = "^\\d+$";
   Pattern pattern = Pattern.compile(regex);
   Matcher matcher = pattern.matcher(prop.getIndex());
   if (prop.getIndex() != null) {
       if(!matcher.matches())
       {
           map.put(prop.getIndexName(), value);
           return;
      }
      Object collection = resolveCollection(prop, map);
      setCollectionValue(prop, collection, value);
    } else {
      map.put(prop.getName(), value);
    }
}

this is ok?

@harawata
Copy link
Member

harawata commented Jan 18, 2024 •
edited
Loading

No, it is not OK.
We will not support escape syntax.
And I already explained why.

Please use the custom map wrapper in the pull request I sent.

@harawata harawata mentioned this issue Jan 18, 2024
Closed
@harawata harawata changed the title select error NumberFormatException is thrown if a Map key contains brackets Jan 21, 2024
@harawata harawata changed the title NumberFormatException is thrown if a Map key contains brackets NumberFormatException is thrown if a column label contains brackets and result type is Map Jan 21, 2024
harawata added a commit to harawata/mybatis-3 that referenced this issue Jan 24, 2024
@harawata
Allow custom Map wrapper to handle keys with dots or breackets
c2f18b7 
It's now  possible to write a custom Map wrapper that allows keys that contain period (dot) `.` or brackets `[]`.
Also, it's now possible to write a custom Map wrapper by extending the built-in `MapWrapper`.

Should fix mybatis#13 mybatis#2298 mybatis#3062
@harawata harawata mentioned this issue Jan 24, 2024
Merged
@harawata harawata added enhancement Improve a feature or add a new feature and removed enhancement Improve a feature or add a new feature labels Jan 27, 2024
@harawata
Copy link
Member

@scf18857887860 ,

The fix #3076 will be included in the next release (3.5.16).
Until then, you can test the fix with the SNAPSHOT (see the wiki entry for how to use snapshot build).
Please let me know the details if you find any problem.

Thank you for your contribution!

@harawata harawata added the enhancement Improve a feature or add a new feature label Jan 28, 2024
@harawata harawata self-assigned this Jan 28, 2024
@harawata harawata added this to the 3.5.16 milestone Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@harawata harawata

Labels
enhancement Improve a feature or add a new feature
Projects
None yet
Milestone
3.5.16
Development

No branches or pull requests
2 participants
@harawata @scf18857887860