Re: Bug in AbstracJdbc2Statement.replaceProcessing when using dollar quoting?

Am Dienstag, 9. Oktober 2012 19:49:17 UTC+2 schrieb "David Johnston":
> Hi,
>
>
>
> See below for my thoughts on how this is limited and inefficient.
>
...
>
> This patch appears to throw the exception when an unquoted identifier
>
> includes a dollar-sign.  I cannot speak to the standard but it would seem
>
> that presence of the dollar-sign should only be evaluated if we are
>
> "IN_SQLCODE" and the preceding character is NOT (alphanumeric or underscore)
>
> {specifically whatever characters are allowed to begin an unquoted
>
> identifier}.
>
>
>
> Also, with the logic provided the addition of an exception and a try/catch
>
> block appears to add unnecessary overhead.  At the point the exception is
>
> thrown I would suggest that we instead:
>
>
>
> "return p_sql;"
>
>
>
> and let the replaceProcessing method remain ignorant of whether the SQL it
>
> is getting back is the original SQL or a modified version.
>
>
>
> The fact is the presence of dollar-quoting with enabled escaping is going to
>
> be the common situation.  Combine that with the fact we are not prompting
>
> the user to "fix" anything means that using the exception mechanism a poor
>
> choice - it is not Exceptional but rather ordinary.  Everything that needs
>
> to be checked and confirmed can be done in the parseSQL method and since it
>
> has access to the original statement it can return the original when
>
> necessary.
>
>
>
> David J.

Hi,
I know that throwing that exception is a bit... uncommon. But I did that because of the while loop in
replaceProcessing.The code there does call parseSql several times,  at least it can. And therefor I don't know if it is
thefirst call when i found a dollar quote. If i just return the sql unchanged it might have had changes before so the
resultwould be replaced sql code together with original sql code. 
What I wanted to achieve was that replaceProcessing get's to know that there is a dollar quote and stop the
replaceProcessingand return the original sql string (just like replaceProcessingEnabled would have been set to false).
Iwanted to have a kind of stateful exit of the parseSql method. But since it only returns an int value, and might have
beencalled before, throwing an exception was the only thing I found to be able to do that.