sylvainlaurent / JDBC-Performance-Logger

A JDBC driver wrapper and GUI to analyze statement performance
Apache License 2.0
84 stars 26 forks source link

fix 65 Incorrect "filled SQL" if comment in SQL contains "?" #71

Closed samyBadjoudj closed 4 years ago

samyBadjoudj commented 4 years ago

Parser without lexical analyser to remove comments which can appear on prepared statements In case of malformed comments or empty string return the original rawsql

sylvainlaurent commented 4 years ago

hello Samy ! long time no see ! thanks for your PR. The CI was no longer working because of JDK8 is no longer on Travis-CI... I fixed it on master branch. Please rebase your work on the latest commit of master.

samyBadjoudj commented 4 years ago

Yes it has been years :) hope you are doing well !! Did the rebase for travis conf. Have noticed for the oracle jdk 8 https://travis-ci.community/t/install-of-oracle-jdk-8-failing/3038

sylvainlaurent commented 4 years ago

I think I found a better way of dealing with all of this : the following regex allows to find question marks outside of SQL comments and outside of string literal:

(?:'(?:''|[^'])*')|(?:--[^\r\n]*$)|(?:\/\*(?:[^\*]|\*(?!\/))*\*\/)|(\?)

Basically it tries to find: (?:'(?:''|[^'])*') : string literal, taking into account escaped single quotes (double single quotes). non-capturing group because we are not interested in it (?:--[^\r\n]*$) : end-of-line comments (still non-capturing) (?:\/\*(?:[^\*]|\*(?!\/))*\*\/) : block comments (still non-capturing) (\?) : question marks, with a capturing group

Here is a sample test on how to use it:

package ch.sla.jdbcperflogger.logger;

import org.junit.Assert;
import org.junit.Test;

import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static java.util.regex.Pattern.DOTALL;
import static java.util.regex.Pattern.MULTILINE;
import static org.junit.Assert.assertEquals;

public class PreparedStatementFillerTest {
    private Pattern pattern = Pattern.compile("(?:'(?:''|[^'])*')|(?:--[^\\r\\n]*$)|(?:\\/\\*(?:[^\\*]|\\*(?!\\/))*\\*\\/)|(\\?)", MULTILINE | DOTALL);
    private final static String REPL = "___REPL___";

    @Test
    public void test() {
        String sql = "select  from table '--co''oo?' dlkj 'ti-- /* ti ? */' --toto  \n" +
                "2nd ligne /* comment */ lkjl /* comment\n" +
                "' ok ' * ok\n" +
                "on multilines */\n" +
                "where toto = ? kfjlskd";

        String replaced = replaceQuestionMarkForBindVariables(sql);

        System.out.println("FINAL sql: " + replaced);

        assertEquals("", replaceQuestionMarkForBindVariables(""));
        assertEquals(REPL, replaceQuestionMarkForBindVariables("?"));
        assertEquals(REPL + " + " + REPL, replaceQuestionMarkForBindVariables("? + ?"));
        assertEquals("select * from toto", replaceQuestionMarkForBindVariables("select * from toto"));
        assertEquals("select * from toto where name=" + REPL, replaceQuestionMarkForBindVariables("select * from toto where name=?"));
        assertEquals("select * from toto where name=" + REPL + " || '?'", replaceQuestionMarkForBindVariables("select * from toto where name=? || '?'"));
        assertEquals("select * from toto where name=" + REPL + " -- eol comment?", replaceQuestionMarkForBindVariables("select * from toto where name=? -- eol comment?"));
        assertEquals("select * from toto where name=" + REPL + " /*comment?*/", replaceQuestionMarkForBindVariables("select * from toto where name=? /*comment?*/"));
        assertEquals("select * from toto where name=" + REPL + " /*multiline\ncomment?*/", replaceQuestionMarkForBindVariables("select * from toto where name=? /*multiline\ncomment?*/"));
    }

    private String replaceQuestionMarkForBindVariables(String sql) {

        Matcher matcher = pattern.matcher(sql);

        StringBuilder strBuilder = new StringBuilder();

        int lastReplacementIndex = 0;

        while (matcher.find()) {
            if (matcher.group(1) != null) {
                strBuilder.append(sql.substring(lastReplacementIndex, matcher.start(1)));
                strBuilder.append(REPL);
                lastReplacementIndex = matcher.end(1);
            }
        }

        strBuilder.append(sql.substring(lastReplacementIndex, sql.length()));
        return strBuilder.toString();
    }
}

What do you think @samyBadjoudj ? would you integrate this into your PR?