Skip to content
This repository has been archived by the owner on Nov 23, 2021. It is now read-only.

#671: add support for hana as dbType #671

Merged
merged 3 commits into from
Sep 11, 2018
Merged

#671: add support for hana as dbType #671

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bba240f
#671: add support for hana as dbType
hohwille Aug 16, 2018
a1c84e3
#671: rewview rework
hohwille Aug 20, 2018
0e8b401
#671: included review improvement suggestions
hohwille Sep 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions templates/server/src/main/resources/archetype-resources/core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@
<groupId>org.mariadb.jdbc</groupId>
<artifactId>mariadb-java-client</artifactId>
<version>1.5.4</version>
#elseif ($dbType == 'hana')
<groupId>com.sap.cloud.db.jdbc</groupId>
<artifactId>ngdbc</artifactId>
<version>2.3.48</version>
#elseif ($dbType == 'oracle')
<groupId>com.oracle.jdbc</groupId>
<artifactId>ojdbc8</artifactId>
Expand Down
4 changes: 4 additions & 0 deletions ...ver/src/main/resources/archetype-resources/core/src/main/resources/application.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ spring.datasource.username=${rootArtifactId}
# spring.jpa.database-platform=org.hibernate.dialect.MySQL5Dialect
# spring.datasource.driver-class-name=org.mariadb.jdbc.Driver
spring.datasource.username=${rootArtifactId}
#elseif ($dbType == 'hana')
Copy link
Contributor

@AbhayChandel AbhayChandel Aug 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Property spring.jpa.database is set to hana, which is not a valid value. As of now hana database cannot be set explicitly using this property. So from the list of available values only DEFAULT seems to be a sensible choice and using it Hibernate is able to resolve the database correctly. So it is better to set this property to DEFAULT

Copy link

Choose a reason for hiding this comment

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

