A platform-specific defect in the ODBC driver

Hello Hiroshi,

I hope I am addressing this message correctly.

Consider this code in `statement.c':

------------------------------------------------------------
static void SC_init_parse_method(StatementClass *self)
{
if (self->multi_statement <= 0 && ...)
SC_set_parse_tricky(self);
}

StatementClass *
SC_Constructor(ConnectionClass *conn)
{
rv->multi_statement = -1; /* unknown */
}
------------------------------------------------------------

There is obviously an assumption here that `multi_statement' can,
under certain circumstances, be negative. And, indeed, this is what
`statement.h' says about it:

------------------------------------------------------------
struct StatementClass_
{
char multi_statement; /* -1:unknown 0:single 1:multi */
};

------------------------------------------------------------

so, `multi_statement' is a three-valued entity, with -1 being one of
the possible values.

But what if `char' is an unsigned type? Then assigning -1 to
`multi_statement' will result in it being 255, and comparisons like
`<= 0' won't make sense.

Can `char' be unsigned? Yes, it can, per paragraph 15 in the ISO
C, Section 6.2.5:

15 The three types char, signed char, and unsigned char are
collectively called the character types. The implementation
shall define char to have the same range, representation, and
behavior as either signed char or unsigned char.35)

Various compilers on AIX consider `char' to be unsigned, and the code
compiled there (with gcc), will fail miserably on some operations.

A possible and probably the best resolution is to give
`multi_statement' and similar "to-be-three-valued" variables a good
"indicator" type, which can allow all the expected values, -1, 0 and 1
and legit comparisons between them.

The enclosed patch does it and resolves the failures on AIX
completely.

Please consider including this or a similar fix into the official
distribution of the ODBC driver.

Thanks,

-- Alex -- alex-goncharov(at)comcast(dot)net --

