[Home]

Summary:ASTERISK-04311: [patch] changed app_addons_sql_mysql to set status variable instead of returning -1
Reporter:Roy Sigurd Karlsbakk (rkarlsba)Labels:
Date Opened:2005-06-01 11:10:24Date Closed:2005-06-07 13:35:15
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) new.patch
Description:app_addons_sql_mysql normally returns -1 on error, making it impossible to trap the error from the dialplan. this changes that to set a channel variable to OK or NOTOK.

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

disclaimer will be sent as soon as I'm near a fax again, prolly tomorrow.
Comments:By: Roy Sigurd Karlsbakk (rkarlsba) 2005-06-01 11:13:59

To use this, create a macro as something like this. this allows for a query and up to three return rows.

[macro-dblookup]
exten => s,1,MYSQL(Connect CONNID ${DBHOST} ${DBUSER} ${DBPASS} ${DBNAME})
exten => s,2,GotoIf($[ "${MYSQL_STATUS}" = "NOTOK" ]?generic_error,s,1)
exten => s,3,MYSQL(Query RESULTID ${CONNID} ${ARG1})
exten => s,4,GotoIf($[ "${MYSQL_STATUS}" = "NOTOK" ]?generic_error,s,1)
exten => s,5,GotoIf($[ "${ARG2}" = "" ]?generic_error,s,1)
exten => s,6,GotoIf($[ "${ARG3}" != "" ]?10)
exten => s,7,GotoIf($[ "${ARG4}" != "" ]?12)
exten => s,8,Set(MYSQLFETCHSTRING=Fetch FETCHID ${RESULTID} ${ARG2})
exten => s,9,Goto(13)
exten => s,10,Set(MYSQLFETCHSTRING=Fetch FETCHID ${RESULTID} ${ARG2} ${ARG3})
exten => s,11,Goto(13)
exten => s,12,Set(MYSQLFETCHSTRING=Fetch FETCHID ${RESULTID} ${ARG2} ${ARG3} ${ARG4})
exten => s,13,MYSQL(${MYSQLFETCHSTRING})
exten => s,14,GotoIf($[ "${MYSQL_STATUS}" = "NOTOK" ]?generic_error,s,1)
exten => s,15,MYSQL(Clear ${RESULTID})
exten => s,16,GotoIf($[ "${MYSQL_STATUS}" = "NOTOK" ]?generic_error,s,1)
exten => s,17,MYSQL(Disconnect ${CONNID})
exten => s,18,GotoIf($[ "${MYSQL_STATUS}" = "NOTOK" ]?generic_error,s,1)
By: Tilghman Lesher (tilghman) 2005-06-01 11:43:13

It's actually kind of silly to set up a buffer, if you're only ever going to copy a single static string into it.  Just do:

if (result == 0) {
   pbx_builtin_setvar_helper(chan, "MYSQL_STATUS", "OK");
} else {
   pbx_builtin_setvar_helper(chan, "MYSQL_STATUS", "FAIL");
}

Don't make your code more complex than it needs to be.
By: Roy Sigurd Karlsbakk (rkarlsba) 2005-06-01 11:52:27

it really wasn't any point of doing it complex. I just wanted it to look good, so I used a temporary variable for that. As long as it works, i don't care :)
By: Michael Jerris (mikej) 2005-06-01 11:55:46

The basics of this are fine.  I agree with corydon's comments on setting up the buffer, also, OK vs. NOTOK?  Let's try to have the returns make a little more sense, OK and FAIL are fine I suppose.  Is there any other pattern of return var values that we should stick too?  

Junky-  Why did you fail this on architecture?
By: Clod Patry (junky) 2005-06-02 06:56:32

Based on cory's note.
And i was thinking of return option COMPLETED and FAILED ?
By: Tilghman Lesher (tilghman) 2005-06-02 08:17:43

junky:  I don't think we need to go for the most explicit return options; short and sweet are optimal.  Perhaps we even should go for the shortest possible: 0 and -1.  That gives us the ability to do $[${MYSQL_STATUS} < 0]
for a failure detection.
By: Kevin P. Fleming (kpfleming) 2005-06-02 14:28:42

I agree, let's make the code as simple as possible, and make the MYSQL_STATUS variable be 0 or non-zero as it is internally. With that, you can use

GotoIF($[${MYSQL_STATUS}] ? ... : ...)

without even using a conditional operator.
By: Roy Sigurd Karlsbakk (rkarlsba) 2005-06-07 05:56:51

yeah, why not.
new supapatch attached
By: Kevin P. Fleming (kpfleming) 2005-06-07 13:34:32

Committed to CVS HEAD with minor mods, thanks!