Thread: ODBC Boolean handling
I had a few issues with boolean handling in ODBC driver. 1) When a row is retrieved, and then a SQL_FETCH_FIRST is issued, the check in convert.c does not consider the fact that the value in the field has been altered to be a '1' if the backend handed it a 't'. The net result being that the first row on any subsequent queries has all it's boolean set to 0. 2) I had issues with some utilities that, when casting from string to boolean, and having the global 'bools as char' set, require that -1 = True, and 0 = false. The following patch fixes both of these issues.. Cheers for building a fantastic product Aidan cvs diff convert.c (in directory C:\CVS-POSTGRES\pgsql\src\interfaces\odbc\) Index: convert.c =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/odbc/convert.c,v retrieving revision 1.44 diff -r1.44 convert.c 295c295,307 < if (s[0] == 'T' || s[0] == 't') --- > /* Aidan Mountford (aidan@oz.to) 1/08/2001: > > >> if (s[0] == 'T' || s[0] == 't') <<< This wont work... > > When MoveFirst is called twice on one set of tuples, > this will have the effect of setting s[0] to 1 on the > first pass, and s[0] on the second. > > This is bad ;) > > */ > > if (s[0] == 'T' || s[0] == 't' || s[0] == '1') 415c428 < len = 1; --- > 418c431,449 < strcpy(rgbValueBindRow, value); --- > /* Aidan Mountford (aidan@oz.to) 1/08/2001: > > When returning a CHAR datatype, return -1 > instead of +1. > > That way if someone casts it back to a boolean > again - it will work. > */ > if ( *(char *) value = '1') > { > strcpy(rgbValueBindRow, "-1"); > len = 2; > } > else > { > strcpy(rgbValueBindRow, "0"); > len = 1; > } > 1160a1192,1193 > > 1161a1195 > ..................................................... . Cisco Certified Network Associate . Microsoft Certified Professional . 3Com 3Wizard . . Technical Director . MindVision Interactive Pty. Ltd. . http://www.mindvision.com.au . PH: +61-8-8212-9544 . FAX: +61-8-8212-9644 . 48 Light Square . Adelaide SA . AUSTRALIA 5000 ..................................................... "Remember, There is always scope for things to be worse"
Aidan Mountford wrote: > > I had a few issues with boolean handling in ODBC driver. > > 1) When a row is retrieved, and then a SQL_FETCH_FIRST is issued, the check > in convert.c > does not consider the fact that the value in the field has been altered to > be a '1' if the > backend handed it a 't'. The net result being that the first row on any > subsequent queries > has all it's boolean set to 0. > You seem to be right. I would commit it as soon as I am able to have a cvs access( I hope it would be before 7.2 release). > 2) I had issues with some utilities that, when casting from string to > boolean, and having > the global 'bools as char' set, require that -1 = True, and 0 = false. > Hmm I don't understand this. Please explain more in detail. regards, Hiroshi Inoue
I have reviewed the current CVS code and it looks quite different from the patch you submitted. Any chance of you downloading the snapshot and sending a context diff, rather than an ordinary one. I could apply that easily. > I had a few issues with boolean handling in ODBC driver. > > 1) When a row is retrieved, and then a SQL_FETCH_FIRST is issued, the check > in convert.c > does not consider the fact that the value in the field has been altered to > be a '1' if the > backend handed it a 't'. The net result being that the first row on any > subsequent queries > has all it's boolean set to 0. > > 2) I had issues with some utilities that, when casting from string to > boolean, and having > the global 'bools as char' set, require that -1 = True, and 0 = false. > > The following patch fixes both of these issues.. > > Cheers for building a fantastic product > > Aidan > > > > cvs diff convert.c (in directory C:\CVS-POSTGRES\pgsql\src\interfaces\odbc\) > Index: convert.c > =================================================================== > RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/odbc/convert.c,v > retrieving revision 1.44 > diff -r1.44 convert.c > 295c295,307 > < if (s[0] == 'T' || s[0] == 't') > --- > > /* Aidan Mountford (aidan@oz.to) 1/08/2001: > > > > > >> if (s[0] == 'T' || s[0] == 't') > <<< This wont work... > > > > When MoveFirst is called twice on > one set of tuples, > > this will have the effect of setting > s[0] to 1 on the > > first pass, and s[0] on the second. > > > > This is bad ;) > > > > */ > > > > if (s[0] == 'T' || s[0] == 't' || s[0] == > '1') > 415c428 > < len = 1; > --- > > > 418c431,449 > < strcpy(rgbValueBindRow, value); > --- > > /* Aidan Mountford (aidan@oz.to) > 1/08/2001: > > > > When returning a CHAR > datatype, return -1 > > instead of +1. > > > > That way if someone casts it > back to a boolean > > again - it will work. > > */ > > if ( *(char *) value = '1') > > { > > strcpy(rgbValueBindRow, "-1"); > > len = 2; > > } > > else > > { > > strcpy(rgbValueBindRow, "0"); > > len = 1; > > } > > > 1160a1192,1193 > > > > > 1161a1195 > > > > > ..................................................... > . Cisco Certified Network Associate > . Microsoft Certified Professional > . 3Com 3Wizard > . > . Technical Director > . MindVision Interactive Pty. Ltd. > . http://www.mindvision.com.au > . PH: +61-8-8212-9544 > . FAX: +61-8-8212-9644 > . 48 Light Square > . Adelaide SA > . AUSTRALIA 5000 > ..................................................... > > "Remember, There is always scope for things to be worse" > > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Here is the submitted patch so everyone can see it. > As Requested ... > > > > > -----Original Message----- > From: Bruce Momjian [mailto:pgman@candle.pha.pa.us] > Sent: Thursday, 2 August 2001 12:30 PM > To: Aidan Mountford > Cc: 'pgsql-patches@postgresql.org' > Subject: Re: [PATCHES] ODBC Boolean handling > > > > I have reviewed the current CVS code and it looks quite different from > the patch you submitted. Any chance of you downloading the snapshot and > sending a context diff, rather than an ordinary one. I could apply that > easily. > > > > > I had a few issues with boolean handling in ODBC driver. > > > > 1) When a row is retrieved, and then a SQL_FETCH_FIRST is issued, the > check > > in convert.c > > does not consider the fact that the value in the field has been altered to > > be a '1' if the > > backend handed it a 't'. The net result being that the first row on any > > subsequent queries > > has all it's boolean set to 0. > > > > 2) I had issues with some utilities that, when casting from string to > > boolean, and having > > the global 'bools as char' set, require that -1 = True, and 0 = false. > > > > The following patch fixes both of these issues.. > > > > Cheers for building a fantastic product > > > > Aidan -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 *** ./interfaces/odbc/convert.c Thu Aug 02 15:28:28 2001 --- ./interfaces/odbc/.#convert.c.1.44 Wed Aug 01 23:36:58 2001 *************** *** 292,301 **** { /* change T/F to 1/0 */ char *s = (char *) value; ! if (s[0] == 'T' || s[0] == 't') s[0] = '1'; else ! s[0] = '0'; } break; --- 292,314 ---- { /* change T/F to 1/0 */ char *s = (char *) value; ! /* Aidan Mountford (aidan@oz.to) 1/08/2001: ! ! >> if (s[0] == 'T' || s[0] == 't') <<< This wont work... ! ! When MoveFirst is called twice on one set of tuples, ! this will have the effect of setting s[0] to 1 on the ! first pass, and s[0] on the second. ! ! This is bad ;) ! ! */ ! ! if (s[0] == 'T' || s[0] == 't' || s[0] == '1') s[0] = '1'; else ! s[0] = '0'; ! } break; *************** *** 412,421 **** break; case PG_TYPE_BOOL: ! len = 1; if (cbValueMax > len) { ! strcpy(rgbValueBindRow, value); mylog("PG_TYPE_BOOL: rgbValueBindRow = '%s'\n", rgbValueBindRow); } break; --- 425,452 ---- break; case PG_TYPE_BOOL: ! if (cbValueMax > len) { ! /* Aidan Mountford (aidan@oz.to) 1/08/2001: ! ! When returning a CHAR datatype, return -1 ! instead of +1. ! ! That way if someone casts it back to a boolean ! again - it will work. ! */ ! if ( *(char *) value = '1') ! { ! strcpy(rgbValueBindRow, "-1"); ! len = 2; ! } ! else ! { ! strcpy(rgbValueBindRow, "0"); ! len = 1; ! } ! mylog("PG_TYPE_BOOL: rgbValueBindRow = '%s'\n", rgbValueBindRow); } break; *************** *** 1158,1164 **** --- 1189,1198 ---- } else { + + used = stmt->parameters[param_number].used ? *stmt->parameters[param_number].used : SQL_NTS; + buffer = stmt->parameters[param_number].buffer; }
> You seem to be right. > I would commit it as soon as I am able to have a cvs access( > I hope it would be before 7.2 release). Hiroshi, I will take care of applying the patch. Are you having CVS troubles we can help with? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Bruce Momjian wrote: > > Here is the submitted patch so everyone can see it. > I agree to the first change but am suspicious about the second change. > > > 2) I had issues with some utilities that, when casting from string to > > > boolean, and having > > > the global 'bools as char' set, require that -1 = True, and 0 = false. > > > Though Aidan is right, is -1 guaranteed to be true in any application conversely ? It seems also different from the ODBC spec. I don't have the solution now. regards, Hiroshi Inoue
Bruce Momjian wrote: > > > You seem to be right. > > I would commit it as soon as I am able to have a cvs access( > > I hope it would be before 7.2 release). > > Hiroshi, I will take care of applying the patch. Are you having CVS > troubles we can help with? > My entry in hub.org seems to have gone about 25 days ago by some accident. I got my entry in another host instead but I don't know how to do with it. regards, Hiroshi Inoue
I wrote: > > Bruce Momjian wrote: > > > > Here is the submitted patch so everyone can see it. > > > > I agree to the first change but am suspicious about the > second change. > > > > > 2) I had issues with some utilities that, when casting from string to > > > > boolean, and having > > > > the global 'bools as char' set, require that -1 = True, and 0 = false. > > > > > > Though Aidan is right, is -1 guaranteed to be true in > any application conversely ? It seems also different > from the ODBC spec. I don't have the solution now. > Please never apply the second part. I would propose another idea instead soon(hopefully). regards, Hiroshi Inoue -- 425,452 ---- break; case PG_TYPE_BOOL: ! if (cbValueMax > len) <<<<<<< len isn't set at this point. >>>>>>> { ! /* Aidan Mountford (aidan@oz.to) 1/08/2001: ! ! When returning a CHAR datatype, return -1 ! instead of +1. ! ! That way if someone casts it back to a boolean ! again - it will work. ! */ ! if ( *(char *) value = '1') <<<<<<<< '=' must be '=='. >>>>>>>>
> > Though Aidan is right, is -1 guaranteed to be true in > > any application conversely ? It seems also different > > from the ODBC spec. I don't have the solution now. > > > > Please never apply the second part. > I would propose another idea instead soon(hopefully). OK, I will not apply the patch and wait to hear from you. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
> Bruce Momjian wrote: > > > > > You seem to be right. > > > I would commit it as soon as I am able to have a cvs access( > > > I hope it would be before 7.2 release). > > > > Hiroshi, I will take care of applying the patch. Are you having CVS > > troubles we can help with? > > > > My entry in hub.org seems to have gone about 25 days ago > by some accident. I got my entry in another host instead > but I don't know how to do with it. I use postgresql.org for CVS. Not sure if that is the same as hub.org. nslookup shows them with different IP addresses. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
I am going to apply the first part only. Hiroshi has ideas on the second part. > As Requested ... > > > > > -----Original Message----- > From: Bruce Momjian [mailto:pgman@candle.pha.pa.us] > Sent: Thursday, 2 August 2001 12:30 PM > To: Aidan Mountford > Cc: 'pgsql-patches@postgresql.org' > Subject: Re: [PATCHES] ODBC Boolean handling > > > > I have reviewed the current CVS code and it looks quite different from > the patch you submitted. Any chance of you downloading the snapshot and > sending a context diff, rather than an ordinary one. I could apply that > easily. > > > > > I had a few issues with boolean handling in ODBC driver. > > > > 1) When a row is retrieved, and then a SQL_FETCH_FIRST is issued, the > check > > in convert.c > > does not consider the fact that the value in the field has been altered to > > be a '1' if the > > backend handed it a 't'. The net result being that the first row on any > > subsequent queries > > has all it's boolean set to 0. > > > > 2) I had issues with some utilities that, when casting from string to > > boolean, and having > > the global 'bools as char' set, require that -1 = True, and 0 = false. > > > > The following patch fixes both of these issues.. > > > > Cheers for building a fantastic product > > > > Aidan > > > > > > > > cvs diff convert.c (in directory > C:\CVS-POSTGRES\pgsql\src\interfaces\odbc\) > > Index: convert.c > > =================================================================== > > RCS file: > /home/projects/pgsql/cvsroot/pgsql/src/interfaces/odbc/convert.c,v > > retrieving revision 1.44 > > diff -r1.44 convert.c > > 295c295,307 > > < if (s[0] == 'T' || s[0] == 't') > > --- > > > /* Aidan Mountford (aidan@oz.to) 1/08/2001: > > > > > > > > >> if (s[0] == 'T' || s[0] == 't') > > <<< This wont work... > > > > > > When MoveFirst is called twice on > > one set of tuples, > > > this will have the effect of setting > > s[0] to 1 on the > > > first pass, and s[0] on the second. > > > > > > This is bad ;) > > > > > > */ > > > > > > if (s[0] == 'T' || s[0] == 't' || s[0] == > > '1') > > 415c428 > > < len = 1; > > --- > > > > > 418c431,449 > > < strcpy(rgbValueBindRow, value); > > --- > > > /* Aidan Mountford (aidan@oz.to) > > 1/08/2001: > > > > > > When returning a CHAR > > datatype, return -1 > > > instead of +1. > > > > > > That way if someone casts it > > back to a boolean > > > again - it will work. > > > */ > > > if ( *(char *) value = '1') > > > { > > > strcpy(rgbValueBindRow, "-1"); > > > len = 2; > > > } > > > else > > > { > > > strcpy(rgbValueBindRow, "0"); > > > len = 1; > > > } > > > > > 1160a1192,1193 > > > > > > > > 1161a1195 > > > > > > > > > ..................................................... > > . Cisco Certified Network Associate > > . Microsoft Certified Professional > > . 3Com 3Wizard > > . > > . Technical Director > > . MindVision Interactive Pty. Ltd. > > . http://www.mindvision.com.au > > . PH: +61-8-8212-9544 > > . FAX: +61-8-8212-9644 > > . 48 Light Square > > . Adelaide SA > > . AUSTRALIA 5000 > > ..................................................... > > > > "Remember, There is always scope for things to be worse" > > > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 2: you can get off all lists at once with the unregister command > > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > > > > -- > Bruce Momjian | http://candle.pha.pa.us > pgman@candle.pha.pa.us | (610) 853-3000 > + If your life is a hard drive, | 830 Blythe Avenue > + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly > [ Attachment, skipping... ] -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Here is the patchd I applied. It fixes item 1, but not item 2. Hiroshi wants to work on that one. > As Requested ... > > > > > -----Original Message----- > From: Bruce Momjian [mailto:pgman@candle.pha.pa.us] > Sent: Thursday, 2 August 2001 12:30 PM > To: Aidan Mountford > Cc: 'pgsql-patches@postgresql.org' > Subject: Re: [PATCHES] ODBC Boolean handling > > > > I have reviewed the current CVS code and it looks quite different from > the patch you submitted. Any chance of you downloading the snapshot and > sending a context diff, rather than an ordinary one. I could apply that > easily. > > > > > I had a few issues with boolean handling in ODBC driver. > > > > 1) When a row is retrieved, and then a SQL_FETCH_FIRST is issued, the > check > > in convert.c > > does not consider the fact that the value in the field has been altered to > > be a '1' if the > > backend handed it a 't'. The net result being that the first row on any > > subsequent queries > > has all it's boolean set to 0. > > > > 2) I had issues with some utilities that, when casting from string to > > boolean, and having > > the global 'bools as char' set, require that -1 = True, and 0 = false. > > > > The following patch fixes both of these issues.. > > > > Cheers for building a fantastic product > > > > Aidan > > > > > > > > cvs diff convert.c (in directory > C:\CVS-POSTGRES\pgsql\src\interfaces\odbc\) > > Index: convert.c > > =================================================================== > > RCS file: > /home/projects/pgsql/cvsroot/pgsql/src/interfaces/odbc/convert.c,v > > retrieving revision 1.44 > > diff -r1.44 convert.c > > 295c295,307 > > < if (s[0] == 'T' || s[0] == 't') > > --- > > > /* Aidan Mountford (aidan@oz.to) 1/08/2001: > > > > > > > > >> if (s[0] == 'T' || s[0] == 't') > > <<< This wont work... > > > > > > When MoveFirst is called twice on > > one set of tuples, > > > this will have the effect of setting > > s[0] to 1 on the > > > first pass, and s[0] on the second. > > > > > > This is bad ;) > > > > > > */ > > > > > > if (s[0] == 'T' || s[0] == 't' || s[0] == > > '1') > > 415c428 > > < len = 1; > > --- > > > > > 418c431,449 > > < strcpy(rgbValueBindRow, value); > > --- > > > /* Aidan Mountford (aidan@oz.to) > > 1/08/2001: > > > > > > When returning a CHAR > > datatype, return -1 > > > instead of +1. > > > > > > That way if someone casts it > > back to a boolean > > > again - it will work. > > > */ > > > if ( *(char *) value = '1') > > > { > > > strcpy(rgbValueBindRow, "-1"); > > > len = 2; > > > } > > > else > > > { > > > strcpy(rgbValueBindRow, "0"); > > > len = 1; > > > } > > > > > 1160a1192,1193 > > > > > > > > 1161a1195 > > > > > > > > > ..................................................... > > . Cisco Certified Network Associate > > . Microsoft Certified Professional > > . 3Com 3Wizard > > . > > . Technical Director > > . MindVision Interactive Pty. Ltd. > > . http://www.mindvision.com.au > > . PH: +61-8-8212-9544 > > . FAX: +61-8-8212-9644 > > . 48 Light Square > > . Adelaide SA > > . AUSTRALIA 5000 > > ..................................................... > > > > "Remember, There is always scope for things to be worse" > > > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 2: you can get off all lists at once with the unregister command > > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > > > > -- > Bruce Momjian | http://candle.pha.pa.us > pgman@candle.pha.pa.us | (610) 853-3000 > + If your life is a hard drive, | 830 Blythe Avenue > + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly > [ Attachment, skipping... ] -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 *** ./interfaces/odbc/convert.c Thu Aug 02 15:28:28 2001 --- ./interfaces/odbc/.#convert.c.1.44 Wed Aug 01 23:36:58 2001 *************** *** 292,301 **** { /* change T/F to 1/0 */ char *s = (char *) value; ! if (s[0] == 'T' || s[0] == 't') s[0] = '1'; else ! s[0] = '0'; } break; --- 292,314 ---- { /* change T/F to 1/0 */ char *s = (char *) value; ! /* Aidan Mountford (aidan@oz.to) 1/08/2001: ! ! >> if (s[0] == 'T' || s[0] == 't') <<< This wont work... ! ! When MoveFirst is called twice on one set of tuples, ! this will have the effect of setting s[0] to 1 on the ! first pass, and s[0] on the second. ! ! This is bad ;) ! ! */ ! ! if (s[0] == 'T' || s[0] == 't' || s[0] == '1') s[0] = '1'; else ! s[0] = '0'; ! } break; *************** *** 1158,1164 **** --- 1189,1198 ---- } else { + + used = stmt->parameters[param_number].used ? *stmt->parameters[param_number].used : SQL_NTS; + buffer = stmt->parameters[param_number].buffer; }