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

Add greenplum DB support #1219

Merged
merged 4 commits into from Mar 30, 2017
Merged

Add greenplum DB support #1219

merged 4 commits into from Mar 30, 2017

Conversation

@gitlabbin
Copy link
Contributor

@gitlabbin gitlabbin commented Mar 1, 2016

No description provided.

README.md Outdated
@@ -4,6 +4,9 @@

![Flyway](https://flywaydb.org/assets/logo/flyway-logo-tm.png "Flyway")

#### Changes log
* Support GreenPlum 4.3.7.1 Database

Copy link
Contributor

@axelfontaine axelfontaine Mar 1, 2016

Choose a reason for hiding this comment

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

No new sections please, just add this to the regular list of databases. Version details go on flywaydb.org

@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented Mar 1, 2016

This needs some cleanup before it can be merged.

Some things to consider:

  • Can you add the tests (MigrationMediumTest + Concurrent one) and make sure they pass?
  • Can you ensure the tests only run with a dedicated Maven profile?
  • Can you also add the url autodetection to the datasource as well as url examples to the flyway.conf file?
  • Do you agree to maintain and support this in the future?
  • Can you also submit a PR with the relevant changes against flyway/flywaydb.org?

Thanks!

@gitlabbin
Copy link
Contributor Author

@gitlabbin gitlabbin commented Mar 14, 2016

Add test cases, mvn profile, jdbc driver autodetections, plus some code cleanup.

-- limitations under the License.
--

ALTER TABLE "${schema}"."${table}" DROP COLUMN "version_rank";
Copy link
Contributor

@axelfontaine axelfontaine Mar 14, 2016

Choose a reason for hiding this comment

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

This file is not necessary. It is only there to allow Flyway 3.x metadata tables to be migrated to the 4.0 format.

Copy link
Contributor

@axelfontaine axelfontaine Feb 8, 2017

Choose a reason for hiding this comment

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

This file should be removed the pull request as per my comment above.

@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented Mar 14, 2016

Thanks. I've added a few comments.

Do you agree to maintain and support this in the future?

@gitlabbin
Copy link
Contributor Author

@gitlabbin gitlabbin commented Mar 14, 2016

@axelfontaine , Happy to support and fix any issue.

README.md Outdated
@@ -16,6 +16,7 @@ Maven, Gradle, Ant and SBT
#### Supported databases
Oracle, SQL Server, SQL Azure, DB2, DB2 z/OS, MySQL, MariaDB, Google Cloud SQL, PostgreSQL, Redshift, Vertica, H2, Hsql, Derby, SQLite


Copy link
Contributor

@axelfontaine axelfontaine Mar 14, 2016

Choose a reason for hiding this comment

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

Can you undo this change or simply add Greenplum to the list?

@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented Mar 14, 2016

Thanks. Can you also submit a PR for the docs against the flyway-4.1 branch of flyway/flywaydb.org ?

Some things to include:

  • update to DB list on the main page
  • DB-specific page
  • updated navigation in the documentation section to include it
  • update to the DB setup contribution page
  • update the documentation and about pages containg the list of supported DBs with link to new greenplum page

@axelfontaine axelfontaine added this to the Flyway 4.1 milestone Mar 14, 2016
@gitlabbin
Copy link
Contributor Author

@gitlabbin gitlabbin commented Mar 16, 2016

Added greenplum driver into the commandline package, the flyway.org documents were finished. pretty much all tasks in your comments were covered

@tygern
Copy link

@tygern tygern commented Jul 22, 2016

I'd love to see this implemented. Can I help in any way to get this branch merged?

Copy link
Contributor

@axelfontaine axelfontaine left a comment

See my inline comments

@@ -55,6 +55,10 @@
<artifactId>jtds</artifactId>
</dependency>
<dependency>
<groupId>com.pivotal.jdbc</groupId>
<artifactId>greenplumdriver</artifactId>
</dependency>
Copy link
Contributor

@axelfontaine axelfontaine Feb 8, 2017

Choose a reason for hiding this comment

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

What's the license on this? Does it allow redistribution?

Copy link
Contributor Author

@gitlabbin gitlabbin Feb 8, 2017

Choose a reason for hiding this comment

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

<id>git-tzolov</id>
<name>tzolov's Git based repo</name>
<url>https://github.com/tzolov/maven-repo/raw/master/</url>
</repository>
Copy link
Contributor

@axelfontaine axelfontaine Feb 8, 2017

Choose a reason for hiding this comment

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

Why the wierd repo? Is there no official one we can get this from?

Copy link
Contributor Author

@gitlabbin gitlabbin Feb 8, 2017

Choose a reason for hiding this comment

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

I can't find official mvn repo for this

Copy link
Contributor

@axelfontaine axelfontaine Feb 8, 2017

Choose a reason for hiding this comment

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

Does this mean it's now using the plain PostgreSQL JDBC driver?

Copy link
Contributor Author

@gitlabbin gitlabbin Feb 8, 2017

Choose a reason for hiding this comment

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

sorry didn't read carefully, for schema management, better use the gp driver.

Copy link
Contributor

@axelfontaine axelfontaine Feb 8, 2017

Choose a reason for hiding this comment

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

OK, and is that one open-source (which license?) and/or available anywhere?

Copy link
Contributor Author

@gitlabbin gitlabbin Feb 8, 2017

Choose a reason for hiding this comment

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

@axelfontaine axelfontaine added this to the Flyway 5.0 milestone Feb 8, 2017
@axelfontaine axelfontaine removed this from the Flyway 4.1 milestone Feb 8, 2017
  - add greenplum jdbc url and the driver autodetection
  - add test case for greenplum
  - add greenplum drvier to parent pom
  - add greenplum jdbc driver into commandline distribution
  - remove upgradeMetaDataTable.sql
@gitlabbin
Copy link
Contributor Author

@gitlabbin gitlabbin commented Feb 8, 2017

upgradeMetaDataTable.sql was deleted.

@axelfontaine axelfontaine added this to the Flyway 4.2.0 milestone Mar 20, 2017
@axelfontaine axelfontaine removed this from the Flyway 5.0.0 milestone Mar 20, 2017
@axelfontaine axelfontaine merged commit 9c8e4f5 into flyway:master Mar 30, 2017
axelfontaine added a commit that referenced this issue Mar 30, 2017
@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented May 18, 2017

@gitlabbin Can you get in touch via email with me? (axel at boxfuse.com) Thanks!

dohrayme pushed a commit to dohrayme/flyway that referenced this issue Feb 3, 2020
dohrayme pushed a commit to dohrayme/flyway that referenced this issue Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants