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

#1531: Fix for CREATE PROCEDURE in SAP HANA dialect #1532

Merged
merged 9 commits into from Mar 3, 2017
Expand Up @@ -15,6 +15,7 @@
*/
package org.flywaydb.core.internal.dbsupport.saphana;

import org.flywaydb.core.api.FlywayException;
import org.flywaydb.core.internal.dbsupport.Delimiter;
import org.flywaydb.core.internal.dbsupport.SqlStatementBuilder;
import org.flywaydb.core.internal.util.StringUtils;
Expand All @@ -24,14 +25,10 @@
*/
public class SapHanaSqlStatementBuilder extends SqlStatementBuilder {
/**
* Are we currently inside a BEGIN END; block?
* Are we currently inside a BEGIN END; block? How deeply are begin/end nested?
*/
private boolean insideBeginEndBlock;

/**
* Holds the beginning of the statement.
*/
private String statementStart = "";
private int beginEndNestedDepth = 0;
private String statementStartNormalized = "";

@Override
protected String cleanToken(String token) {
Expand All @@ -44,24 +41,56 @@ protected String cleanToken(String token) {

@Override
protected Delimiter changeDelimiterIfNecessary(String line, Delimiter delimiter) {
if (StringUtils.countOccurrencesOf(statementStart, " ") < 4) {
statementStart += line;
statementStart += " ";

// need only accumulate 16 characters of normalized statement start in order to determine if it is an 'interesting' statement
if (statementStartNormalized.length() < 16) {
final String effectiveLine = cutCommentsFromEnd(line);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change actually breaks the case where CREATE and PROCEDURE are on separate lines

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found the approach useful to determine the start of the statement we're building. But it only worked in very limited cases. I tried to extend it by ignoring multiple blank lines, line comments and the beginning of block comments.

I also added tests specifically for the case you mentioned.

statementStartNormalized += effectiveLine + " ";
statementStartNormalized = StringUtils.trimLeadingWhitespace(StringUtils.collapseWhitespace(statementStartNormalized));
}
boolean insideStatementAllowingNestedBeginEndBlocks =
statementStartNormalized.startsWith("CREATE PROCEDURE")
|| statementStartNormalized.startsWith("CREATE FUNCTION")
|| statementStartNormalized.startsWith("CREATE TRIGGER");

if (statementStart.startsWith("CREATE TRIGGER")) {
if (insideStatementAllowingNestedBeginEndBlocks) {
if (line.startsWith("BEGIN")) {
insideBeginEndBlock = true;
beginEndNestedDepth++;
}

if (line.endsWith("END;")) {
insideBeginEndBlock = false;
beginEndNestedDepth--;
if (beginEndNestedDepth < 0) {
throw new FlywayException("Syntax error in SQL script: unpaired END; statement");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't worry so much about this. A warning is probably enough here. If the script is incorrect (in itself, not due to our parsing) the DB will tell the user.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I replaced the exception with a warning in the logs,
Glad to see the pull request was added to the upcoming milestone.

} else if (beginEndNestedDepth == 0) {
insideStatementAllowingNestedBeginEndBlocks = false;
}
}
}

if (insideBeginEndBlock) {
if (insideStatementAllowingNestedBeginEndBlocks) {
return null;
}
return getDefaultDelimiter();
}

private static String cutCommentsFromEnd(String line) {
if (null == line) {
return line;
}
final int beginOfLineComment = line.indexOf("--");
final int beginOfBlockComment = line.indexOf("/*");
if (-1 != beginOfLineComment) {
if (-1 != beginOfBlockComment) {
return line.substring(0, Math.min(beginOfBlockComment, beginOfLineComment));
} else {
return line.substring(0, beginOfLineComment);
}
} else {
if (-1 != beginOfBlockComment) {
return line.substring(0, beginOfBlockComment);
}
}
return line;
}
}
Expand Up @@ -16,13 +16,18 @@
package org.flywaydb.core.internal.dbsupport.saphana;

import org.flywaydb.core.internal.util.StringUtils;
import org.flywaydb.core.internal.util.scanner.classpath.ClassPathResource;
import org.junit.Test;

import java.util.ArrayList;
import java.util.List;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;


/**
* Test for PostgreSQLSqlStatementBuilder.
* Test for SapHanaSqlStatementBuilder
*/
public class SapHanaSqlStatementBuilderSmallTest {
/**
Expand Down Expand Up @@ -77,4 +82,43 @@ public void temporalString() {

assertTrue(statementBuilder.isTerminated());
}

@Test
public void parseSimpleProcedure() {
parseProcedureWithEmbeddedSemicola("org/flywaydb/core/internal/dbsupport/saphana/procedures/procedure.sql", 3);
}

@Test
public void parseProcedureWithNestedBlocks() {
parseProcedureWithEmbeddedSemicola("org/flywaydb/core/internal/dbsupport/saphana/procedures/procedureWithNestedBlocks.sql", 3);
}
@Test
public void parseProcedureInComment() {
parseProcedureWithEmbeddedSemicola("org/flywaydb/core/internal/dbsupport/saphana/procedures/procedureInComment.sql", 3);
}
@Test
public void parseProcedureWithComments() {
parseProcedureWithEmbeddedSemicola("org/flywaydb/core/internal/dbsupport/saphana/procedures/procedureWithComments.sql", 3);
}
@Test
public void parseProcedureWithLinebreaks() {
parseProcedureWithEmbeddedSemicola("org/flywaydb/core/internal/dbsupport/saphana/procedures/procedureWithLinebreaks.sql", 3);
}

private void parseProcedureWithEmbeddedSemicola(String resourceName, int numStatements) {
final String sqlScriptSource =
new ClassPathResource(resourceName, SapHanaSqlStatementBuilderSmallTest.class.getClassLoader())
.loadAsString("UTF-8");
SapHanaSqlStatementBuilder statementBuilder = new SapHanaSqlStatementBuilder();
String[] lines = StringUtils.tokenizeToStringArray(sqlScriptSource, "\n");
final List<String> statements = new ArrayList<String>();
for (String line : lines) {
statementBuilder.addLine(line);
if (statementBuilder.isTerminated()) {
statements.add(statementBuilder.getSqlStatement().getSql());
statementBuilder = new SapHanaSqlStatementBuilder();
}
}
assertEquals("Number of recognized statements", numStatements, statements.size());
}
}
@@ -0,0 +1,27 @@
-- just to have one statement before the actual test statement
CREATE VIEW all_misters AS SELECT * FROM test_user WHERE name LIKE 'Mr.%';

-- create statement with embedded ';'
-- it is expected that this statement is read as a single statement ending after 'END;'
-- and not broken into separate statements ending after each ';' withing the BEGIN...END-block.
-- this example statement is borrowed from the HANA reference for
-- CREATE PROCEDURE (http://help-legacy.sap.com/saphelp_hanaplatform/helpdata/en/20/d467407519101484f190f545d54b24/content.htm)
CREATE PROCEDURE orchestrationProc
LANGUAGE SQLSCRIPT AS
BEGIN
DECLARE v_id BIGINT;
DECLARE v_name VARCHAR(30);
DECLARE v_pmnt BIGINT;
DECLARE v_msg VARCHAR(200);
DECLARE CURSOR c_cursor1 (p_payment BIGINT) FOR
SELECT id, name, payment FROM control_tab
WHERE payment > :p_payment ORDER BY id ASC;
CALL init_proc();
OPEN c_cursor1(250000);
FETCH c_cursor1 INTO v_id, v_name, v_pmnt; v_msg := :v_name || ' (id ' || :v_id || ') earns ' || :v_pmnt || ' $.';
CALL ins_msg_proc(:v_msg);
CLOSE c_cursor1;
END;

-- just to have a trailing statement for testing
DROP VIEW all_misters;
@@ -0,0 +1,11 @@
-- just to have one statement before the actual test statement
CREATE VIEW all_misters AS SELECT * FROM test_user WHERE name LIKE 'Mr.%';

-- a statement with a comment containing the Strings CREATE PROCEDURE and BEGIN
CREATE TABLE
-- could as well be achieved by CREATE PROCEDURE get_misters
-- BEGIN
test_user (name VARCHAR(100));

-- just to have a trailing statement for testing
DROP VIEW all_misters;
@@ -0,0 +1,14 @@
-- just to have one statement before the actual test statement
CREATE VIEW all_misters AS SELECT * FROM test_user WHERE name LIKE 'Mr.%';

-- a statement with line break and interspersed comment between CREATE and PROCEDURE
CREATE
-- my special
PROCEDURE nested_block(OUT val INT) LANGUAGE SQLSCRIPT
READS SQL DATA AS
BEGIN
DECLARE a INT = 1;
END;

-- just to have a trailing statement for testing
DROP VIEW all_misters;
@@ -0,0 +1,13 @@
-- just to have one statement before the actual test statement
CREATE VIEW all_misters AS SELECT * FROM test_user WHERE name LIKE 'Mr.%';

-- a statement with line break between CREATE and PROCEDURE
CREATE
PROCEDURE nested_block(OUT val INT) LANGUAGE SQLSCRIPT
READS SQL DATA AS
BEGIN
DECLARE a INT = 1;
END;

-- just to have a trailing statement for testing
DROP VIEW all_misters;
@@ -0,0 +1,23 @@
-- just to have one statement before the actual test statement
CREATE VIEW all_misters AS SELECT * FROM test_user WHERE name LIKE 'Mr.%';

-- a procedure with nested begin-end; blocks
-- also taken from
-- SAP HANA SQLScript Reference - SAP Library at:
-- http://help-legacy.sap.com/saphelp_hanaplatform/helpdata/en/f0/a6dceac8b94cca98dd2741ac6541b8/content.htm?frameset=/en/c6/558a64245942ebb52f6a6ddb3e4278/frameset.htm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see an example with nested blocks there. Where did you see it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I used the "share" link on the SAP Help page, which seems not to include the full link.
I updated the link in file.

CREATE PROCEDURE nested_block(OUT val INT) LANGUAGE SQLSCRIPT
READS SQL DATA AS
BEGIN
DECLARE a INT = 1;
BEGIN
DECLARE a INT = 2;
BEGIN
DECLARE a INT;
a = 3;
END;
val = a;
END;
END;

-- just to have a trailing statement for testing
DROP VIEW all_misters;