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

Remove blank lines in SQL #1126

Closed
zhaoxi2000 opened this issue Nov 2, 2017 · 17 comments
Closed

Remove blank lines in SQL #1126

zhaoxi2000 opened this issue Nov 2, 2017 · 17 comments
Assignees
Labels
enhancement Improve a feature or add a new feature
Milestone

Comments

@zhaoxi2000
Copy link

zhaoxi2000 commented Nov 2, 2017

Question:
MyBatis build SQL include many blank lines from XML.
The blank lines is unnecessary.

https://issues.apache.org/jira/browse/IBATIS-143

class XMLScriptBuilder
Let XNode replace '\n' or "\r\n" to ' ' .
I want the feature can be configurable.

@kazuki43zoo
Copy link
Member

Hi @zhaoxi1988 , contributing welcome!! :D

@zhaoxi2000
Copy link
Author

Oh. I accept it.

By the way.
Why do not mybatis's package name 'org.apache.mybatis' ?

@George5814
Copy link

@zhaoxi1988 Is the problem solved?

@zhaoxi2000
Copy link
Author

@George5814 Still not solved...I busy on mybatis UnitTest...

@zhaoxi2000
Copy link
Author

zhaoxi2000 commented Dec 8, 2017

@George5814
My idea:
1.add in org.apache.ibatis.session.Configuration
2.push configuration down , let SqlSourceBuilder trim '\r|\n' or replace('\r|\n', ' ')

单元测试在IntelliJ上, 只安装JDK8有点尴尬...

@harawata
Copy link
Member

harawata commented Dec 8, 2017

I'm wondering why this feature request gets many votes.
Is there any actual harm caused by the extra blank lines?

@George5814
Copy link

This is to delete the extra whitespace produced by the dynamic tags, not to beautify the SQL.
Improve readability.

@harawata
Copy link
Member

harawata commented Dec 9, 2017

@George5814
Recent versions of MyBatis remove blank lines when printing the log, so where do you see the SQLs with blank lines?

@zhaoxi2000
Copy link
Author

@harawata Because the Database Server logging SQL is not convenient.
Then the next requirement is that tracing SQL instance in MyBatis , e.g. underlying one MapperFile.

@zhaoxi2000
Copy link
Author

@harawata I was an DBA. I see the problem like MySQL truncate the SQL because of the many blank lines.

@harawata
Copy link
Member

@zhaoxi1988 ,
Do you mean that the length of a generated SQL exceeds max_allowed_packet ?

@zhaoxi2000
Copy link
Author

zhaoxi2000 commented Dec 12, 2017

@harawata

Do you mean that the length of a generated SQL exceeds max_allowed_packet

It is not.
It is MySQL's query-log or InnoDB log when had a dead lock.

@harawata
Copy link
Member

@zhaoxi1988 ,

Thank you for the comment!

It is MySQL's query-log or InnoDB log when had a dead lock.

Okay. As I commented on the PR, the best we can do is to reduce the number of line breaks, probably.

@zhboseu
Copy link

zhboseu commented Sep 15, 2018

Question:
MyBatis build SQL include many blank lines from XML.
The blank lines is unnecessary.

https://issues.apache.org/jira/browse/IBATIS-143

class XMLScriptBuilder
Let XNode replace '\n' or "\r\n" to ' ' .
I want the feature can be configurable.

Hi, is this problem solved?

@harawata
Copy link
Member

harawata commented Sep 20, 2018

Nope.
If it's very important, writing a custom language driver and script builder should work.

The language driver can be pretty simple.

public class CustomXmlLanguageDriver extends XMLLanguageDriver {
  @Override
  public SqlSource createSqlSource(Configuration configuration, XNode script, Class<?> parameterType) {
    CustomXmlScriptBuilder builder = new CustomXmlScriptBuilder(configuration, script, parameterType);
    return builder.parseScriptNode();
  }
}

CustomXmlScriptBuilder is almost identical to org.apache.ibatis.scripting.xmltags.XMLScriptBuilder.
Just remove whitespaces in parseDynamicTags() method as @zhaoxi1988 did in #1153 .

And set defaultScriptLanguage in the config.

<settings>
  <setting name="defaultScriptingLanguage"
    value="xxx.yyy.CustomXmlLanguageDriver" />
</settings>

@harawata
Copy link
Member

It might be cleaner to do the substitution in the language driver.

import java.util.regex.Pattern;

import org.apache.ibatis.builder.xml.XMLMapperEntityResolver;
import org.apache.ibatis.mapping.SqlSource;
import org.apache.ibatis.parsing.PropertyParser;
import org.apache.ibatis.parsing.XNode;
import org.apache.ibatis.parsing.XPathParser;
import org.apache.ibatis.scripting.defaults.RawSqlSource;
import org.apache.ibatis.scripting.xmltags.DynamicSqlSource;
import org.apache.ibatis.scripting.xmltags.TextSqlNode;
import org.apache.ibatis.scripting.xmltags.XMLLanguageDriver;
import org.apache.ibatis.session.Configuration;

public class CompactXMLLanguageDriver extends XMLLanguageDriver {
  private static final Pattern pattern = Pattern.compile("[\\s]+");

  @Override
  public SqlSource createSqlSource(Configuration configuration, XNode script, Class<?> parameterType) {
    script.getNode().setTextContent(pattern.matcher(script.getNode().getTextContent()).replaceAll(" "));
    return super.createSqlSource(configuration, script, parameterType);
  }

  @Override
  public SqlSource createSqlSource(Configuration configuration, String script, Class<?> parameterType) {
    if (script.startsWith("<script>")) {
      XPathParser parser = new XPathParser(script, false, configuration.getVariables(), new XMLMapperEntityResolver());
      return createSqlSource(configuration, parser.evalNode("/script"), parameterType);
    } else {
      script = PropertyParser.parse(pattern.matcher(script).replaceAll(" "), configuration.getVariables());
      TextSqlNode textSqlNode = new TextSqlNode(script);
      if (textSqlNode.isDynamic()) {
        return new DynamicSqlSource(configuration, textSqlNode);
      } else {
        return new RawSqlSource(configuration, script, parameterType);
      }
    }
  }
}

You don't have to modify XMLScriptBuilder anymore.

@harawata harawata added this to the 3.5.5 milestone May 17, 2020
@harawata harawata added the enhancement Improve a feature or add a new feature label May 17, 2020
@harawata harawata self-assigned this May 17, 2020
@harawata
Copy link
Member

Fixed via #1901
We added a new option shrinkWhitespacesInSql
Please test 3.5.5.-SNAPSHOT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve a feature or add a new feature
Projects
None yet
Development

No branches or pull requests

5 participants