Google Code skipped issue 41 about flyway HOT 11 CLOSED

From [email protected] on October 27, 2010 15:12:25
Hi,

I have created a clone at 26/10/2010 and added PostgreSQL support to flyway.
The clone is available at
https://code.google.com/r/godhood-flyway-postgresql/

All test apart from the failedMigration() test pass just fine.
However, the I think there is a design flaw in the failedMigration test or flyway itself (I didn't look into that and you might be able to tell right away):

PostgreSQL supports DDL transactions.
The failedMigration test starts with a clean database and issues some false statements identified by Version 1. As these statements were bad, the transaction is rolled back and there is no row in the schema_version table. The test tries to get the current version --> NullPointerException, as there is no version yet.

The test actually works fine if you change PostgreSQLDbSupport to NOT support DDL Transactions.

It would be great to see this coming into the main flyway pretty soon.
btw: I cleaned up the imports in flyway-core and didn't realize that all Eclipse was doing was actually reordering them - sorry.

I believe the only "real" changes I made are:

• to the POMs of the parent and core
• changed src/main/java/com/googlecode/flyway/core/dbsupport/DbSupportFactory
• added src/main/java/com/googlecode/flyway/core/dbsupport/postgresql
• added src/main/resources/com/googlecode/flyway/core/dbsupport/postgresql
• added src/test/java/com/googlecode/flyway/core/dbsupport/postgresql
• added src/test/resources/migration/postgresql

Thanks!
Philipp

from flyway.

From [email protected] on October 27, 2010 16:44:54
Hello Philipp !

Thanks! That's great news as we were planning to address this issue in November! Unfortunately I don't have time this evening or tomorrow, but I'll have a look at your code this weekend. If it's fine, I'll merge it and release a new version.

I'll have a look at the failing test as well. It's possible that it might have to be adjusted as PostgrSQL is the first DB that Flyway supports that actually handles ddl transactions.

I'll also look at the SqlParser, to see how it should be extended to support PostgreSQL stored procedures like

CREATE FUNCTION getQtyOrders(customerID int) RETURNS int AS $$
DECLARE
qty int;
BEGIN
SELECT COUNT(*) INTO qty
FROM Orders
WHERE accnum = customerID;
RETURN qty;
END;
$$ LANGUAGE plpgsql;

It shouldn't be that hard though.

BTW: Don't worry about Eclipse. Christian and I both work with IntelliJ and virtually gave up trying to align formatter settings with Eclipse. I'll look at the files you mentioned. It should be fine.

Cheers,
Axel

from flyway.

From [email protected] on October 27, 2010 17:20:07
Hi Axel,

sounds great!
As for the SQL parser:
I initially thought about changing all of that flyway was actually trying to understand the SQL and find the end of an SQL statement by counting ; and watch for other keywords, but was eventually too lazy to do it. In the commited version I just sticked to the way it was implemented for Oracle (ie. trailing /) - btw: is this actually officially Oracle or is it just made up by you?

I did however put quite some effort into detecting the arguments of existing functions when cleaning the database: the code will drop both, someFunc(integer) and someFunc(integer, integer)

from flyway.

From [email protected] on October 27, 2010 17:29:09
Hi Philipp,

No, I didn't invent the Oracle syntax :-) This trailing / convention is supported by a lot of tools, including Oracle's official command line client SqlPlus.

Flyway actually doesn't really care about the actual sql. It pretty much only looks for delimiters to divide the character stream into statements.

For PostgreSQL it looks as if a bit of analysis will be required as the delimiters are variable (can be $$, $body$, ...), and part of the actual statement itself. Let's see... (I hope a lazy solution will do ;-) )

Cheers,
Axel

from flyway.

From [email protected] on October 27, 2010 17:51:15
:(
Well, the / is not supported by psql and AFAIK there is no way to tell postgres to use another delimiter during a session.

I have looked at this a bit and I believe the only way to detect this properly is to count DECLARE, BEGIN, END and ; and count the depth level. $$ is only used for changing the quoting character and is not needed anyway - these are all valid:

CREATE FUNCTION add(integer, integer) RETURNS integer
AS 'select $1 + $2;'
LANGUAGE SQL
IMMUTABLE
RETURNS NULL ON NULL INPUT;

CREATE FUNCTION add(integer, integer) RETURNS integer LANGUAGE SQL
AS 'select $1 + $2;'
IMMUTABLE
RETURNS NULL ON NULL INPUT;

CREATE OR REPLACE FUNCTION increment(i integer) RETURNS integer AS $$
BEGIN
RETURN i + 1;
END;
$$ LANGUAGE plpgsql;

ie. you cannot rely on 'LANGUAGE .+;' at the end or $$ or $.*$

Have fun :)
Philipp

from flyway.

From [email protected] on October 29, 2010 12:41:20
Hello Philipp,

Will you need support for stored procedures in your project? And if so, would a subset of the syntax such as only supporting one type of delimiter ($$ for example) be enough as a start?

Cheers,
Axel

from flyway.

From [email protected] on October 29, 2010 13:08:17
Hi Axel,

as for me: I don't need any procedures in my project at all...

I just checked how pg_dump creates the functions (v. 9.0). I think that using a regular expression such as "$[A-Za-z0-9-]*$" will catch most (if not all) cases for the SQL produced by pg_dump.
In the few tests I did, pg_dump was either using $$ or $$ as delimiter. The procedure definition always ended with ';' ie:

CREATE FUNCTION add(integer, integer) RETURNS integer
LANGUAGE sql IMMUTABLE STRICT
AS $$select $1 + $2;$$;

or

CREATE FUNCTION inc(i integer) RETURNS integer
LANGUAGE sql
AS $$SELECT * FROM test1$$;

Best,
Philipp

from flyway.

From [email protected] on October 31, 2010 21:16:07
Hello Philipp,

I've committed all the necessary code for PostgreSQL support, including a parser extension to deal with functions that have the pg_dump syntax.

Could you please give it a shot and tell me if it works for you (don't forget to test the clean functionality as well)?

If it does, I'll release the new version in the next few days.

Cheers,
Axel

from flyway.

From [email protected] on November 01, 2010 22:41:57
Hi Axel,

looks great so far!
One thing I noticed while testing is that SqlStatement.execute should probably call jdbcTemplate.execute(sql) instead of jdbcTemplate.update(sql)
Especially, since with .update() you cannot do ie. SELECT setval('sequence', 400)

But other than that, it looks great so far.
Best
Philipp

from flyway.

From [email protected] on November 02, 2010 19:26:12
Hi Philipp,

thanks for your feedback. We are currently planning a release for Thursday night. If anything comes up until then, please let us know. I will look into the fix for the jdbcTemplate.execute(sql) issue. If it doesn't break anything, which I don't think it will, I'll include it as well.

Cheers,
Axel

from flyway.

From [email protected] on February 10, 2011 21:10:47
Better late, than never: Credit were credit is due. I just added godhood to the list of contributers for Flyway. Thanks again for making PostgreSQL support a reality !

from flyway.