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

[WIP] Add support for SAP HANA #346

Closed
wants to merge 6 commits into from
Closed

Conversation

MitchK
Copy link

@MitchK MitchK commented Feb 2, 2016

NOT READY FOR MERGE

I'm currently developing a HANA driver, as I have found no schema migration tools for HANA so far where migrations can be written in version-able code.

Based on this documentation: https://help.sap.com/saphelp_hanaplatform/helpdata/en/20/9ce8cd75191014bcd59c2b379a17c9/content.htm?frameset=/en/20/a6791f751910148d2bfe3814192a01/frameset.htm&current_toc=/en/2e/1ef8b4f4554739959886e55d4c127b/plain.htm&node_id=199&show_children=true#jump200

Ground work

  • Base integration with the hdb driver for Node.js works already.

Map data types

  • Map basic data types
  • Map geo spatial types

Tables

ALTER TABLE

  • ALTER TABLE
  • ALTER VIRTUAL TABLE (Functionality not planned for first release)

CREATE TABLE

  • CREATE TABLE
  • table_type
  • table_contents_source
  • logging_option
  • auto_merge_option
  • unload_priority_clause
  • schema_flexibility_option
  • partition_clause
  • group_option
  • location_clause
  • replica_clause
  • remote_location_clause
  • remote_property_clause
  • global_temporary_option
  • series_clause
  • unused_retention_period_option
  • asynchronous_replica_clause

RENAME TABLE

  • RENAME TABLE

DROP TABLE

  • DROP TABLE

OTHER

  • COMMENT ON (Functionality not planned for first release)

Indices

CREATE INDEX

  • CREATE INDEX
  • CREATE FULLTEXT INDEX

DROP INDEX

  • DROP INDEX
  • DROP FULLTEXT INDEX

ALTER INDEX

  • ALTER FULLTEXT INDEX
  • ALTER INDEX

RENAME INDEX

  • RENAME INDEX

Schemas

CREATE SCHEMA

  • CREATE SCHEMA

DROP SCHEMA

  • DROP SCHEMA

Columns

RENAME COLUMN

  • RENAME COLUMN

Sequences

CREATE SEQUENCE

  • CREATE SEQUENCE

DROP SEQUENCE

  • DROP SEQUENCE

ALTER SEQUENCE

  • ALTER SEQUENCE

Views

  • CREATE VIEW (Functionality not planned for first release)
  • ALTER VIEW (Functionality not planned for first release)
  • DROP VIEW (Functionality not planned for first release)

Statistics

  • CREATE STATISTICS (Functionality not planned for first release)
  • REFRESH STATISTICS (Functionality not planned for first release)
  • DROP STATISTICS (Functionality not planned for first release)

Synonyms

  • CREATE SYNONYM (Functionality not planned for first release)
  • DROP SYNONYM (Functionality not planned for first release)

Triggers

  • CREATE TRIGGER (Functionality not planned for first release)
  • DROP TRIGGER (Functionality not planned for first release)

@wzrdtales wzrdtales changed the title Add support for SAP HANA [WIP] Add support for SAP HANA Feb 3, 2016
@wzrdtales
Copy link
Member

Great! If you need assistance or have questions feel free to ask.

Some notes about this WIP PR, only officially supported drivers are included in the main project, there are some things that need to be done for this:

  • If you want to have this driver officially supported, we may move this into the db-migrate organisation if you don't mind it. I would add you as team member and owner of this driver within the db-migrate organisation.
  • Driver must be testable
  • Driver must be supportable (if you want to help out on answering issues and supporting it would greatly welcome this!)
  • Default API must be supported, unless not possible in any way
  • Driver specific documentation should be added to the docs

Thank you for taking time to create a new driver, great to see this!

@MitchK
Copy link
Author

MitchK commented Feb 3, 2016

Sure! I'd be very excited. Let me test some things first and try to make the most important use cases work first. I will mostly stick to your MySQL implementations as reference.

I also work for SAP so I can also give support for this driver.

I would also need to check with my legal department (SAP) first before submitting the contribution, but that is usually just formality as my company is very inbound/outbound open-source friendly/driven.

I might be also able to provide with a small or shared HANA test instance on SAP HANA Cloud Platform, as HANA is not very cheap to operate for this use case (32GB RAM is the minimum requirement). I can also provide with preparations for Travis CI.

@MitchK MitchK mentioned this pull request Feb 3, 2016
@MitchK
Copy link
Author

MitchK commented Feb 3, 2016

Added checklist items

@wzrdtales
Copy link
Member

Sounds good, as soon as you have checked back with your legal department just let me know and we will get on the rest of the details.

If any questions arise I'm almost always here to help.

@MitchK
Copy link
Author

MitchK commented Feb 4, 2016

@wzrdtales
How can I extend the API not just with own implementations but also with new functionality, such as createSequence(...). Does this require an API change?

