Support triggers & stored procedures alongside standard statements in SQL scripts [SPR-13389]

[spring-projects-issues]

Amol Khanolkar opened SPR-13389 and commented

Status Quo

In the current version of Spring we cannot execute an SQL script like the following if the script also contains standard SQL statements (e.g., INSERT, DELETE, etc.), because nested semicolons in the trigger declaration are interpreted as the default statement separator.

CREATE TRIGGER {TriggerNameName}
	BEFORE INSERT ON  {TableName} 
		FOR EACH ROW BEGIN 
			SET NEW.date_modified = CURTIME(6);
			SET NEW.modified_by = USER(); 
		END;

We can easily fix this by escaping the semicolons within the trigger declaration like this:

CREATE TRIGGER {TriggerNameName}
	BEFORE INSERT ON  {TableName} 
		FOR EACH ROW BEGIN 
			SET NEW.date_modified = CURTIME(6)\;
			SET NEW.modified_by = USER()\; 
		END\; ;

Proposal

Actually I see the following snippet in the source code for ScriptUtils:

if (inEscape) {
	inEscape = false;
	sb.append(c);
	continue;
}
// MySQL style escapes
if (c == '\\') {
	inEscape = true;
	sb.append(c);
	continue;
}

Modifying as follows fixes the issue. I am not sure why in original code Escape String was appended to the StringBuffer.

if (inEscape) {
	inEscape = false;
	sb.append(c);
	continue;
}
// MySQL style escapes
if (c == '\\') {
	inEscape = true;
	continue;
}

---

Affects: 4.1.7

Issue Links:

• Support per-script statement separator configuration for SQL scripts [SPR-8817] #13459 Support per-script statement separator configuration for SQL scripts
• Support EOF as statement separator in SQL scripts [SPR-11687] #16310 Support EOF as statement separator in SQL scripts

[spring-projects-issues]

Sam Brannen commented

The default syntax parsing rules for ScriptUtils are not intended to support nested semicolons, since most SQL statements use semicolons as the statement separator.

If your script only contains definitions for triggers, you should be able to simply set the script statement separator to "END;" instead of relying on the default of ";" or "\n".

Please give that a try and let us know if that resolves your issue.

[spring-projects-issues]

Sam Brannen commented

Awaiting feedback from the reporter.

[spring-projects-issues]

Amol Khanolkar commented

Yes but in my case Script is combination of triggers and sql scripts.Which is very much of common usecase
Also any reason for appending escape in Stringbuffer.I believe we can atleast modify code to be like if we are not in single/double quote the mysql escape should actually be ignored and not appended to StringBuffer .

Doing this fixes the issue. What are your thoughts on it ?

[spring-projects-issues]

Sam Brannen commented

Also any reason for appending escape in Stringbuffer?

Yes, there is a very good reason for appending both the escape character '\' and the character immediately following it: this is the required syntax supported by MySQL for string literals. Consult the paragraph beginning with "Within a string, certain sequences..." (preceding Table 9.1 Special Character Escape Sequences) in the MySql reference manual for details.

Your proposal not to append the escaped character would therefore break MySQL support. So that is not an option.

Yes but in my case Script is combination of triggers and sql scripts.Which is very much of common usecase

I understand the desire to include triggers and standard SQL statements within the same script; however, it is currently not possible unless you use a statement separator other than ";" for both triggers and standard statements.

Is that an option for you? Can you introduce a unique statement separator that works for both cases -- for example, something like "@@", "\n\n\n", or ;"\n\n\n"?

I believe we can at least modify code to be like if we are not in single/double quote the mysql escape should actually be ignored and not appended to StringBuffer.

That is a possible improvement for the current code base (i.e., that escaped string literals should only be supported within quoted strings); however, that doesn't mean that the escape character "\" should be completely stripped from scripts outside of string literals.

[spring-projects-issues]

Sam Brannen commented

FYI: when making proposals for changes to ScriptUtils, it is advisable that you ensure that your proposed changes do not break regression tests in ScriptUtilsUnitTests and ScriptUtilsIntegrationTests.

[spring-projects-issues]

Amol Khanolkar commented

Thanks..

You mentioned:

That is a possible improvement for the current code base (i.e., that escaped string literals should only be supported within quoted strings); however, that doesn't mean that the escape character "\" should be completely stripped from scripts outside of string literals.

Any scenario where we should not strip escape character outside string literals?

[spring-projects-issues]

Sam Brannen commented

Any scenario where we should not strip escape character outside string literals?

Yes, the following is valid for SQL Server. See the discussion on Stack Overflow for details.

SELECT TOP 1 *  
FROM [DevServerB\2K5].master.sys.tables

Therefore, I'm afraid we cannot implement your proposal to omit any character following a backslash when encountered outside a singly or doubly quoted string literal.

... unless we add support for treating "[...]" as a string literal, but even then I'm not sure if we wouldn't be breaking other valid use cases for backslashes in SQL scripts.

[spring-projects-issues]

Sam Brannen commented

Based on the current implementation of ScriptUtils, you have two options to achieve your goal:

1. Use a statement separator that works both with your database's SQL import tools directly as well as with ScriptUtils.
   • For example, as I mentioned previously, you could define a convention whereby each statement or trigger is followed by a semicolon and multiple newlines, such as ;"\n\n\n".
2. You can store your triggers and stored procedures in separate scripts from your standard SQL statements.

[spring-projects-issues]

Sam Brannen commented

This issue is related to #13459 and #16310.

[spring-projects-issues]

Sam Brannen commented

Due to the existing support for an EOF_STATEMENT_SEPARATOR that was introduced in ScriptUtils in Spring Framework 4.0.4 (see #16310) and due to the fact that providing a full-blown SQL parser is out of scope for ScriptUtils, I am closing this issue as Won't Fix.