Add MS SQLServer support to JDBC device registry

[lhotari]

Motivation

• Add Microsoft SQLServer support to JDBC device registry

Changes

• include Microsoft SQLServer driver
• port DDL scripts to SQLServer syntax
  • CREATE TABLE table_name IF NOT EXISTS ->
    IF OBJECT_ID('table_name', 'U') IS NULL
    CREATE TABLE table_name
  • BOOLEAN -> BIT
  • TEXT -> NTEXT (for unicode support)
  • TIMESTAMP -> DATETIME2
  • CHAR/VARCHAR -> NVARCHAR
    • support Unicode
  • char -> varchar to get rid of issues with trailing spaces
• Implement upsert queries for MS SQLServer
  • using MERGE syntax,
    https://docs.microsoft.com/en-us/sql/t-sql/statements/merge-transact-sql
• Test MS SQLServer with Testcontainers
• Use code to accept MS SQL Server Docker image license
  • see https://www.testcontainers.org/modules/databases/mssqlserver/
    for more info

Notice

SQLServer requires SelectMethod=Cursor in JDBC URL. This was needed to support the "SELECT ... FOR UPDATE" syntax used in JDBC device registry.

[kaniyan]

@lhotari Would you mind creating a separate PR for the last 2 commits (Include hono-service-device-registry-jdbc image in pushed images & tenantAdapterStore should use tenantsProperties().getAdapter())? One is a bug and the other extends the script also to push the JDBC device registry image to the Docker Hub. IMHO those can be separately reviewed and merged.

[lhotari]

Would you mind creating a separate PR for the last 2 commits (Include hono-service-device-registry-jdbc image in pushed images & tenantAdapterStore should use tenantsProperties().getAdapter())? One is a bug and the other extends the script also to push the JDBC device registry image to the Docker Hub. IMHO those can be separately reviewed and merged.

@kaniyan thanks for the feedback. I moved those commits out to new PRs #2321 and #2322 as you have suggested.

[kaniyan]

@kaniyan thanks for the feedback. I moved those commits out to new PRs #2321 and #2322 as you have suggested.

Thank your for those PRs and they have been merged.

[ctron]

Looks good to me. With a small note on the naming … and big question mark on the "license acceptance".

[ctron]

I have no idea what this means.

[lhotari]

https://www.testcontainers.org/modules/databases/mssqlserver/ explains it. I'll remove that file and use the code based method for accepting the Docker container EULA.

[lhotari]

@ctron I have address the review comments. Please review the changes.

[ctron]

https://www.testcontainers.org/modules/databases/mssqlserver/ explains it. I'll remove that file and use the code based method for accepting the Docker container EULA.

So that would be a no-go for me. Unless I misunderstood the situation:

Everyone running the Hono tests now would automatically accept this EULA. Which definitely is not the case, as people aren't actively accepting anything when running Hono tests. And we are not confirming that with the end user.

[lhotari]

https://www.testcontainers.org/modules/databases/mssqlserver/ explains it. I'll remove that file and use the code based method for accepting the Docker container EULA.

So that would be a no-go for me. Unless I misunderstood the situation:

Everyone running the Hono tests now would automatically accept this EULA. Which definitely is not the case, as people aren't actively accepting anything when running Hono tests. And we are not confirming that with the end user.

@ctron do you think that it would be possible to accept the EULA in CI (Github Actions Workflow) ? In that case I could modify the default build to skip the MS SQL Server tests unless a certain flag is given.
I'm open for any proposals to get this resolved. I think it would be valuable to have integration tests as part of the Hono CI for MS SQL Server since that's the default DB on Azure.

[ctron]

https://www.testcontainers.org/modules/databases/mssqlserver/ explains it. I'll remove that file and use the code based method for accepting the Docker container EULA.

So that would be a no-go for me. Unless I misunderstood the situation:
Everyone running the Hono tests now would automatically accept this EULA. Which definitely is not the case, as people aren't actively accepting anything when running Hono tests. And we are not confirming that with the end user.

@ctron do you think that it would be possible to accept the EULA in CI (Github Actions Workflow) ? In that case I could modify the default build to skip the MS SQL Server tests unless a certain flag is given.
I'm open for any proposals to get this resolved. I think it would be valuable to have integration tests as part of the Hono CI for MS SQL Server since that's the default DB on Azure.

I have no idea. And I would forward this question to the EMO (Eclipse Management Organization).

[sophokles73]

@lhotari I am not sure about the outcome of the discussion around the SQL Server license issue. Regardless of that, the fixes for the PostgreSQL dialect are very valuable FMPOV. Would you mind splitting the PR up into two parts, one dealing with fixes for PostgreSQL and another one for adding support for SQL server? We could then go on and merge the first PR while dealing with the licensing issues.

@ctron WDYT? Does that make sense to you as well?

[lhotari]

@lhotari I am not sure about the outcome of the discussion around the SQL Server license issue. Regardless of that, the fixes for the PostgreSQL dialect are very valuable FMPOV. Would you mind splitting the PR up into two parts, one dealing with fixes for PostgreSQL and another one for adding support for SQL server? We could then go on and merge the first PR while dealing with the licensing issues.

@ctron WDYT? Does that make sense to you as well?

Yes, I'll remove the SQL server parts. Works for me.

[lhotari]

@sophokles73 @ctron I have moved the Postgres specific parts to a new PR, that is #2375 .

[lhotari]

I have rebased the changes.