Mssql server test fixes

[DavideD]

Down to 100 test failures

[gavinking]

Excellent.

[DavideD]

@tsegismont I'm now having issues with null.
If we created this table:

 create table Basic (
       id int not null,
        dessimal numeric(19,2),
        inteja numeric(19,2),
        string varchar(255),
        version int,
        primary key (id)
    )

and then try to insert a null dessimal, this happens:

Caused by: javax.persistence.PersistenceException: org.hibernate.HibernateException: io.vertx.mssqlclient.MSSQLException: {number=515, state=2, severity=16, message='Cannot insert the value NULL into column 'dessimal', table 'master.dbo.Basic'; column does not allow nulls. INSERT fails.', serverName='55a982ccb425', lineNumber=1}

Any idea?

[gavinking]

@DavideD override Dialect.getNullColumnString() to return " null" as a temporary workaround.

[tsegismont]

@DavideD I had a discussion about this with @gavinking in #744

The issue is tracked upstream by eclipse-vertx/vertx-sql-client#963

[DavideD]

@DavideD override Dialect.getNullColumnString() to return " null" as a temporary workaround.

Thanks all! That worked!
We are now down to 12 test failed on my machine

I've also enabled the build on CI for this branch so that we can can see it the build.

I will continue tomorrow.

[DavideD]

I've also enabled the build on CI for this branch so that we can can see it the build.

Nevermind... there is an issue with testcontainer, probably the JDBC driver is missing. I will check tomorrow

[gavinking]

We are now down to 12 test failed on my machine

Excellent. At that rate we could essentially go ahead and disable the failing tests and ship it.

[DavideD]

@blafond Maybe we can exploit the different time zones and see if you can fix some of the tests while I'm off?

[DavideD]

Test containers works now.

[DavideD]

@tsegismont How do I get the generated id when a column is of type identity?
Example:

    create table EntityWithIdentity (
       id bigint identity not null,
        name varchar(255) null,
        position int null,
        primary key (id)
    )

JDBC returns it in the metadata after an insert but Vert.x returns an empty result instead. Do I need to do a second query?

[DavideD]

@tsegismont I think something is not quite right with datetime2 types:

    create table Basic (
       id int not null,
        alocalDT datetime2 null,
        primary key (id)
    )

With the vert.x client, if I try to run an insert query with the value 2021-06-09T14:26:34.243, what gets saved instead is 9 Jun 2021, 14:30:37.

[tsegismont]

JDBC returns it in the metadata after an insert but Vert.x returns an empty result instead. Do I need to do a second query?

You can use SCOPE_IDENTITY() in a follow-up query.

I haven't tried OUTPUT clause, can you try? If that works, can you file an issue for documentation upstream?

The Pg client has a doc section about retrieving data using RETURNING clause.

[tsegismont]

With the vert.x client, if I try to run an insert query with the value 2021-06-09T14:26:34.243, what gets saved instead is 9 Jun 2021, 14:30:37.

Can you please file a complete reproducer? We have tests for datetime2 (using various precisions/scales), but we may have missed something.

[tsegismont]

@DavideD @gavinking the null column string workaround is no longer needed with 4.1.1-SNAPSHOT

eclipse-vertx/vertx-sql-client#963

[DavideD]

The Pg client has a doc section about retrieving data using RETURNING clause.

Yes, we already use it for Postgres

[DavideD]

OUTPUT doesn't work. I've tried to insert using this query:

insert into EntityWithIdentity (name, position) values (@P1, @P2) OUTPUT INSERTED.id

and it gives me:

javax.persistence.PersistenceException: org.hibernate.HibernateException: io.vertx.mssqlclient.MSSQLException: {number=102, state=1, severity=15, message='Incorrect syntax near 'OUTPUT'.', serverName='40629818ac55', lineNumber=1, additional=[io.vertx.mssqlclient.MSSQLException: {number=8180, state=1, severity=16, message='Statement(s) could not be prepared.', serverName='40629818ac55', lineNumber=1}]}
java.util.concurrent.CompletionException: javax.persistence.PersistenceException: org.hibernate.HibernateException: io.vertx.mssqlclient.MSSQLException: {number=102, state=1, severity=15, message='Incorrect syntax near 'OUTPUT'.', serverName='40629818ac55', lineNumber=1, additional=[io.vertx.mssqlclient.MSSQLException: {number=8180, state=1, severity=16, message='Statement(s) could not be prepared.', serverName='40629818ac55', lineNumber=1}]}

[tsegismont]

The OUTPUT clause must come before VALUES. Can you try:

insert into EntityWithIdentity (name, position) OUTPUT INSERTED.id values (@P1, @P2)

[DavideD]

Nice one! That seems to work

[DavideD]

@tsegismont About datetime2, see eclipse-vertx/vertx-sql-client#977

[DavideD]

Down to 8 tests failing but there are only 3 types of errors remaining:

• datetime2 not storing the value correctly when it's truncated to ms: [mssql] Incorrect encoding of datatime2 for LocalDateTime values to the ms  eclipse-vertx/vertx-sql-client#977
• Something related to the temporary tables: @blafond is looking into it
• An issue with UUID types I haven't check yet (I will have a look at it tomorrow).

Cheers

