Bug 90534 - SQLExecute returns success for UPDATE when no data is updated
Summary: SQLExecute returns success for UPDATE when no data is updated
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Red Hat Linux
Classification: Retired
Component: postgresql-odbc
Version: 7.2
Hardware: i386
OS: Linux
medium
low
Target Milestone: ---
Assignee: Andrew Overholt
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2003-05-09 13:38 UTC by Tim Woodall
Modified: 2007-04-18 16:53 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2003-06-12 16:20:22 UTC
Embargoed:


Attachments (Terms of Use)
Patch that applies against CVS (1.53 KB, patch)
2003-06-12 17:00 UTC, Tim Woodall
no flags Details | Diff

Description Tim Woodall 2003-05-09 13:38:30 UTC
postgresql-odbc-7.1.3-4bp.2

All the ODBC drivers I have ever used have all returned SQL_NO_DATA_FOUND when attempting to do an update and no rows are updated.
 
The postgresql drivers appear to always be returning SQL_SUCCESS.
 
In ODBC 3.5 Developers guide by Roger E. Sanders for SQLExecute there is the statement:
 
"If the SQL statement being executed is a searched UPDATE statement or a searched DELETE statement that does not affect rows in the data source, 
SQL_NO_DATA is returned to the aclling application."
 
The same statement also applies to SQLExecDirect although I haven't tested this with postgres.
 
How reproducible:
Always

I have logged this bug at http://gborg.postgresql.org/project/psqlodbcplus/bugs/bugupdate.php?499 although I don't know whether that site applies to 
the unix odbc driver or just the windows one. The same issue occurs on both platforms (both talking to a linux postgresql database)

Comment 1 Tim Woodall 2003-05-15 16:27:56 UTC
The following patch (also submitted to the psql-interfaces mailing list) fixes 
this issue:

diff -ur postgresql-7.1.3/src/interfaces/odbc/statement.c postgresql-7.1.3-
patched/src/interfaces/odbc/statement.c
--- postgresql-7.1.3/src/interfaces/odbc/statement.c	Mon Apr 23 02:00:49 2001
+++ postgresql-7.1.3-patched/src/interfaces/odbc/statement.c	Thu May 15 
16:30:46 2003
@@ -864,6 +864,13 @@
 	Int2		oldstatus,
 				numcols;
 	QueryInfo	qi;
+	char		was_rows_affected = 1;
+	/* was_rows_affected is set to 0 iff an UPDATE or DELETE affects 
+	 * no rows. In this instance the driver should return
+	 * SQL_NO_DATA_FOUND and not SQL_SUCCESS.
+	 * I'm not sure about the use of char rather than int but this is
+	 * consistent with the other was_* variables above.
+	 */
 
 
 	conn = SC_get_conn(self);
@@ -998,6 +1005,13 @@
 
 		was_ok = QR_command_successful(self->result);
 		was_nonfatal = QR_command_nonfatal(self->result);
+		if(self->result->command &&
+			(strncmp(self->result->command, "UPDATE", 6) == 0 ||
+			strncmp(self->result->command, "DELETE", 6) == 0) &&
+			strtoul(self->result->command + 7, NULL, 0) == 0)
+		{
+			was_rows_affected = 0;
+		}
 
 		if (was_ok)
 			self->errornumber = STMT_OK;
@@ -1055,7 +1069,10 @@
 	}
 
 	if (self->errornumber == STMT_OK)