Setting HANA via spring.jpa.database will be possible starting with Spring 5.1 (see Jira item https://jira.spring.io/browse/SPR-16460)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for all your valuable feedback. Also great seeing how hana evolves and becomes first class citizen by all frameworks 👍
With this information, I would suggest not to change anything. We just leave it like this and it will get fixed in the future when the update to spring 5.1 takes place.

Copy link
Contributor

@AbhayChandel AbhayChandel Aug 31, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@hohwille but with the current setting application fails to startup. So until Spring 5.1 is available we need to provide a valid value that works for Hana.

# spring.jpa.database-platform=org.hibernate.dialect.HANAColumnStoreDialect
# spring.datasource.driver-class-name=com.sap.db.jdbc.Driver
spring.datasource.username=${rootArtifactId}
#elseif ($dbType == 'oracle')
# spring.jpa.database-platform=org.hibernate.dialect.Oracle10gDialect
# spring.jpa.database-platform=org.hibernate.dialect.Oracle12cDialect
Expand Down
3 changes: 3 additions & 0 deletions .../main/resources/archetype-resources/core/src/main/resources/config/application.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ spring.datasource.url=jdbc:mysql://address=(protocol=tcp)(host=localhost)(port=3
#elseif ($dbType == 'mariadb')
spring.datasource.password=todo
spring.datasource.url=jdbc:mariadb://localhost:3306/db
#elseif ($dbType == 'hana')
spring.datasource.password=todo
spring.datasource.url=jdbc:sap://localhost:39015
Copy link
Contributor

Choose a reason for hiding this comment

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

As explained here, there is variation in port number as per the version of database. I would suggest not to hard code the port number and let the user figure out what port number to use. Otherwise chances are that users(especially junior members of the team) will be confused why he/she is getting connection error without realizing that the port number configured is not correct for their setup.

Copy link

Choose a reason for hiding this comment

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

39015 is the default port of the HANA express cloud and VM images. IMHO this is a sensible default. I'd just add a comment how to adapt it for other HANA installations.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks also for this information. We intentionally configure this here as this is the configuration to be used by developers in the development environment. This config file will not be included into the project result artifact (WAR or JAR). However, all developers should have a common setup for all their machines to simplify. When the default port number is not suitable for a project, it can be easily changed after the project template has been instantiated.
@breglerj Do you have a recommended link to a guide how to adapt it for other HANA installations? I would add this as a comment then.

Copy link

Choose a reason for hiding this comment

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

@hohwille: Here is a link to the client documentation that describes which port to use: https://help.sap.com/viewer/0eec0d68141541d1b07893a39944924e/latest/en-US/b250e7fef8614ea0a0973d58eb73bda8.html

#elseif ($dbType == 'oracle')
spring.datasource.password=todo
spring.datasource.url=jdbc:oracle:thin:@localhost:1521/XE
Expand Down
18 changes: 18 additions & 0 deletions ...urces/archetype-resources/core/src/main/resources/db/type/hana/V0001__Create_Sequence.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-- Leave a large ID space reserved for master-data and test-data
CREATE SEQUENCE HIBERNATE_SEQUENCE START WITH 1000000;

-- hana does not support Dateadd function out of the box so we add it here to be able to use it for master-data SQLs
CREATE FUNCTION DATEADD(IN DATETYPE NVARCHAR(256), IN NUMBER INTEGER, IN TS TIMESTAMP)
RETURNS TSADD TIMESTAMP
AS
BEGIN
IF :DATETYPE = 'DAY'
THEN
TSADD = ADD_DAYS(:TS, :NUMBER);
ELSEIF :DATETYPE = 'HOUR'
THEN
TSADD = ADD_SECONDS(:TS, :NUMBER * 3600);
ELSE
SIGNAL SQL_ERROR_CODE 10000 SET MESSAGE_TEXT = 'Unsupported date type: ' || :DATETYPE;
END IF;
END;
7 changes: 7 additions & 0 deletions ...ources/archetype-resources/core/src/main/resources/db/type/hana/V0002__Create_RevInfo.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- *** RevInfo (Commit log for envers audit trail) ***
CREATE COLUMN TABLE RevInfo(
id BIGINT NOT NULL GENERATED BY DEFAULT AS IDENTITY (START WITH 1),
"timestamp" BIGINT NOT NULL,
Copy link

@breglerj breglerj Aug 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Unquoted columns will be upper-cased while quoted columns will be mixed-case. So for consistency reasons you should name this column "TIMESTAMP".

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason here is that we had to quote this property also in the JPA annotation as this is a reserved keyword in Oracle.
https://github.com/oasp/oasp4j/blob/develop/modules/jpa-basic/src/main/java/io/oasp/module/jpa/dataaccess/api/AdvancedRevisionEntity.java#L41

For some reasons (case-sensitivity) in DDL for most databases this property then also has to be quoted in the same way. To keep this consistent we do it the same way for the DDLs of all database types.

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW: Using "TIMESTAMP" in AdvancedRevisionEntity.java would indeed have been the better option for 2.6.x. Changing this again? Or leave it as is?

userLogin VARCHAR(255),
PRIMARY KEY (ID)
);
9 changes: 9 additions & 0 deletions ...s/archetype-resources/core/src/main/resources/db/type/hana/V0003__Create_BinaryObject.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
-- *** BinaryObject (BLOBs) ***
CREATE TABLE BinaryObject (
Copy link

Choose a reason for hiding this comment

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

CREATE COLUMN TABLE BinaryObject (

Copy link
Member Author

Choose a reason for hiding this comment

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

Good finding. I will fix that asap.

id BIGINT NOT NULL GENERATED BY DEFAULT AS IDENTITY,
modificationCounter INTEGER NOT NULL,
content CLOB,
Copy link

Choose a reason for hiding this comment

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

The data type should be BLOB.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. I will fix that and hope it works with @lob annotation and hibernate validation.

filesize BIGINT NOT NULL,
mimeType VARCHAR(255),
CONSTRAINT PK_BinaryObject_id PRIMARY KEY(ID)
Copy link

Choose a reason for hiding this comment

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

This could just be PRIMARY KEY(ID) if you don't need an explicit name.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thing was that at least in Oracle we often get constraint violations that do not tell the context in the error message. However, we always get the name of the constraint in the error message. If no name is given, then an autogenerated name will be choose what is kind of random and hence you still do not know what is wrong in case of an error.
Therefore we defined strict naming conventions as best practices for all constraints:
https://github.com/oasp/oasp4j/wiki/guide-sql#ddl

In general you are right that functionally this is just clutter.

);