-
Notifications
You must be signed in to change notification settings - Fork 90
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
Fix failing Netezza symbols core tests #579
Conversation
@MMatten, I hope that should solve the Netezza tests failure. Possibly we may need to also rename the input parameter (e.g. to |
de3fac2
to
fc6edd4
Compare
Pretty much looks like we should follow the $n naming so I just changed and force pushed it that way. |
I'm getting a failure in
|
Note that in the Perhaps we can use this to return the |
Indeed - Netezza dbfit environment doesn't support return values yet. Perhaps RETURNS column is what we need to form the return value descriptor. |
@javornikolov let me know if you need me to run any SQL queries, tests, etc, etc. |
@MMatten, I just pushed an attempt to add Netezza function return value support. May we test with it please. (It might worth adding some specific acceptance test for that but for a start we may use the Symbols tests). |
606ec11
to
74b24ce
Compare
I got a build error: -
|
Yes, sorry, I saw that too. I just force-pushed a version which should compile OK. |
In
|
Executing
|
74b24ce
to
737dd75
Compare
That was because I forgot to add |
|
Hmm... I just added in FlowMode.CallProcedures another test for sp_procedure return value and Inspect Procedure and direct calls to CalcLength. |
Gee, looks like we're already in debt with that (#11): https://www.ibm.com/developerworks/community/forums/html/threadTopic?id=6ab238ce-f4d2-48f9-ae4e-f9e648fff9fe |
No, no out params in stored procs and no function return values? May be the what we created for Informix would suit this. Let me test your updates now. |
I decompiled the JDBC driver to see what's going on inside. Looks like it has some support for return values but not through I'm just working on a |
@MMatten, I just pushed the patched Netezza environment. |
Great :-) |
At first glimpse the Informix approach for function calls should work here out of the box. I'll change that shortly to see if we can get rid of my getObject overrides. |
Done - pushed with 4bee54e. |
In
|
My bad. I moved more calls than needed. should be OK now.
Perhaps we sholdn't adjust the parameter index as we do for Informix. I changed that. Let's see if that helps. |
Awesome. A clean bill of health again. No test failures. |
Nice :-) I need to tidy up some bits here but looks like that we finally managed to get Netezza functions working. |
Great work! Interestingly there are also functions in Netezza in addition to stored procs that return values. I think they have to be built in C++ and compiled into the DB. One for another day! |
* Parameterize CalcLength parameters in Symbols acceptance tests in order to be able to configure it in an Netezza-compatible way. * Create CalcLength stored procedure for Netezza
Netezza JDBC driver doesn't support CallableStatement.getObject. We handle that limitation by retrieving return value through the statement's ResultSet.
Split getAllProcedureParameters into several routines using the same approach (and copy of some code) as in Postgres environment
c4609c7
to
76aa8b9
Compare
@MMatten, I did some refactoring. Can we test it again? |
We're still missing the But I'll test all the others (I just have Teradata to do). |
Length specifiers are dropped
@MMatten, could you test that again. I hope we have that error fixed now. |
All tests are passing now. |
Wonderful :-) Thanks for verifying, @MMatten! I guess we're ready to merge that PR? |
Yes. I think so. I've nothing else to add. |
Resolves #578
Parameterize CalcLength parameters in Symbols acceptance tests
Create CalcLength stored procedure for Netezza
Implement support for Neteza functions return values
Fix a small old typo in 001_add_objects_needed_for_acceptance_test.sql