This does not work:

  someNewFunctionality: function (callback) {
    callback();
  },

  createTable: function(tableName, options, callback) {
    this.someNewFunctionality(function() {
      console.log('test');
    })
  });

Returns

[ERROR] TypeError: this.someNewFunctionality is not a function

@wzrdtales
Copy link
Member

To have a separation between seeders and migrations there are interfaces: https://github.com/db-migrate/node-db-migrate/blob/master/lib/interface/migratorInterface.js

As soon as you add driver specific features, you need to extend the seeder or migration interface.
You get automatically passed the extensions objects to the driver: https://github.com/db-migrate/node-db-migrate/blob/master/lib/driver/index.js#L29-L33
This is an example of this here: https://github.com/db-migrate/mysql/blob/master/index.js#L401-L413

@MitchK
Copy link
Author

MitchK commented Feb 4, 2016

Thank you @wzrdtales I'll take a look into it.

@wzrdtales
Copy link
Member

Also one important note:
There is also a special treatment of _functions which are considered to be "private" in node js. Those are also always included and you don't need to specify them in the extensions, as those functions are anyway not intended for the use through the user and aren't documented in the API docs and thus a user might use them in the knownledge the could get broken at any time without a warning as they are for internal purposes only.

@MitchK
Copy link
Author

MitchK commented Feb 4, 2016

Oh, I see. Thanks. Let me fix this.

@MitchK
Copy link
Author

MitchK commented Feb 4, 2016

By the way, functionality like createSequence is something that could be shared across drivers. Oracle, for example, uses sequences as well. There's no Oracle driver yet, but this is definitely that you can keep in mind.

@wzrdtales
Copy link
Member

The top level should be for the generic ones, would need to have a look of how common this is over multiple dbmss and then decide, but might be an option to think of.

@MitchK
Copy link
Author

MitchK commented Feb 4, 2016

Thanks @wzrdtales Works like a charm!

I think it would be best to move the driver to the https://github.com/SAP org, because we need to use our CLA in order to allow contributions due to IP topics. We usually do this through https://cla-assistant.io/

I would also not recommend to make the HANA driver a default included package at this point yet.

I am required to release it as under Apache 2.0 as far as I know, because I use the hdb dependency released under Apache 2.0 as well. Otherwise we would need to include a NOTICE file with a reference to SAP and you might as well.

Furthermore, the driver might not very stable in the beginning, so I would like to collect feedback from HANA devs first before including it on a broader scope.

@wzrdtales
Copy link
Member

I can license db-migrate different from any driver, they're just drivers which can be plugged in, which itself can use whatever license they want, as long as the licenses allow them to. In this case the drivers are created for db-migrate not the other way around and db-migrate does not depend on them, but can use them.

And I'm not sure if you really need to license with the Apache 2.0 license just because of the driver behind it, I would have to take a look at it, but I think they allow the usage from differently licensed products like LGPL does. But I could also be wrong!

This is one of the reasons why I mostly always choose licenses that are friendly and less restrictive like MIT or the LGPL. This depends on the project though.

Unfortunately, if we can't move the driver into the db-migrate organisation, it wont ever get a officially supported one. But I want to add a community driver listing, to give users a better overview over available drivers.

@wzrdtales
Copy link
Member

In fact to talk a bit more about the licenses: Actually using db-migrate is under the terms of MIT, this one is not very restrictive. It now depends on which driver the end user uses, which license he will deal with. This gets only a problem as soon as someone wants to use db-migrate as a programable module in his project. In this case this project will need to fit with the drivers licenses that are used. But that shouldn't be a problem, as they probably already use the drivers that also the db-migrate drivers use, to make their application work.

About Apache 2.0: https://tldrlegal.com/license/apache-license-2.0-%28apache-2.0%29

Apache 2.0 is thus a bit longer than MIT, but still compatible. Thus you must not license your driver under Apache 2.0 if you don't want to.

But I'm currently not sure if you need to redistribute the NOTICE file in your project. Thats a bit tricky, it is theoretically always delivered via the npm package management. I would also need to consult lawyer to answer this question.
I haven't seen any project that is shipping all NOTICE files of the apache 2 licensed modules though, but that does not mean that this is correct.

@wzrdtales
Copy link
Member

As a quick side info, that are all the licenses that db-migrate used modules itself, without drivers, builds on:
LICENSES: (BSD-2-Clause OR MIT OR Apache-2.0), Apache, Apache 2.0, Apache License, Version 2.0, Apache-2.0, BSD, BSD*, BSD-2-Clause, BSD-3-Clause, BSD-3-Clause AND MIT, BSD-like, FreeBSD, ISC, MIT, MIT License, MIT/X11, MPL, Modified MIT / BSD, Apache 2.0, WTFPL

@wzrdtales
Copy link
Member

Closing :) If any questions hmu.

Drivers are not part of the core.

@wzrdtales wzrdtales closed this Nov 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants