Fix postgres symbol tests failure #565

javornikolov · 2017-03-19T14:41:38Z

Unify Multipy function across environments, use MultiplyIO name where an inout parameter has been used
Unify CalcLength function so that it can be reused across acceptance tests of all environments
Add support for PostgreSQL OUT and INOUT parameters
Fix Symbols acceptance tests incompatibility with PostgreSQL (always use a String parameter for CalcLength parameter otherwise PostgreSQL fails since it doesn't try to do an implicit conversion)
Bump up PostgreSQL jdbc driver version

resolves #557 together with #562

MMatten · 2017-03-19T19:38:27Z

In the commit message, I didn't really understand

the usual output parameters are not really currently supported for postgres.

What is lacking for Postgres?

javornikolov · 2017-03-19T19:43:47Z

PostgreSQL demands if there is an output parameter:

the function return type is mandatory and it must match the type of the output parameter
if there are multiple output parameters, the return type should be a record

Essentially - seems that PostgreSQL always returns output parameters as a function return value. I'm not sure if there are alternative ways for retrieving them. What we currently support in DbFit is function/procedure argments of INPUT type only. (E.g. we cannot have a named output parameter which was the trouble with the initial Symbols test).

javornikolov · 2017-03-19T19:45:30Z

Currently there is some trouble with DB2. I don't know what's wrong with it - no obvious mistakes at first glimpse.

MMatten · 2017-03-19T19:58:01Z

Essentially - seems that PostgreSQL always returns output parameters as a function return value. I'm not sure if there are alternative ways for retrieving them. What we currently support in DbFit is function/procedure argments of INPUT type only. (E.g. we cannot have a named output parameter which was the trouble with the initial Symbols test).

For Postgres could we could retrieve the OUT parameter values using a StatementExecutionCapturingResultSetValue?

MMatten · 2017-03-19T20:01:31Z

Can we somehow upload (e.g. to the AWS S3 area) the test reports during the builds so we could see that is in

There were failing tests. See the report at: file:///home/travis/build/dbfit/dbfit/dbfit-java/db2/build/reports/tests/integrationTest/index.html

MMatten · 2017-03-19T20:02:12Z

Otherwise I can install DB2 again on my vagrant VM and run the tests via the UI.

javornikolov · 2017-03-19T20:05:56Z

I managed to run the UI tests through docker (seems quicker to fire it up compared to Vagrant). But the error message is quite cryptic: db2 SQL Error: SQLCODE=-440, SQLSTATE=42884 - I think while trying to set a Multiply parameter (via setObject).

MMatten · 2017-03-19T20:13:24Z

Could it be the actual call?

What is the current schema when you call the proc?

http://stackoverflow.com/questions/18367803/db2-java-stored-procedure-call-return-error-sqlcode-440-sqlstate-42884

javornikolov · 2017-03-19T20:16:35Z

MMatten · 2017-03-19T20:25:18Z

Can you log into DB2 and check that the proc exists?

http://stackoverflow.com/questions/15600755/db2-list-of-stored-procedure-in-a-database

MMatten · 2017-03-19T20:52:39Z

I'm not sure, perhaps you need to include more clauses (as in the example below) to ensure that it gets created as a SQL, rather C/Java external function?

    CREATE FUNCTION TAN (X DOUBLE)
      RETURNS DOUBLE
      LANGUAGE SQL
      CONTAINS SQL
      NO EXTERNAL ACTION
      DETERMINISTIC
      RETURN SIN(X)/COS(X);

javornikolov · 2017-03-19T21:48:02Z

select procname from syscat.procedures where procschema = 'DFTEST'

PROCNAME                                                                                                                        
------------------
CALCLENGTH                                                                                                                      
CONCATENATESTRINGS                                                                                                              
MAKEUSER                                                                                                                        
MULTIPLYIO

The other function which I used as example (ConcatenateF) is also not there.

javornikolov · 2017-03-19T22:02:01Z

Looks like functions and procedures in DB2 is quite different. I can use the function from sql query but not in a call:

db2 => call multiply(2, 3)
call multiply(2, 3)
SQL0440N  No authorized routine named "MULTIPLY" of type "PROCEDURE" having 
compatible arguments was found.  SQLSTATE=42884

db2 => SELECT multiply(3, 2) FROM SYSIBM.SYSDUMMY1
SELECT multiply(3, 2) FROM SYSIBM.SYSDUMMY1

1          
-----------
          6

  1 record(s) selected.

javornikolov · 2017-03-19T22:30:21Z

http://www.dbforums.com/showthread.php?1631503-how-can-i-call-a-simple-sql-pl-defined-function-using-jdbc

MMatten · 2017-03-20T06:28:31Z

So it looks like we could just form a different statement text (a query instead of a CALL) and retrieve the return value from the ResultSet (hopefully using StatementExecutionCapturingResultSetValue as it currently is).

javornikolov · 2017-03-25T21:43:43Z

So it looks like we could just form a different statement text (a query instead of a CALL) and retrieve the return value from the ResultSet (hopefully using StatementExecutionCapturingResultSetValue as it currently is).

It seems too exotic to try calling DB2 functions explicitly (I'm not even sure whether it's possible to extract their metadata with list of parameters).

On the other hand PostgreSQL seems to allow calling procedures with output parameters in the standard way (in addition to calling it as function). So I'm rather looking into adding support for that postgres output parameters. Some preparation committed to b8430d5 (perhaps its worth a separate PR).

MMatten · 2017-03-25T22:00:16Z

(I'm not even sure whether it's possible to extract their metadata with list of parameters).

Perhaps via JDBC methods rather than DB metadata.

MMatten · 2017-03-25T22:05:55Z

It seems too exotic to try calling DB2 functions explicitly

Maybe using something like:

SELECT F(1,2) FROM SYSIBM.SYSDUMMY1 and then reading the ResultSet.

javornikolov · 2017-03-25T22:21:39Z

SELECT F(1,2) FROM SYSIBM.SYSDUMMY1 and then reading the resell set.

Yes, I got your idea from before. It needs some research to figure out in which order to pass the parameters and how many are they - we may try to deal with DB2 separately. I would vote for PostgreSQL here - as it looks easier to implement and harder to workaround the current gap with it in case of multiple output params. (For DB2 the function may be wrapped in a SQL and tested within Query).

javornikolov · 2017-04-03T21:34:33Z

@MMatten, would you be able to check if that works on the Vagrant test VM for the databases outside Travis build? I touched some stored procs e.g. in Informix which I haven't tested.

I managed to add support for PostgreSQL output parameters here (it was possible without passing the list of arguments to getAllProcedure parameters - so I removed that change). With that we get the common symbols test pass throughout all environments.

MMatten · 2017-04-04T17:52:27Z

@yavornikolov yes, certainly. It will take me a couple of days to get to it though. I'll take a look at the whole PR again too.

Unify the Multiply function used in acceptance tests across all environments - rename the old one to MultiplyIO where inout parameter has been used.

Add support for calling PostgreSQL stored procedures with output parameters.

Use string bind variable for CalcLength stored procedure parameter - otherwise environments with strict type check like PostgreSQL could fail.

* Break down getAllParameters into smaller pieces * Extract DatabaseObjectName from Derby to reuse in PostgreSQL * Bump up postgres version to 42.0.0.jre7 (for getSchema support)

javornikolov · 2017-05-03T20:37:58Z

Just rebased on top of master to pick the Informix fix so I hope everything should be OK now.

MMatten · 2017-05-03T20:41:37Z

OK. I'll rebuild a dev VM and run it all through.

MMatten · 2017-05-11T20:15:44Z

Looking good so far. I've still got to test with Teradata, Netezza and SQL Server though.

MMatten · 2017-05-16T19:08:32Z

@javornikolov I'm still having a bit of a nightmare with testing on Windows 10 (it's awful and it's blue screening on me) when trying to build the VM. Will get back to you as soon as.

MMatten · 2017-05-21T08:30:59Z

Teradata tests are passing.

Netezza has been a massive pain to get the emulator running. I've moved on to the 7.2 version. This can run on Linux under VMWare. I need to somehow upload the 7.2 driver though. What's the best way to get this to you? Commit it?

Also for Netezza we need the new CalcLength routine added.

javornikolov · 2017-05-21T08:38:32Z

What's the best way to get this to you? Commit it?

Not a good idea to "pollute" the repo with a large binary jar. If it's hard to get the driver directly from IBM site, any file sharing should be fine (gdrive, dropbox, expirebox,...). You may share some hashsum here to ensure the file has been transferred intact.

javornikolov · 2017-05-21T09:23:14Z

I forgot to mention - for local testing you may just copy jdbc drivers to custom_libs folder.

MMatten · 2017-05-21T09:29:54Z

What's the best way to get this to you? Commit it?

Not a good idea to "pollute" the repo with a large binary jar. If it's hard to get the driver directly from IBM site, any file sharing should be fine (gdrive, dropbox, expirebox,...). You may share some hashsum here to ensure the file has been transferred intact.

I've just shared the driver file with you. We'll need to rename it and update the grade build.

MMatten · 2017-05-21T09:42:51Z

Also, it looks like Netezza doesn't support named proc params or output params.

MMatten · 2017-05-21T09:43:49Z

I think it's PostgreSQL based. It can return a single value (scalar or a (single column) table of values).

javornikolov · 2017-05-21T09:47:24Z

I've just shared the driver file with you. We'll need to rename it and update the grade build.

Thank you @MMatten! I've just uploaded the file, sha-1:

d89e4e54280b1ef0747346bba6799fbd064a3645 *nzjdbc3-7.2.jar

MMatten · 2017-05-21T10:07:17Z

d89e4e54280b1ef0747346bba6799fbd064a3645

That hash is correct. 👍

javornikolov · 2017-05-21T10:12:14Z

Also, it looks like Netezza doesn't support named proc params or output params.

Any ideas how to overcome that?

javornikolov · 2017-05-21T10:20:33Z

Perhaps the issue with Netezza stored procs in symbols tests is already in master. If everything else seems OK, maybe we rather merge the current PR (named after postgresql) and take care about Netezza separately.

MMatten · 2017-05-21T10:35:19Z

Perhaps the issue with Netezza stored procs in symbols tests is already in master.

Let me test that.

MMatten · 2017-05-21T10:55:35Z

Let me test that.

Yes, it looks like we introduced this breakage in 8d3e2c7. I can't remember which DBMSs we tested this on but at least not Netezza.

MMatten · 2017-05-21T11:01:45Z

Note that I still should test this PR with SQL Server too I guess.

MMatten · 2017-05-24T20:03:23Z

All SQL Server tests apart from the know issues in master as passing 👍

javornikolov · 2017-05-24T20:32:05Z

All SQL Server tests apart from the know issues in master as passing

Great :-) So I think we can already merge that to master to finally fix the broken Travis build. What do you think, is there anything else which may need to do before merging?