============================================================
diff -rwup psqlodbc-08.03.0400.orig/bind.c psqlodbc-08.03.0400/bind.c
--- psqlodbc-08.03.0400.orig/bind.c 2008-10-06 17:46:09.000000000 -0400
+++ psqlodbc-08.03.0400/bind.c 2009-04-20 16:34:06.000000000 -0400
@@ -471,7 +471,7 @@ inolog("num_params=%d,%d\n", stmt->num_p
}
else
{
- char multi = FALSE, proc_return = 0;
+ indicator_type multi = FALSE, proc_return = 0;

stmt->proc_return = 0;
SC_scanQueryAndCountParams(stmt->statement, SC_get_conn(stmt), NULL, pcpar, &multi, &proc_return);
diff -rwup psqlodbc-08.03.0400.orig/convert.c psqlodbc-08.03.0400/convert.c
--- psqlodbc-08.03.0400.orig/convert.c 2008-10-29 09:40:54.000000000 -0400
+++ psqlodbc-08.03.0400/convert.c 2009-04-20 16:35:50.000000000 -0400
@@ -2323,7 +2323,8 @@ RETCODE prep_params(StatementClass *stmt
BOOL ret, once_descr;
ConnectionClass *conn = SC_get_conn(stmt);
QResultClass *res, *dest_res = NULL;
- char plan_name[32], multi;
+ char plan_name[32];
+ indicator_type multi;
int func_cs_count = 0;
const char *orgquery = NULL, *srvquery = NULL;
Int4 endp1, endp2;
diff -rwup psqlodbc-08.03.0400.orig/pgtypes.h psqlodbc-08.03.0400/pgtypes.h
--- psqlodbc-08.03.0400.orig/pgtypes.h 2008-07-04 09:49:33.000000000 -0400
+++ psqlodbc-08.03.0400/pgtypes.h 2009-04-20 16:37:36.000000000 -0400
@@ -119,4 +119,7 @@ SQLSMALLINT sqltype_to_default_ctype(con
Int4 ctype_length(SQLSMALLINT ctype);

#define USE_ZONE FALSE
+
+typedef signed char indicator_type;
+
#endif
diff -rwup psqlodbc-08.03.0400.orig/statement.c psqlodbc-08.03.0400/statement.c
--- psqlodbc-08.03.0400.orig/statement.c 2008-09-21 11:35:44.000000000 -0400
+++ psqlodbc-08.03.0400/statement.c 2009-04-20 17:03:11.000000000 -0400
@@ -856,7 +856,7 @@ inolog("%s statement=%p ommitted=0\n", f
void
SC_scanQueryAndCountParams(const char *query, const ConnectionClass *conn,
Int4 *next_cmd, SQLSMALLINT * pcpar,
- char *multi_st, char *proc_return)
+ indicator_type *multi_st, indicator_type *proc_return)
{
CSTR func = "SC_scanQueryAndCountParams";
char literal_quote = LITERAL_QUOTE, identifier_quote = IDENTIFIER_QUOTE, dollar_quote = DOLLAR_QUOTE;
@@ -865,7 +865,8 @@ SC_scanQueryAndCountParams(const char *q
char tchar, bchar, escape_in_literal = '\0';
char in_literal = FALSE, in_identifier = FALSE,
in_dollar_quote = FALSE, in_escape = FALSE,
- del_found = FALSE, multi = FALSE;
+ del_found = FALSE;
+ indicator_type multi = FALSE;
SQLSMALLINT num_p;
encoded_str encstr;

diff -rwup psqlodbc-08.03.0400.orig/statement.h psqlodbc-08.03.0400/statement.h
--- psqlodbc-08.03.0400.orig/statement.h 2008-08-09 19:30:53.000000000 -0400
+++ psqlodbc-08.03.0400/statement.h 2009-04-20 16:44:43.000000000 -0400
@@ -225,25 +225,25 @@ struct StatementClass_
Int2 current_exec_param; /* The current parameter for
* SQLPutData */
PutDataInfo pdata_info;
- char parse_status;
- char proc_return;
- char put_data; /* Has SQLPutData been called ? */
- char catalog_result; /* Is this a result of catalog function ? */
- char prepare; /* is this a prepared statement ? */
- char prepared; /* is this statement already
+ indicator_type parse_status;
+ indicator_type proc_return;
+ indicator_type put_data; /* Has SQLPutData been called ? */
+ indicator_type catalog_result; /* Is this a result of catalog function ? */
+ indicator_type prepare; /* is this a prepared statement ? */
+ indicator_type prepared; /* is this statement already
* prepared at the server ? */
- char internal; /* Is this statement being called
+ indicator_type internal; /* Is this statement being called
* internally ? */
- char transition_status; /* Transition status */
- char multi_statement; /* -1:unknown 0:single 1:multi */
- char rbonerr; /* rollback on error */
- char discard_output_params; /* discard output parameters on parse stage */
- char cancel_info; /* cancel information */
- char ref_CC_error; /* refer to CC_error ? */
- char lock_CC_for_rb; /* lock CC for statement rollback ? */
- char join_info; /* have joins ? */
- char parse_method; /* parse_statement is forced or ? */
- char curr_param_result; /* current param result is set ? */
+ indicator_type transition_status; /* Transition status */
+ indicator_type multi_statement; /* -1:unknown 0:single 1:multi */
+ indicator_type rbonerr; /* rollback on error */
+ indicator_type discard_output_params; /* discard output parameters on parse stage */
+ indicator_type cancel_info; /* cancel information */
+ indicator_type ref_CC_error; /* refer to CC_error ? */
+ indicator_type lock_CC_for_rb; /* lock CC for statement rollback ? */
+ indicator_type join_info; /* have joins ? */
+ indicator_type parse_method; /* parse_statement is forced or ? */
+ indicator_type curr_param_result; /* current param result is set ? */
pgNAME cursor_name;
char *plan_name;

@@ -485,7 +485,7 @@ int SC_set_current_col(StatementClass *
void SC_setInsertedTable(StatementClass *, RETCODE);
void SC_scanQueryAndCountParams(const char *, const ConnectionClass *,
Int4 *next_cmd, SQLSMALLINT *num_params,
- char *multi, char *proc_return);
+ indicator_type *multi, indicator_type *proc_return);

BOOL SC_IsExecuting(const StatementClass *self);
BOOL SC_SetExecuting(StatementClass *self, BOOL on);
============================================================