[Home]

Summary:ASTERISK-05903: [patch] odbcstorage + voicemail + !mysql = no good
Reporter:Jason Parker (jparker)Labels:
Date Opened:2005-12-25 23:02:45.000-0600Date Closed:2006-04-11 17:26:34
Priority:CriticalRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) message_data_type.diff
Description:Corydon said it best - "ODBC stuff was all written and tested using mysql... and it shows..."

Let's see if we can't make that statement a little less true.


****** ADDITIONAL INFORMATION ******

I set this as crash, because it does cause (at least) pgsql to crash, due to #1.

There are currently two issues I can think of, with pgsql and/or mssql.

1) On pgsql, by default, you will get a segfault when trying to insert a voicemail record.  This appears to be caused by line ~1247, where it uses SQL_BINARY to save the voicemail binary file.  Changing this to use SQL_VARLONGBINARY fixes this on pgsql.  A similar thing happens on mssql, but I don't believe it crashes (perhaps the crash is in the pgsql lib?) - the same change fixes this on mssql as well.

2) On mssql, when you insert a SQL_CHAR (or SQL_VARCHAR?) param which is an empty string, unixodbc/freetds/mssql (I don't know which one) tries to use a char(0), which mssql will promptly fail with.
Comments:By: Jason Parker (jparker) 2005-12-25 23:09:26.000-0600

There is a problem with #1 that I'd like to discuss.

Changing the SQL_BINARY in only one place (it's used three times), is probably not the best idea.  However, I have tried changing the other two, and I believe that it was overflowing the SQLULEN colsize variable.  I believe that SQL_VARLONGBINARY is a 64 bit var, where SQL_BINARY is only 32, and I also believe that SQLULEN is (on most platforms) 64.

I did briefly look for a 64 bit SQL len type, but didn't get very far.  fdlen is size_t - what are the implications here?


Now for #2.  The obvious easy fix, is to just check the length, if it's 0, ignore that field altogether.  I would be okay with that, however, some people may expect it to not insert null values, and it may break for people who already use it.  Perhaps we can give a 1 time message (similar to when one uses USE_ODBC_STORAGE but not EXTENDED_ODBC_STORAGE)?
By: Jason Parker (jparker) 2006-01-09 11:41:58.000-0600

Update: per ASTERISK-6023, 1) also affects MySQL.
By: Vadim Berezniker (kryptolus) 2006-01-09 12:02:16.000-0600

I don't think it really affects MySQL because mysql doesn't seem to care if it's SQL_BINARY or not. However, others servers do. If it's changed to SQL_VARLONGBINARY, it will then work properly with MSSQL (and it won't brake MySQL I presume) so it seems like a safe change.
By: Jason Parker (jparker) 2006-01-09 12:46:52.000-0600

I've copied the following from ASTERISK-6023, so that can be closed:


At around line 1244 of app_voicemail.c where the 'recording' parameter is bound, its type should be SQL_LONGVARBINARY and not SQL_BINARY. Using SQL_BINARY fails when running against MSSQL. According to http://dev.mysql.com/doc/refman/5.0/en/myodbc-data-types.html, the proper type for mysql should be SQL_LONGVARBINARY as well. I guess mysql doesn't really care so it accepts SQL_BINARY as well, but MSSQL is a bit more pickier.

This affects 1.2.1, but I looked at the trunk and it seems to be the same there.
By: Vadim Berezniker (kryptolus) 2006-01-10 05:25:20.000-0600

I attached a patch for the incorrect data type if that helps in any way.
This patch is enough to fix storing voicemail message in MSSQL.
By: Omar Belakhdar (belakdar) 2006-01-24 08:45:53.000-0600

What is the type chosen for the recording field in the case of pgsql: bytea or lo?

Thanks.

Omar.


By: Omar Belakhdar (belakdar) 2006-01-27 08:49:41.000-0600

Good afternoon,
I'm really sorry to opportunate you but as I read this bug report, it seems you are one of few people who are using odbc with voicemail. I try to do the same with psql and all things are working correctly, the only thing is not working is the fact that I cannot read/check messages from the database.
See please the bug report ASTERISK-6286343.

Many thanks for your time.

Omar.


By: Tilghman Lesher (tilghman) 2006-04-11 17:26:34

Committed to 1.2, merged to trunk