Summary:	ASTERISK-03436: [patch] [post-1.4] updates on res_odbc
Reporter:	Ivan F. Martinez (ivanfm)	Labels:
Date Opened:	2006-02-09 18:56:17.000-0600	Date Closed:	2006-11-12 23:58:47.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/Configuration
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 20061112__bug6453.diff.txt ( 1) asterisk_res_odbc_3.patch ( 2) asterisk_res_odbc_5.patch ( 3) asterisk_res_odbc_6.patch ( 4) asterisk_res_odbc_7.patch
Description:	support for configurable test_sql (for use with databases that does not suport only "select 1") add generic diagnostic funtion to show database error information function to create new statement handle ****** ADDITIONAL INFORMATION ****** if this patch is accepted I can check other routines, tho use this diagnostic and create statement calls. this changes are tested with IBM DB2, using via unixodbc
Comments:	By: Tilghman Lesher (tilghman) 2006-02-11 11:42:10.000-0600 res_odbc is currently undergoing a substantial restructuring.  Please see http://svn.digium.com/svn/asterisk/team/tilghman/res_odbc_rewrite/ By: Ivan F. Martinez (ivanfm) 2006-02-13 18:52:02.000-0600 new patch based on res_odbc_rewrite By: Tilghman Lesher (tilghman) 2006-03-06 14:27:53.000-0600 Looks like you added odbc_init() back in.  It's been removed for a reason. By: Tilghman Lesher (tilghman) 2006-03-22 11:03:41.000-0600 Reporter seems to have lost interest. By: Ivan F. Martinez (ivanfm) 2006-03-28 05:18:46.000-0600 Sory for the delay, but I still has interest in this. If you have a reason to remove the code which allocate the SQL_HANDLE_ENV please explain to me, because in my database this alloc is really necessary before allocating a new connection. Then I can change my patch to work with more databases. I have searched again (now) in the code on http://svn.digium.com/svn/asterisk/team/tilghman/res_odbc_rewrite/res/res_odbc.c And still does not have any place where the call res = SQLAllocHandle(SQL_HANDLE_ENV, SQL_NULL_HANDLE, &cls->env); is made, or something with same result. By: Tilghman Lesher (tilghman) 2006-03-28 11:36:26.000-0600 I just committed another update to my branch, one of which was the missing ENV allocation handle. By: Ivan F. Martinez (ivanfm) 2006-03-28 20:42:36.000-0600 updated patch By: Andrey S Pankov (casper) 2006-04-10 13:50:48 Corydon76: any useful ideas here for your odbc_rewrite effort? By: Tilghman Lesher (tilghman) 2006-04-11 00:24:51 Customizing the sanity check is certainly an interesting idea.  If that portion of the patch were isolated, that could go in.  The rest of it, though, seems like pointless wrappers that do nothing but increase the barrier to entry for new programmers and increase the required stack size for ODBC apps. By: Ivan F. Martinez (ivanfm) 2006-04-11 06:32:16 the odbc_diag from my patch is a very usefull to report errors. A single place to give the complete information about the error. the alloc_statement too, its an simple thing but make easier to reuse in other places. good simple funcions in res_odbc make possible to reuse it in other places like cdr_odbc. I have started the conversion in  cdr_odbc in 1.2 but stopped when cory pointed to  use the new trunk. I stopped the rewrite to wait a final version of res_odbc. The patch in res_odbc_rewrite trunk can require more memory as Cory said, but permit better reuse of connections as it create a reusable pool of connections, a necessary thing for a heavy loaded server. By: Serge Vecher (serge-v) 2006-05-04 10:27:33 ivanfm: can you please update your patch, since Corydon's work has been merged into the trunk already? Thanks. By: Tilghman Lesher (tilghman) 2006-05-19 11:14:51 No response from reporter.  Reopen when you have a new patch to post. By: Ivan F. Martinez (ivanfm) 2006-06-17 08:39:21 I updated the patch to trunk By: Ivan F. Martinez (ivanfm) 2006-06-17 08:41:02 I also refactored app_voicemail to use the new functions. By: Tilghman Lesher (tilghman) 2006-06-17 09:45:22 More things will need to happen before this patch can be considered: 1) Coding guidelines (for example, no C++ comments) 2) You have todo's in your patch.  These need to be fleshed out in code. 3) You do not need to prefix the module name on ast_log() in the message; that is taken care of for you.  The only place where this could be conceivably useful is in ast_verbose() calls. 4) Instead of creating a new function odbc_prepare_statement that suffers the same deficiencies as the old model (namely a reconnect causing all statements to become invalid), I'd like you to use the prepare callback model.  res_config_odbc.c has an example of this to which I'd like to see all modules that use ODBC be converted. By: Tilghman Lesher (tilghman) 2006-06-22 10:07:14 No response from reporter. By: Ivan F. Martinez (ivanfm) 2006-06-22 12:23:06 we have talked on irc abou the use of callback and I have sent to you a link to check my new implementation before I post here, probably you did not received the link. I have tried again to talk to you without success. I will attach the current patch with the callback changed as we discussed before. By: Ivan F. Martinez (ivanfm) 2006-08-22 19:37:37 updated to trunk 40871. By: jmls (jmls) 2006-10-31 12:22:57.000-0600 Corydon76, is this something that is still of interest ? By: Tilghman Lesher (tilghman) 2006-10-31 14:49:40.000-0600 If we could break out the custom sanity-check SQL and the conversions to app_voicemail, this could go into trunk immediately.  The other changes I still don't see the point of. By: jmls (jmls) 2006-10-31 15:45:31.000-0600 ivanfm, would you agree with Corydon76 ? By: Ivan F. Martinez (ivanfm) 2006-11-12 19:34:39.000-0600 I can break the things, but not now, only in the next month I will have some time to make changes and do testing before submiting a new version of patch. By: Tilghman Lesher (tilghman) 2006-11-12 23:58:47.000-0600 Sanity check merged in 47530.