-		return SQL_SUCCESS;
+		if(was_rows_affected)
+			return SQL_SUCCESS;
+		else
+			return SQL_NO_DATA_FOUND;
 
 	else
 	{


Comment 2 Andrew Overholt 2003-06-12 16:20:22 UTC
Tim,

Did you ever get a response from the community lists?  Did you post to
pgsql-odbc as well or just pgsql-interfaces?  The bug that you filed on GBorg is
filed against psqlodbcplus when it should be filed against psqlodbc ... this may
explain some delay.  Have you spoken to Dave Page?  He's the psqlODBC guy
AFAICT.  I'm going to mark this UPSTREAM, but I'm interested in hearing what
happens and will watch for traffic in the mailing list(s).

Comment 3 Tim Woodall 2003-06-12 16:55:27 UTC
I have posted to the psql-odbc list (I realised that the interfaces one wasn't the correct one after I had posted)I have had an email from Bruce Momjian (who was the only person I could find with any contact information) where he states "Got it and will apply shortly."I believe he has picked this up from the interfaces list. The posting to the odbc list included the patch (attached) which is against cvsrather than the RedHat code.

Comment 4 Tim Woodall 2003-06-12 17:00:36 UTC
Created attachment 92362 [details]
Patch that applies against CVS

I haven't built and tested this patch 
(I haven't had time to work out how to integrate the cvs code into a buildable 
sourcebase)

I suspect that some of the "self->result" I have added will need replacing with
"res".

Comment 5 Andrew Overholt 2003-06-12 17:07:26 UTC
That's good (the version of psqlODBC that you are using is quite old but I'm not
sure how much has changed since then).  I'm sure Bruce will look after it now
that it's in his list of to-be-applied patches.

Comment 6 Tim Woodall 2003-06-18 13:45:15 UTC
I now have a patch that applies against RH9. It also applies (with an offset) 
against the cvs version. This has been tested on RH9.
(Also sent to the pgsql-odbc list)

Regards,

Tim.

[tim@twlinux psqlodbc]$ patch -p1 <psqlodbc-7.2.5-update.patch
patching file statement.c
Hunk #1 succeeded at 1022 (offset 60 lines).
Hunk #3 succeeded at 1258 (offset 65 lines).
[tim@twlinux psqlodbc]$ cat psqlodbc-7.2.5-update.patch
diff -ur psqlodbc-7.2.5.orig/statement.c psqlodbc-7.2.5/statement.c
--- psqlodbc-7.2.5.orig/statement.c     2002-11-29 15:43:50.000000000 +0000
+++ psqlodbc-7.2.5/statement.c  2003-06-16 11:50:32.000000000 +0100
@@ -962,7 +962,14 @@
        static char *func = "SC_execute";
        ConnectionClass *conn;
        APDFields       *apdopts;
-       char            was_ok, was_nonfatal;
+       char            was_ok, was_nonfatal, was_rows_affected = 1;
+       /* was_rows_affected is set to 0 iff an UPDATE or DELETE affects
+        * no rows. In this instance the driver should return
+        * SQL_NO_DATA_FOUND and not SQL_SUCCESS.
+        * I'm not sure about the use of char rather than int but this is
+        * consistent with the other was_* variables above.
+        */
+
        QResultClass    *res = NULL;
        Int2            oldstatus,
                                numcols;
@@ -1082,6 +1089,13 @@
        {
                was_ok = QR_command_successful(res);
                was_nonfatal = QR_command_nonfatal(res);
+               if(res->command &&
+                       (strncmp(res->command, "UPDATE", 6) == 0 ||
+                       strncmp(res->command, "DELETE", 6) == 0) &&
+                       strtoul(res->command + 7, NULL, 0) == 0)
+               {
+                       was_rows_affected = 0;
+               }

                if (was_ok)
                        SC_set_errornumber(self, STMT_OK);
@@ -1179,7 +1193,10 @@
                }
        }
        if (SC_get_errornumber(self) == STMT_OK)
-               return SQL_SUCCESS;
+               if(was_rows_affected)
+                       return SQL_SUCCESS;
+               else
+                       return SQL_NO_DATA_FOUND;
        else if (SC_get_errornumber(self) == STMT_INFO_ONLY)
                return SQL_SUCCESS_WITH_INFO;
        else




Note You need to log in before you can comment on or make changes to this bug.