Skip to content
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

tds_version not available in pymssql.connect #323

Closed
cweimann opened this issue Apr 15, 2015 · 3 comments
Closed

tds_version not available in pymssql.connect #323

cweimann opened this issue Apr 15, 2015 · 3 comments
Labels

Comments

@cweimann
Copy link

Specifying the tds_version is possible with _mssql.connect but not with pymssql.connect. Since _mssql defaults to tds_version='7.1' there is no way to connect with any other version without calling _mssql.connect directly. The tds version in freetds.conf seems to be ignored in all cases. I'd say it is a very good thing to be able to connect without needing a freetds.conf but rejecting it altogether seems to be going to far. I've patched local copy to allow specifying a tds_version in pymssql.connect and second to NOT default/force 7.1 in MSSQLConnection.init but instead to allow specifying tds_vesion but allow the freetds.conf to take effect if it is not specified.

I have seen a variety of other questions on the list that I think this is at the root of. BTW the environment var TDSVER does work but other than that there is currently no way to escape tds_version 7.1.

diff --git a/_mssql.pyx b/_mssql.pyx
index 3572fcb..fc2fd4c 100644
--- a/_mssql.pyx
+++ b/_mssql.pyx
@@ -535,7 +535,7 @@ cdef class MSSQLConnection:
         self.column_types = None

     def __init__(self, server="localhost", user="sa", password="",
-            charset='UTF-8', database='', appname=None, port='1433', tds_version='7.1', conn_properties=None):
+            charset='UTF-8', database='', appname=None, port='1433', tds_version=None, conn_properties=None):
         log("_mssql.MSSQLConnection.__init__()")

         cdef LOGINREC *login
@@ -569,7 +569,8 @@ cdef class MSSQLConnection:
         DBSETLUSER(login, user_cstr)
         DBSETLPWD(login, password_cstr)
         DBSETLAPP(login, appname_cstr)
-        DBSETLVERSION(login, _tds_ver_str_to_constant(tds_version))
+        if tds_version:
+            DBSETLVERSION(login, _tds_ver_str_to_constant(tds_version))

         # add the port to the server string if it doesn't have one already and
         # if we are not using an instance
diff --git a/pymssql.pyx b/pymssql.pyx
index 6ec4e7d..67c28b0 100644
--- a/pymssql.pyx
+++ b/pymssql.pyx
@@ -584,7 +584,7 @@ cdef class Cursor:

 def connect(server='.', user='', password='', database='', timeout=0,
         login_timeout=60, charset='UTF-8', as_dict=False,
-        host='', appname=None, port='1433', conn_properties=None, autocommit=False):
+        host='', appname=None, port='1433', tds_version=None, conn_properties=None, autocommit=False):
     """
     Constructor for creating a connection to the database. Returns a
     Connection object.
@@ -636,7 +636,7 @@ def connect(server='.', user='', password='', database='', timeout=0,
     try:
         conn = _mssql.connect(server=server, user=user, password=password,
                               charset=charset, database=database,
-                              appname=appname, port=port,
+                              appname=appname, port=port, tds_version=tds_version,
                               conn_properties=conn_properties)

     except _mssql.MSSQLDatabaseException, e:
@ramiro
Copy link
Contributor

ramiro commented Apr 16, 2015

ok to the proposal from me.

See 1e5d136.

@ramiro
Copy link
Contributor

ramiro commented Oct 19, 2015

See #350.

My proposal there is:

For the next 2.1.x release, to add the parameter to pymssql.connect() but preserve the '7.1' default values.

Defer to 2.2 the change of not enforcing any default TDS version value. This could be accompained with documentation admonitions about the possible backward incompatible change (people not passing tds_version are getting TDS 7.1 and that's going to change so they might need to modify their code.)

@ramiro ramiro added the bug label Oct 19, 2015
@ramiro
Copy link
Contributor

ramiro commented Nov 8, 2015

Fixed in fad26b9

@ramiro ramiro closed this as completed Nov 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants