[Home]

Summary:ASTERISK-09512: [patch] Multirow results
Reporter:Tilghman Lesher (tilghman)Labels:
Date Opened:2007-05-24 17:11:33Date Closed:2007-05-31 10:06:00
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Functions/func_odbc
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20070530__func_odbc_multirow__2.diff.txt
Description:Changes the mode in which func_odbc works, by allowing multiple rows to be retrieved from a single query.
Comments:By: jmls (jmls) 2007-05-28 01:46:52

would it not be better to

a) Add an option to rowlimit to enable unlimited rows (rowlimit=unlimited)
b) default rowlimit to something like 25 or 100, in order to make anyone wanting unlimited rows having to specify that option

This would stop any number of idiots like me merrily filling up memory and creating performance problems by trying to read millions of records ..
By: Tilghman Lesher (tilghman) 2007-05-28 02:41:35

There are any number of ways to make Asterisk blow up.  I cannot begin to protect an administrator from herself.

However, multirow mode is NOT the default mode of operation; you need to enable it, and, if you enable that mode, then you should set a rowlimit accordingly.
By: Terry Wilson (twilson) 2007-05-30 00:51:52

In my testing, the patch doesn't currently work.  I'm trying to get to the bottom of it, and am not done yet.  What I've found so far is that the rowlimit, if not specified ends up being 0 which bypasses the for loop:

for (y=0;y<rowlimit;y++) {

in the read function.  If I set a rowlimit and try to return a multi-row result (mine is a single column result that has 3 rows), it loops through with y=0, y=1 (counting from top of the loop), then bombs out with the SQL Get Data error @ 391 or so (which also sets y = -1 which is what we will set the number of rows to even if we had vaild rows previously).

Also, if I set rowlimit=1 and do a limit 1 in my readsql (to avoid previously mentioned problems, but leaving multirow), ODBC_FETCH doesn't seem to work?  Perhaps my dialplan is wrong?  Here is what I've got:

[test]
exten => _X.,1,Answer
exten => _X.,n,Set(result=${ODBC_test()})
exten => _X.,n,NoOp(result id is ${result})
exten => _X.,n,NoOp(rows: ${ODBCROWS})
exten => _X.,n,Set(name=${ODBC_FETCH(${result})})
exten => _X.,n,NoOp(name is ${name})
exten => _X.,n,Hangup

produces:

   -- Executing [123@test:1] Answer("OSS/dsp", "") in new stack
<< Console call has been answered >>
   -- Executing [123@test:2] Set("OSS/dsp", "result=1") in new stack
   -- Executing [123@test:3] NoOp("OSS/dsp", "result id is 1") in new stack
   -- Executing [123@test:4] NoOp("OSS/dsp", "rows: 1") in new stack
   -- Executing [123@test:5] Set("OSS/dsp", "name=") in new stack
   -- Executing [123@test:6] NoOp("OSS/dsp", "name is ") in new stack
   -- Executing [123@test:7] Hangup("OSS/dsp", "") in new stack


table is just a test table called names with an id column and a name column.  readsql=SELECT name from names (with a LIMIT 1 for above test).  Works fine doing a single row, so no db connectivity issues.  Put in some debug lines and it is getting to the end_acf_read label without any gotos.

Anyway, looking forward to this.  Getting ready to go to bed right now.  Can take a look at making some fixes if you are busy with other things (though, from what I saw of you at the devcon, you may have it fixed by the time I wake up...) :-)
By: Tilghman Lesher (tilghman) 2007-05-30 11:00:54

Okay, fixed.
By: Terry Wilson (twilson) 2007-05-30 12:27:52

Ok, getting much better.  ODBCROWS is off by one (for loop starts at 0, so the sprintf needs to be y+1 instead of y) and calling ODBCFinish results in this segfault:
Program terminated with signal 11, Segmentation fault.
#0  0x00fe62d8 in odbc_datastore_free (data=0xb7558f38) at func_odbc.c:103
103             while ((row = AST_LIST_REMOVE_HEAD(result, list))) {
(gdb) bt full
#0  0x00fe62d8 in odbc_datastore_free (data=0xb7558f38) at func_odbc.c:103
       cur = (struct odbc_datastore_row *) 0x31
       result = (struct odbc_datastore *) 0xb7558f38
       row = (struct odbc_datastore_row *) 0x1b
       __PRETTY_FUNCTION__ = "odbc_datastore_free"
#1  0x00fe7af5 in exec_odbcfinish (chan=0x86771f8, data=0xb7558f38) at func_odbc.c:542
No locals.
#2  0x080ba50c in pbx_exec (c=0x86771f8, app=0x85ed890, data=0xb7558f38) at pbx.c:531
       res = 130
       saved_c_appl = 0x0
       saved_c_data = 0x0
#3  0x080bd5b1 in pbx_extension_helper (c=0x86771f8, con=0x0, context=0x8677380 "test", exten=0x86773d0 "123", priority=8, label=0x0,
   callerid=0x8677e00 "", action=E_SPAWN) at pbx.c:1757
       e = (struct ast_exten *) 0x8653570
       app = (struct ast_app *) 0x85ed890
       res = -1219118192
       q = {incstack = {0x0 <repeats 128 times>}, stacklen = 0, status = 5, swo = 0x0, data = 0x0, foundcontext = 0x8677380 "test"}
       passdata = "1\000\000\000\000\000\000\000\001\000\000\000\000\000\000\000j^\000\000\000\000\000\000\001", '\0' <repeats 8166 times>
       matching_action = 0
       __PRETTY_FUNCTION__ = "pbx_extension_helper"
