minus5 / gofreetds

Go Sql Server database driver.
MIT License
113 stars 48 forks source link

Fix inability to use ExecSp against Sybase ASE #35

Closed marzolfb closed 8 years ago

marzolfb commented 8 years ago

Fixes the inability to use ExecSp against a Sybase ASE database when calling a stored procedure.

This PR uses the newly-introduced Sybase compatibility mode setting on the connection string to decide whether to use a MSSQL-specific or Sybase-specific SQL command to use in getting a list of stored procedure parameters for a given stored procedure

While you can just use the Exec method to execute a stored procedure that only involves returned result sets, you are forced to use ExecSp when your stored procedure returns data in output parameters. In initial usage of ExecSp, I received this error:

Msg 20018, Level 16
General SQL Server error: Check messages from the SQL Server

Msg 20018, Level 16
General SQL Server error: Check messages from the SQL Server

Msg 208, Level 16, State 1
Server 'SYBASE', Line 2
    sys.all_parameters not found. Specify owner.objectname or use sp_help to check whether the object exists (sp_help may produce lots of output).

Msg 208, Level 16, State 1
Server 'SYBASE', Line 2
    sys.all_objects not found. Specify owner.objectname or use sp_help to check whether the object exists (sp_help may produce lots of output).

Added a couple of unit tests to exercise the conditional logic as well.

caledhwa commented 8 years ago

Nice work on fixing something I didn't test on my compatibility mode. Since we have no CI testing on Sybase (or SQL), did you run any bench tests to make sure your code runs against Sybase ASE?

marzolfb commented 8 years ago

I did perform successful bench testing against the Sybase ASE database I was working on (I should have mentioned that above). Thanks for the positive feedback.

caledhwa commented 8 years ago

+1 @ianic - this would be a good add to my compatibility mode and looks like it should not negatively affect the existing SQL Server functionality.

ianic commented 8 years ago

Thank you marzolfb.

You are running gofreetds tests against Sybase ASE? They are all passing or there are some Mssql specific.

caledhwa commented 8 years ago

Hey @ianic - thanks for merging this! I know your library wasn't really suited for Sybase but this really helps me. Also, I'm looking into setting up a CI pipeline to run tests against a SQL Server and a Sybase ASE server in the cloud. When I get it done in my fork, maybe we can discuss it and hook it up to the master - without any expense. When I do this, I will likely adapt pubs.sql to Sybase so that all the tests pass on both platforms. May take me a month to set up.