MMatten · 2017-05-24T20:52:32Z

I think we should merge this one now. 👍 :)

javornikolov · 2017-05-24T20:58:54Z

Nice, I'm merging it... :-) Thanks @MMatten for the thorough testing - it has revealed quite a few things!

javornikolov added the techdebt label Mar 19, 2017

javornikolov added this to the Next milestone Mar 19, 2017

javornikolov force-pushed the topic/fix-postgres-symbol-tests-failure branch 4 times, most recently from 679e721 to 2c1b695 Compare March 19, 2017 15:24

javornikolov force-pushed the topic/fix-postgres-symbol-tests-failure branch from b8430d5 to 6e37e10 Compare April 3, 2017 20:42

javornikolov added the enhancement label Apr 3, 2017

javornikolov force-pushed the topic/fix-postgres-symbol-tests-failure branch from 4af084e to 6de2715 Compare April 8, 2017 12:24

javornikolov added 5 commits May 3, 2017 23:36

Unify Multily test function accross environments

4bf356c

Unify the Multiply function used in acceptance tests across all environments - rename the old one to MultiplyIO where inout parameter has been used.

Move ParamDescriptor, IsParameter to core to enable reuse

90e9f6b

Postgres output params support

4c1b2d8

Add support for calling PostgreSQL stored procedures with output parameters.