[blafond]

1. I was able to create temp table with #BooksJS in DbVisualizer and add a row using same create and insert SQL statements as is generated in our tests.
2. I assume the first sql create table #BookJS (id int not null) probably doesn't work nor throw an exception, so when insert into ... is called it can't find the temp table and throws an exception: Invalid object name '#BookJS'.
3. Cloned and searched the vertx-sql-client repo. There is temp table support for MYSQL client, but can't find any reference to tests for temp tables in any of the other clients including vertx-mssql-client

I found this issue that @Gavin King logged last year, but found a work around for Db2.

So vertx development hasn't gotten around to fully supporting temp tables for all dbs. So for now I'd suggest disabling these tests for mssql.

[tsegismont]

An issue with UUID types I haven't check yet

@DavideD guid type is not supported yet, see eclipse-vertx/vertx-sql-client#608 (comment)

@blafond about temporary tables: we've added this to HR and it fixed some tests

hibernate-reactive/hibernate-reactive-core/src/main/java/org/hibernate/reactive/bulk/impl/ReactiveIdTableSupport.java

Lines 30 to 33 in cafef01

	@Override
	public String generateIdTableName(String baseName) {
	return (dialect instanceof SQLServerDialect ? "#" : "ht_") + baseName;
	}

So I'm surprised you found temporary tables cannot be created with the Reactive MS SQL Client. Of course we may have missed something. In this case please file an issue upstream with a reproducer.

[DavideD]

@DavideD guid type is not supported yet, see eclipse-vertx/vertx-sql-client#608 (comment)

Yes, but we store them as bytes. Anyway, the issue is that the converter we are using expects a byte[16] but when we read it back is byte[14] because it's not padded to a binary(255) when saved on the db.

Working on it.

[DavideD]

So I'm surprised you found temporary tables cannot be created with the Reactive MS SQL Client. Of course we may have missed something. In this case please file an issue upstream with a reproducer.

Yes, I don't think that's the issue. We create and select temporary tables several times without problems before reaching the error. It's probably something specific that we are doing with the temporary table in those tests that are failing that it's causing the exception.

[DavideD]

I was able to create temp table with #BooksJS in DbVisualizer and add a row using same create and insert SQL statements as is generated in our tests.

That uses the JDBC driver. Have you tried to run the same queries as native queries with Hibernate Reactive?

[tsegismont]

Yes, but we store them as bytes. Anyway, the issue is that the converter we are using expects a byte[16] but when we read it back is byte[14] because it's not padded to a binary(255) when saved on the db.

Yeah, binary vs varbinary

[DavideD]

Yeah, binary vs varbinary

Well... no... that column on the database is binary(255):

    create table Basic (
       id int not null,
        uuid binary(255) null,
        primary key (id)
    )

I think it's the fact that we convert it using a Buffer.buffer(byte[]). Could it be?

[DavideD]

Yeah, binary vs varbinary

Ah... I understand what you mean now

[DavideD]

@tsegismont Is there a snapshot with this included? eclipse-vertx/vertx-sql-client#963

[DavideD]

@tsegismont Is there a snapshot with this included? eclipse-vertx/vertx-sql-client#963

Never mind, I've just installed it locally.

I've spent some time working on the UUID issue. What's happening is that when we insert the UUID converted to a byte[16], the value in the database is not padded to binary(255).

The column in the database is defined as binary(255) null.

Surprisingly, everything works if I remove [the change about making the columns null]
(8950cd9) and the column on the database is binary(255).

Any idea of what could cause this behaviour?

Note that I've tried to recreate a test for the Vert.x Client but everything seems to work fine with it (with or without the null).
And the error seems related to how the schema is created but I couldn't find any difference between the way the tables have been created.

Anyway... If I use the latest snapshot without the special dialect for Sql Server, it works.
I guess we can just wait for he next release.

[DavideD]

Anyway... If I use the latest snapshot without the special dialect for Sql Server, it works.

It also works when I use the special dialect... well, whatever...

[tsegismont]

@DavideD

Is there a snapshot with this included? eclipse-vertx/vertx-sql-client#963

It seems we have a persistent DB2 test failure which prevents the snapshots from being deployed, will look into this, thanks for the heads-up.

Any idea of what could cause this behaviour?

Perhaps in HR codebase there's an equality test to SqlServerDialect instead of using instanceof?

[DavideD]

Perhaps in HR codebase there's an equality test to SqlServerDialect instead of using instanceof?

Maybe... but I don't think so.

[DavideD]

I think @blafond was right.
@tsegismont, I've tried this test: eclipse-vertx/vertx-sql-client#979

But it's very possible I did something wrong.

[DavideD]

It seems that now we have a version that passes all the tests (using the latest Vert.x Snapshot).

I still have to clean it up and refactor some changes but we should be close.

Thank you all!

[gavinking]

Awesome, that's really great news!!

[tsegismont]

Cool! In case you missed it, I merged the batch queries support today: eclipse-vertx/vertx-sql-client#985

…

[DavideD]

Thanks @tsegismont. I will have a look at it shortly

[DavideD]

@tsegismont batching seems to pass the tests fine. Thanks

Closing this pr and sending a new one in a bit

[tsegismont]

Great news!

…