#4  0x080be64c in ast_spawn_extension (c=0x86771f8, context=0x8677380 "test", exten=0x86773d0 "123", priority=8, callerid=0x8677e00 "")
   at pbx.c:2210
No locals.
ASTERISK-1  0x080beb1c in __ast_pbx_run (c=0x86771f8) at pbx.c:2310
       dst_exten = '\0' <repeats 20 times>, "?1\"", '\0' <repeats 65 times>, "\020\000\000\000(q/", '\0' <repeats 81 times>, "?\017\"\000\f\000\000\000?(\"\000\000\000\000\000?_/\000 q/\000\f\000\000\000?U??K\"\000 q/\000\f\000\000\000\020\000\000\000??\001\000H\177g\b?\"3\000??3\000\000\000\000\000j^\000\000\030?U?\231\227\017\b"
       pos = 0
       digit = 0
       found = 1
---Type <return> to continue, or q <return> to quit---
       res = 0
       autoloopflag = 0
       error = 0
       __PRETTY_FUNCTION__ = "__ast_pbx_run"
ASTERISK-2  0x080bf937 in pbx_thread (data=0x86771f8) at pbx.c:2543
       c = (struct ast_channel *) 0x86771f8
ASTERISK-3  0x080fa93b in dummy_start (data=0x8677e40) at utils.c:545
       __cancel_buf = {__cancel_jmp_buf = {{__cancel_jmp_buf = {3403764, 0, -1219118192, -1219120200, 475164309, -1559726898},
     __mask_was_saved = 0}}, __pad = {0xb755b470, 0x0, 0x0, 0x0}}
       __cancel_routine = (void (*)(void *)) 0x8067e79 <ast_unregister_thread>
       __cancel_arg = (void *) 0xb755bb90
       not_first_call = 0
       ret = (void *) 0x0
       a = {start_routine = 0x80bf920 <pbx_thread>, data = 0x86771f8,
 name = 0x8677e50 "pbx_thread", ' ' <repeats 11 times>, "started at [ 2564] pbx.c ast_pbx_start()"}
ASTERISK-4  0x003303db in start_thread () from /lib/libpthread.so.0
No symbol table info available.
ASTERISK-5  0x0028a26e in clone () from /lib/libc.so.6
No symbol table info available.

Other than that, though everything works like a charm, thanks!

EDIT: I'm assuming that it segfaults because we already use AST_LIST_REMOVE_HEAD in the read function on the result set?


By: Tilghman Lesher (tilghman) 2007-05-30 19:45:14

New patch uploaded.  The ODBCFinish is fixed; it was a minor oversight.  The number of rows returned, though, was a bit trickier.  I suspect you set a rowlimit that was higher than the actual number of rows (or set no rowlimit)?  If it was cut off by the rowlimit, then the number should have been correct.  It was only in early termination that the limit was off, I think.
By: Terry Wilson (twilson) 2007-05-31 00:30:02

Will try new patch tomorrow morning.  Although I had tried it with a limit 1 in the sql with a rowlimit=1 first which alerted me to the problem.  ODBCROWS was set to 0.
By: Terry Wilson (twilson) 2007-05-31 00:43:32

Actually went ahead and tested now.  Works with my test case.  I'd say commit it (as it will limit the number of queries I have to do in my dialplan by a factor of at least 3 or 4).  :-)  Great work, thanks!  Does ODBCFinish need to be called explicitly for each result set, or will channel teardown free up all of the memory?
By: Tilghman Lesher (tilghman) 2007-05-31 08:26:16

Three different things will cause the memory to be freed:
1) Calling ODBC_FETCH when there are no rows left to be fetched.
2) Calling ODBCFinish()
3) Channel destruction

ODBCFinish was added, because you might want to free the memory associated with a resultset prior to hanging up the channel and also prior to fetching
all the rows.
By: Tilghman Lesher (tilghman) 2007-05-31 10:06:00

Committed, rev 66734.