Fix Symbols test: string bind var in CalcLength

30819eb

Use string bind variable for CalcLength stored procedure parameter - otherwise environments with strict type check like PostgreSQL could fail.

Refactor postgres proc parameters retrieval

5182584

* Break down getAllParameters into smaller pieces * Extract DatabaseObjectName from Derby to reuse in PostgreSQL * Bump up postgres version to 42.0.0.jre7 (for getSchema support)

javornikolov force-pushed the topic/fix-postgres-symbol-tests-failure branch from e6406e0 to 5182584 Compare May 3, 2017 20:36

javornikolov merged commit f3a5e9c into master May 24, 2017

javornikolov deleted the topic/fix-postgres-symbol-tests-failure branch May 24, 2017 20:59

MMatten mentioned this pull request May 24, 2017

Fix failing CoreTests for Netezza #578

Closed

Fix postgres symbol tests failure #565

Fix postgres symbol tests failure #565

Conversation

javornikolov commented Mar 19, 2017 • edited

MMatten commented Mar 19, 2017 • edited

javornikolov commented Mar 19, 2017

javornikolov commented Mar 19, 2017

MMatten commented Mar 19, 2017

MMatten commented Mar 19, 2017

MMatten commented Mar 19, 2017

javornikolov commented Mar 19, 2017

