Share blank nodes

[nieqiurong]

In the memory analysis, it was found that there were some blank nodes in mybatis,
Some are line breaks followed by blank strings, which are considered as blank nodes. However, the space occupied by these nodes is meaningless, and it is possible to cache these blank nodes during parsing and only retain one instance

example:

<script>
select * from user
<where>
<if test="1==1">and id = 1</if>
<if test="1==1">and id > 0</if>
</where>
</script>

For example, when parsing the sample script, four line break tags will be created. This is just a simple example, and we will use more dynamic tags in the project.

This modification will not change the original script content.

[coveralls]

coverage: 87.241% (+0.02%) from 87.226%
when pulling 2433820 on nieqiurong:202502222213
into b90a0f8 on mybatis:master.

[harawata]

Thank you for the PR, @nieqiurong !

I really liked the effect of your original PR #3349 which significantly reduces the number of SqlNodes to process.
And I wasn't sure why using <where> broke #3349, so I dug a little.

After evaluating each SqlNode, MyBatis concatenates the result strings.
In MixedSqlNode, it is done by DynamicContext#appendSql() which internally uses StringJoiner#add(), so it adds an extra space when concatenating SQL strings.
However, in TrimSqlNode (which is WhereSqlNode's parent class), FilteredDynamicContext#appendSql() is called and the result strings are concatenated without the extra space.
This is why #3349 broke when <where>, <set> or <trim> is used.

I think, for consistency, TrimSqlNode should add the extra space like MixedSqlNode does. Then we can (re)apply #3349 .
I still haven't made my mind about what to do, but could you take a look at #3422 and see if there is anything I am missing?

Cc: @epochcoder

[nieqiurong]

Thank you for the PR, @nieqiurong !

I really liked the effect of your original PR #3349 which significantly reduces the number of SqlNodes to process. And I wasn't sure why using <where> broke #3349, so I dug a little.

After evaluating each SqlNode, MyBatis concatenates the result strings. In MixedSqlNode, it is done by DynamicContext#appendSql() which internally uses StringJoiner#add(), so it adds an extra space when concatenating SQL strings. However, in TrimSqlNode (which is WhereSqlNode's parent class), FilteredDynamicContext#appendSql() is called and the result strings are concatenated without the extra space. This is why #3349 broke when <where>, <set> or <trim> is used.

I think, for consistency, TrimSqlNode should add the extra space like MixedSqlNode does. Then we can (re)apply #3349 . I still haven't made my mind about what to do, but could you take a look at #3422 and see if there is anything I am missing?

Cc: @epochcoder

If empty rows are removed, it can lead to some extreme cases.

  @Test
  void test(){
    var script = """
<script>
                            select * from `user` -- use sql comment
                            <where>
                                <if test="true">1=1 -- test</if>
                                <if test="true">and 1=1</if>
                                and 2=2
                            </where>
</script>
      """;
    var languageDriver = new XMLLanguageDriver();
    var sqlSource = languageDriver.createSqlSource(new Configuration(), script, Object.class);
    System.out.println(sqlSource.getBoundSql(null).getSql());
  }

For example, in this comment case, - -test is a comment, and if no newline character exists, the following condition is connected, resulting in the later execution condition to be treated as a comment

-- error sql
select * from `user` -- use sql comment
                             WHERE 1=1 -- test and 1=1 
                                and 2=2

-- 
select * from `user` -- use sql comment
                             WHERE 1=1 -- test
                                and 1=1
                                and 2=2

[nieqiurong]

Can we also expose the shrinkWhitespacesInSql processing method to the users' customization?
The original SqlSourceBuilder.removeExtraWhitespaces does not handle this sql case mentioned above.

System.out.println(SqlSourceBuilder.removeExtraWhitespaces("select * from `user` -- use sql comment\n" +
            "                             WHERE 1=1 -- test\n" +
            "                                and 1=1\n" +
            "                                and 2=2"));

// select * from `user` -- use sql comment WHERE 1=1 -- test and 1=1 and 2=2

[harawata]

Thanks for the comment, @nieqiurong !

In general, you should avoid single line comments when you use JDBC.
Use multiline comments /* comment */ instead.

Still, it may be better not to surprise users.
I give up on #3349 (for now) and will evaluate the effect of this PR.

[nieqiurong]

@harawata Thank you for your reply.

Yes, if treated by removing new lines and adding blank spaces, it will cause some incompatibilities.

<script>
                            select * from `user`
                            <where>
                                <if test="true">1=1</if><if test="true">and 1=1</if>
                                and 2=2
                            </where>
                            <if test="true">and 3=3</if><if test="true">and 4=4</if>
</script>

select * from `user`
                             WHERE 1=1 and 1=1 
                                and 2=2 and 3=3 and 4=4

-- old version  
select * from `user`
                             WHERE 1=1and 1=1
                                and 2=2 
                             and 3=3 and 4=4

Maybe there are better options for improvement, but I haven't found it yet.