[freetds] [PATCH] fix defaults for cursor type and concurrency in ctlib

Discussion:

martin.wilck

2012-10-26 14:40:01 UTC

Hello,

I have a problem using python-sybase with a MS-SQL Server data base (TDS 7.1).
cursor.execute() aborts with the error message
"sp_cursoropen: The value of the parameter 'scrollopt' is invalid.".

The same problem has been reported a long time ago already:
https://sourceforge.net/mailarchive/forum.php?thread_name=5c06fa770907291200w5e7aeb3akaba12ab95d419785%40mail.gmail.com&forum_name=python-sybase-misc

As correctly stated there, the problem occurs with python-sybase v0.39 and
later only. This is due to the fact that python-sybase started to use
the ct_cursor() API in 0.39. But that's not a bug of python-sybase.

I analyzed the problem and I found that FreeTDS ctlib code sends a value
of 0 for cursor->type and cursor->concurrency to the server, which is
invalid. It appears that these two fields are never properly initialized
in ctlib. The ODBC code (odbc.c) has code that initializes these fields.

Below is a suggested patch that sets the default values as documented.
in http://jtds.sourceforge.net/apiCursors.html. Please review.

The patch was made against FreeTDS 0.91-3 (Fedora 17).

Best regards
Martin

diff -rup freetds-0.91.orig/include/tds.h freetds-0.91/include/tds.h
--- freetds-0.91.orig/include/tds.h 2011-08-18 03:54:41.000000000 +0200
+++ freetds-0.91/include/tds.h 2012-10-26 15:50:08.000000000 +0200
@@ -378,6 +378,25 @@ enum {
TDS_CUR_ISTAT_DEALLOC = 0x40
};

