Merge Scripting and Parameterization phases in dynamic sql

[mnesarco]

Currently dynamic sql is managed in two phases, one for scripting and one for sql parameterization. This leads to two different languages in a dynamic sql at the same time and is a common source of mistakes.

For example, using velocity scripting we can do something like:

SELECT * from mytable Where mycolumn = '$_parameter.mymethod($_parameter.value)'

but we can't do:

SELECT * from mytable Where mycolumn = @{$_parameter.mymethod($_parameter.value)}

Because SQL parameter binding language is not velocity, but a propietary binding language.

The problem goes worst in loops, because the scope of the scripting variables and sql parameter are not the same. So <bind /> will not work in loops as expected.

My proposal is to extract any sql parameter in the scripting phase and cache it in the BoundSql object as proposed some time ago by Eduardo, then the sql parameterization will use the already extracted values directly by index, so we can use arbitrary scripting expressions.

For example:

SELECT * from mytable Where mycolumn = $sql.bind($_parameter.mymethod($_parameter.value))

the hypothetical $sql.bind method should support more parameters like type (IN,OUT,INOUT), jsdbType, etc...

Also in the case of velocity, the method can be replaced by a more friendly syntax (macro):

#set ($p = $_parameter)
SELECT * from mytable Where mycolumn = #param( $p.mymethod( $p.value ) )

[Drizzt321]

Any idea when this is going to be worked on and completed? We're just starting to use MyBatis, and hitting the exact issue that is described in the last example on #206. We could move forward using ${} syntax for the time being, as the initial functionality is read-only, but will become a big deal moving forward since we'll have user input to store. I'm fine building from source and using that until the official 3.4.0 release.

[Drizzt321]

Are you not including this in 3.4.1 now? In the near future we're going to start be doing user-input, and having this fixed so that we can input data as a prepared statement to prevent injection would be wonderful.

I have no clue where exactly in the code this work would be done or how with the libraries being used, or I'd offer to do some of the work.

[emacarron]

Hi @Drizzt321. Sorry, I moved it to the next mayor release. Nobody is working on this now so please be aware that, as we are volunteers, this may never be implemented.

[Drizzt321]

@emacarron I do understand you're volunteers, is there any direction you could give so I could attempt to implement and provide a PR? I could go through and try and follow the code paths, but honestly I don't have time to go that far, but if I have a section of code I might be able to devote a few days too it.

[emacarron]

@Drizzt321 I am afraid that this change will have a massive impact on code base. The core engine of MyBatis if focused on dealing with parmeters, expressions and statements. So I am afraid I cannot point you to an specific block of code. I am sorry!

[Drizzt321]

Ah, I see. That's too bad. I'm actually a little surprised this hasn't come up earlier than now. Maybe I'll end up having to change my Mapper configs to try and work around it so I won't have this bind problem so I can properly use prepared statement output.

[Drizzt321]

Ok, for those who find this, I solved my problem by having a utility method which returns a Map with the dynamic field name/value set, and iterated through the map. Here's a mapper fragement. getModifiedFields() returns a Map<String,Object>. I have it with a bind so that I can be sure that it iterates the same way for each for loop. For actual implementation, I'm using a TreeMap, but any kind of map with a consistent iteration should work fine.

INSERT INTO ${tableName}
<bind name="data" value="entity.getModifiedFields()"/>
<foreach item="value" index="name" collection="data" open="(" separator="," close=")">
    ${name}
</foreach>
VALUES
<foreach item="value" index="name" collection="data" open="(" separator="," close=")">
    #{value}
</foreach>