#1314 v3.8.0 multiline comment sometimes makes SQuirreL hang

[sfst]

Good news first: snapshot-20170923_1027 does NOT have the problem, but I cannot conclude from the change log whether there was a real bug identified and fixed or the fix was just a lucky side-effect of some other change.
(v.3.7.1 also had no problem.)

Version 3.8.0 hanged with this command (you do not need to create any table):
SELECT F1 /comment/,F2
from (select tt.
FROM (SELECT t.,'1' F1
FROM (select '0' F2
FROM T1 where ID='E1'
) t
) tt
)

I tried to make the command as simple as possible while still reproducing the error, so please don't mind the rather strange command.

SQuirreL no longer responded and took up about 25% of the CPU (I have 4 cores, but the load was spread out over them).

Removing the multi line comment helped.
In the session setting I cannot change the option for the already checked "Remove multi line comment (/.../) from SQL before sending to database", because "[..it is] perdefined (sic) by your database specific Plugin." (I am using Oracle's ojdbc14.jar on Windows 8.1 with the oracle plugin)

This did not hang:

SELECT F1 /comment/,F2
from (select tt.,'1' F1
FROM (SELECT t1.,'0' F2
FROM T1 where ID='E1'
) tt
)

and correctly complained that the table didn't exist.

Removing all indentations from the original command worked:
SELECT F1 /comment/,F2
from (select tt.
FROM (SELECT t.,'1' F1
FROM (select '0' F2
FROM T1 where ID='E1'
) t
) tt
)

So the tabs in the original command apparently have some influence as well, but if I change them all to spaces it hangs again.

[sfst]

Argh! I forgot that tabs are not preserved on posting, the indentation tabs are essential! Sorry about that - I'll add an attachment with the whole post. (I probably would mess up the quoting today :-)

[Dario Orescanin]

Do you have the sqlparam plugin turned on in 3.8.0 and/or 3.7.1? I believe I have found why SQuirreL hangs when you have comments embedded inside your SQL.

Using jvisualvm, I have traced the problem to the mentioned plugin. Specifically, SQLParamExecutionListener.java, line 99:

// Removes /*  */ comments
sql = sql.replaceAll("/\\*(.|\\s)*\\*/", "");

Not only is this code flawed (if you have multiple multiline comments in your SQL, it's possible that it will strip everything from the first to the last comment), it is also slow - it isn't bad for short SQL expressions, but it gets progressively worse as your expressions grow in length.

I propose this as a replacement (a more thorough review is suggested):

int i = 0;
StringBuilder sb = new StringBuilder(sql);

while ((i = sb.indexOf("/*")) != -1) {
    int j = sb.indexOf("*/");
    sb = sb.replace(i, j+2, "");
}
sql = sb.toString();

Not only is it much faster, it also deals with multiple comments correctly.

[sfst]

Do you have the sqlparam plugin turned on[..]

Good catch!
Yes, I always have that turned on and unloading it in 3.8.0 indeed helps - thank you.

The question remains why only 3.8.0 has a problem with it.

I misunderstood the "database specific Plugin", I had tried unloading the Oracle plugin, which still didn't allow me changing that option for removing multi line comments.
Luckily I never had to use Oracle hints so far.

[sfst]

SQuirreL 3.8.1 no longer seems to have this problem - thank you.
The issue can be closed.

[Gerd Wagner]

Closed on user request.