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

<bind> should behave like a local variable inside <foreach> #2754

Open
tsasaki609 opened this issue Dec 11, 2022 · 5 comments
Open

<bind> should behave like a local variable inside <foreach> #2754

tsasaki609 opened this issue Dec 11, 2022 · 5 comments

Comments

@tsasaki609
Copy link

I found the following problem and report it.
If I have time, I will fix it and create a pull request.

MyBatis version

3.5.11

Database vendor and version

SQLite3 (probably for any vendor)

Test case or example project

https://github.com/tsasaki609/mybatis-3-sscce-foreach-with-bind

Steps to reproduce

check out the example project and run

Expected result

09:44:57.720 [main] DEBUG mybatis.sscce.foreach.with.bind.FooMapper.countBar - ==> Preparing: SELECT count(*) FROM bar WHERE id IN ( ? , ? )
09:44:57.755 [main] DEBUG mybatis.sscce.foreach.with.bind.FooMapper.countBar - ==> Parameters: test001(String), test002(String)
09:44:57.787 [main] DEBUG mybatis.sscce.foreach.with.bind.FooMapper.countBar - <== Total: 1
2

Actual result

09:44:57.720 [main] DEBUG mybatis.sscce.foreach.with.bind.FooMapper.countBar - ==> Preparing: SELECT count(*) FROM bar WHERE id IN ( ? , ? )
09:44:57.755 [main] DEBUG mybatis.sscce.foreach.with.bind.FooMapper.countBar - ==> Parameters: test002(String), test002(String)
09:44:57.787 [main] DEBUG mybatis.sscce.foreach.with.bind.FooMapper.countBar - <== Total: 1
1

@harawata
Copy link
Member

Thank you for the report, @tsasaki609 .

This is a duplicate of #575 actually.
In your case, defining getText() may be sufficient, but there is no elegant workaround for complex cases, unfortunately.

@tsasaki609
Copy link
Author

@harawata Thanks for the reply.
Yes, you are right, we just need to define getter in the simple case.
Perhaps a more complex case could be solved as well.
If so, I don't see the benefit of having bind available inside foreach.
Wouldn't it be better if it were not available?

@harawata
Copy link
Member

@tsasaki609 ,

Are you proposing to update the DTD?
I'm not familiar with DTD, but is it possible to avoid <bind> nested inside other tags like <if>?

<foreach ...>
  <if ...>
    <bind ...>

@tsasaki609
Copy link
Author

@harawata ,
Yes, I think updating the DTD would solve the confusion, but I can't be sure.
For example, the following defines that a <bind> can be placed inside a <foreach>.

<!ELEMENT foreach (#PCDATA | include | trim | where | set | foreach | choose | if | bind)*>

harawata added a commit to harawata/mybatis-3 that referenced this issue Dec 17, 2022
harawata added a commit to harawata/mybatis-3 that referenced this issue Dec 17, 2022
…phase

- Evaluated param values are stored in `ParameterMapping` and later used in DefaultParameterHandler
- There is no change when processing RawSqlSource
- Removed unused `injectionFilter` from TextSqlNode (mybatisgh-117)

This should fix mybatis#2754 .

This might also fix mybatis#206 and mybatis#575 , but with this patch, it still is not possible to invoke a method with parameter inside a parameter reference like `#{_parameter.mymethod(_parameter.value)}`.
It might be possible to accept OGNL expression in a param reference (e.g. `#{${_parameter.mymethod(_parameter.value)}}`), but I'm not sure if that's a good idea.
@harawata
Copy link
Member

harawata commented Jan 3, 2023

@tsasaki609 ,

We plan to address the original issue you reported in the next minor update 3.6.x (cf. #2760 )

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

No branches or pull requests

2 participants