[asterisk-dev] [Code Review] CEL cel_odbc backend fails to insert eventtype into integer or other numeric column

[kkm]

> On 2011-04-03 22:39:40, Tilghman Lesher wrote:
> > /trunk/cel/cel_odbc.c, line 562
> > <https://reviewboard.asterisk.org/r/1160/diff/2/?file=16055#file16055line562>
> >
> >     Why is this necessary?
> 
> kkm wrote:
>     Leap second. At least, linux and bsd systems have this in /usr/include/time.h:
>     
>     struct tm
>     {
>       int tm_sec;                   /* Seconds.     [0-60] (1 leap second) */
> 
> Tilghman Lesher wrote:
>     Right, but your database time code should handle these (exceptionally rare) cases correctly.  The danger of not handling them is that this introduces a possible billing error into the system.  Yes, it's only a second, but that could introduce a difference between billing an additional increment or not.
>     
>     If your database doesn't handle those cases correctly, then you need to file a bug, and include (possibly conditionally compiled) code to workaround that bug in the database in the meantime.
>     
>     I'm sure we have that bug in Asterisk, too, so it may be worth allocating an issue report to look at all of these cases and explicitly mention it in the code guidelines.
> 
> kkm wrote:
>     I agree it "should", but it's not in the SQL'03 standard, unfortunately. ODBC does support leap seconds (up to 2, AFAIK). MS SQL Server does not, even the latest, and not going to (by design, documented, compliant to SQL'03, no point filing bugs). MySQL seems to. Do not know about Oracle and Postgress. So I just wanted to stay on the safe side.
>     
>     Remove?

Or maybe add a .conf option? What do you think?


- kkm


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1160/#review3274
-----------------------------------------------------------


On 2011-04-04 13:13:25, kkm wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1160/
> -----------------------------------------------------------
> 
> (Updated 2011-04-04 13:13:25)
> 
> 
> Review request for Asterisk Developers and Tilghman Lesher.
> 
> 
> Summary
> -------
> 
> If eventtype is numeric in the database, it gets set to NULL, not to one of the numeric values as documented in cel_odbc.conf.sample.
> 
> 
> This addresses bug 18964.
>     https://issues.asterisk.org/view.php?id=18964
> 
> 
> Diffs
> -----
> 
>   /trunk/cel/cel_odbc.c 312554 
> 
> Diff: https://reviewboard.asterisk.org/r/1160/diff
> 
> 
> Testing
> -------
> 
> After preproduction testing, the change deployed to large scale production since March, 21.
> 
> 
> Thanks,
> 
> kkm
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110404/57e3199c/attachment.htm>