MMatten commented Mar 19, 2017 • edited

javornikolov commented Mar 19, 2017

MMatten commented Mar 19, 2017

MMatten commented Mar 19, 2017

javornikolov commented Mar 19, 2017

javornikolov commented Mar 19, 2017

javornikolov commented Mar 19, 2017

MMatten commented Mar 20, 2017 • edited

javornikolov commented Mar 25, 2017

MMatten commented Mar 25, 2017

MMatten commented Mar 25, 2017 • edited

javornikolov commented Mar 25, 2017 • edited

javornikolov commented Apr 3, 2017

MMatten commented Apr 4, 2017

javornikolov commented May 3, 2017 • edited

MMatten commented May 3, 2017

MMatten commented May 11, 2017

MMatten commented May 16, 2017

MMatten commented May 21, 2017 • edited

javornikolov commented May 21, 2017

javornikolov commented May 21, 2017

MMatten commented May 21, 2017

MMatten commented May 21, 2017

MMatten commented May 21, 2017

javornikolov commented May 21, 2017

MMatten commented May 21, 2017

javornikolov commented May 21, 2017

javornikolov commented May 21, 2017

MMatten commented May 21, 2017

MMatten commented May 21, 2017 • edited

MMatten commented May 21, 2017

MMatten commented May 24, 2017

javornikolov commented May 24, 2017 • edited

MMatten commented May 24, 2017

javornikolov commented May 24, 2017

javornikolov commented Mar 19, 2017 •

edited

MMatten commented Mar 19, 2017 •

edited

MMatten commented Mar 19, 2017 •

edited

MMatten commented Mar 20, 2017 •

edited

MMatten commented Mar 25, 2017 •

edited

javornikolov commented Mar 25, 2017 •

edited

javornikolov commented May 3, 2017 •

edited

MMatten commented May 21, 2017 •

edited

MMatten commented May 21, 2017 •

edited

javornikolov commented May 24, 2017 •

edited