+/* http://jtds.sourceforge.net/apiCursors.html */
+/* Cursor scroll option, must be one of 0x01 - 0x10, OR'd with other bits */
+enum {
+ TDS_CUR_TYPE_KEYSET = 0x0001, /* default */
+ TDS_CUR_TYPE_DYNAMIC = 0x0002,
+ TDS_CUR_TYPE_FORWARD = 0x0004,
+ TDS_CUR_TYPE_STATIC = 0x0008,
+ TDS_CUR_TYPE_FASTFORWARDONLY = 0x0010,
+ TDS_CUR_TYPE_PARAMETERIZED = 0x1000,
+ TDS_CUR_TYPE_AUTO_FETCH = 0x2000
+};
+
+enum {
+ TDS_CUR_CONCUR_READ_ONLY = 1,
+ TDS_CUR_CONCUR_SCROLL_LOCKS = 2,
+ TDS_CUR_CONCUR_OPTIMISTIC = 4, /* default */
+ TDS_CUR_CONCUR_OPTIMISTIC_VALUES = 8
+};
+
/* environment type field */
#define TDS_ENV_DATABASE 1
#define TDS_ENV_LANG 2
Only in freetds-0.91/include: tds.h~
diff -rup freetds-0.91.orig/src/ctlib/ct.c freetds-0.91/src/ctlib/ct.c
--- freetds-0.91.orig/src/ctlib/ct.c 2010-10-05 10:36:36.000000000 +0200
+++ freetds-0.91/src/ctlib/ct.c 2012-10-26 15:55:38.000000000 +0200
@@ -3704,6 +3704,8 @@ ct_cursor(CS_COMMAND * cmd, CS_INT type,
cursor->status.fetch = _CS_CURS_TYPE_UNACTIONED;
cursor->status.close = _CS_CURS_TYPE_UNACTIONED;
cursor->status.dealloc = _CS_CURS_TYPE_UNACTIONED;
+ cursor->type = TDS_CUR_TYPE_KEYSET;
+ cursor->concurrency = TDS_CUR_CONCUR_OPTIMISTIC;

cmd->cursor = cursor;
ct_set_command_state(cmd, _CS_COMMAND_READY);

Frediano Ziglio

2012-10-26 16:05:11 UTC

Permalink

Seems good to me, I'll apply.

I think constants should be in tdsproto.h cause are related to the
protocol and initialization should be in tds_alloc_cursor to make sure
all libraries get these values initiated properly.

Frediano

Post by martin.wilck
Hello,
I have a problem using python-sybase with a MS-SQL Server data base (TDS 7.1).
cursor.execute() aborts with the error message
"sp_cursoropen: The value of the parameter 'scrollopt' is invalid.".
https://sourceforge.net/mailarchive/forum.php?thread_name=5c06fa770907291200w5e7aeb3akaba12ab95d419785%40mail.gmail.com&forum_name=python-sybase-misc
As correctly stated there, the problem occurs with python-sybase v0.39 and
later only. This is due to the fact that python-sybase started to use
the ct_cursor() API in 0.39. But that's not a bug of python-sybase.
I analyzed the problem and I found that FreeTDS ctlib code sends a value
of 0 for cursor->type and cursor->concurrency to the server, which is
invalid. It appears that these two fields are never properly initialized
in ctlib. The ODBC code (odbc.c) has code that initializes these fields.
Below is a suggested patch that sets the default values as documented.
in http://jtds.sourceforge.net/apiCursors.html. Please review.
The patch was made against FreeTDS 0.91-3 (Fedora 17).
Best regards
Martin
diff -rup freetds-0.91.orig/include/tds.h freetds-0.91/include/tds.h
--- freetds-0.91.orig/include/tds.h 2011-08-18 03:54:41.000000000 +0200
+++ freetds-0.91/include/tds.h 2012-10-26 15:50:08.000000000 +0200
@@ -378,6 +378,25 @@ enum {
TDS_CUR_ISTAT_DEALLOC = 0x40
};
+/* http://jtds.sourceforge.net/apiCursors.html */
+/* Cursor scroll option, must be one of 0x01 - 0x10, OR'd with other bits */
+enum {
+ TDS_CUR_TYPE_KEYSET = 0x0001, /* default */
+ TDS_CUR_TYPE_DYNAMIC = 0x0002,
+ TDS_CUR_TYPE_FORWARD = 0x0004,
+ TDS_CUR_TYPE_STATIC = 0x0008,
+ TDS_CUR_TYPE_FASTFORWARDONLY = 0x0010,
+ TDS_CUR_TYPE_PARAMETERIZED = 0x1000,
+ TDS_CUR_TYPE_AUTO_FETCH = 0x2000
+};
+
+enum {
+ TDS_CUR_CONCUR_READ_ONLY = 1,
+ TDS_CUR_CONCUR_SCROLL_LOCKS = 2,
+ TDS_CUR_CONCUR_OPTIMISTIC = 4, /* default */
+ TDS_CUR_CONCUR_OPTIMISTIC_VALUES = 8
+};
+
/* environment type field */
#define TDS_ENV_DATABASE 1
#define TDS_ENV_LANG 2
Only in freetds-0.91/include: tds.h~
diff -rup freetds-0.91.orig/src/ctlib/ct.c freetds-0.91/src/ctlib/ct.c
--- freetds-0.91.orig/src/ctlib/ct.c 2010-10-05 10:36:36.000000000 +0200
+++ freetds-0.91/src/ctlib/ct.c 2012-10-26 15:55:38.000000000 +0200
@@ -3704,6 +3704,8 @@ ct_cursor(CS_COMMAND * cmd, CS_INT type,
cursor->status.fetch = _CS_CURS_TYPE_UNACTIONED;
cursor->status.close = _CS_CURS_TYPE_UNACTIONED;
cursor->status.dealloc = _CS_CURS_TYPE_UNACTIONED;
+ cursor->type = TDS_CUR_TYPE_KEYSET;
+ cursor->concurrency = TDS_CUR_CONCUR_OPTIMISTIC;
cmd->cursor = cursor;
ct_set_command_state(cmd, _CS_COMMAND_READY);
_______________________________________________
FreeTDS mailing list
FreeTDS at lists.ibiblio.org
http://lists.ibiblio.org/mailman/listinfo/freetds

Frediano Ziglio

2012-10-26 20:49:16 UTC

Permalink

Applied to 0.91 and HEAD

Frediano

Martin Wilck

2012-10-31 08:24:37 UTC

Permalink

Hi Frediano,

Post by Frediano Ziglio
Applied to 0.91 and HEAD

Forgive my ignorance, but I haven't seen the patches show up in CVS. Am
I looking in the wrong place (freetds.cvs.sourceforge.net:/cvsroot/freetds)?

Martin
--
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone: ++49 5251 525 2796
Fax: ++49 5251 525 2820
Email: martin.wilck at ts.fujitsu.com
Internet: http://ts.fujitsu.com
Company Details: http://ts.fujitsu.com/imprint

Frediano Ziglio

2012-10-31 12:33:06 UTC

Permalink

Post by Martin Wilck
Hi Frediano,

Post by Frediano Ziglio
Applied to 0.91 and HEAD

Forgive my ignorance, but I haven't seen the patches show up in CVS. Am
I looking in the wrong place (freetds.cvs.sourceforge.net:/cvsroot/freetds)?
Martin

I think so, have a look at http://gitorious.org/freetds/freetds.

We should remove CVS, it confuses people.

Frediano

Post by Martin Wilck
--
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering
FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone: ++49 5251 525 2796
Fax: ++49 5251 525 2820
Email: martin.wilck at ts.fujitsu.com
Internet: http://ts.fujitsu.com
Company Details: http://ts.fujitsu.com/imprint
_______________________________________________
FreeTDS mailing list
FreeTDS at lists.ibiblio.org
http://lists.ibiblio.org/mailman/listinfo/freetds

Martin Wilck

2012-10-31 15:14:54 UTC

Permalink

Post by Frediano Ziglio
I think so, have a look at http://gitorious.org/freetds/freetds.
We should remove CVS, it confuses people.

Yeah, and you should update the links on the web page...
Thanks,

Martin
--
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone: ++49 5251 525 2796
Fax: ++49 5251 525 2820
Email: martin.wilck at ts.fujitsu.com
Internet: http://ts.fujitsu.com
Company Details: http://ts.fujitsu.com/imprint