<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1149/">https://reviewboard.asterisk.org/r/1149/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 26th, 2011, 5:12 p.m., <b>Tilghman Lesher</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This change reverts revision 236847, which was reported on the -dev list by Nic Colledge. You need to address his comments, before this can go forward:
http://lists.digium.com/pipermail/asterisk-dev/2009-December/041362.html</pre>
</blockquote>
<p>On March 26th, 2011, 5:33 p.m., <b>Tilghman Lesher</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It's probably better for you to specify a default in your table, instead of making a source code change. A blank is almost never a good option for an enum value (especially as it applies to Asterisk).</pre>
</blockquote>
<p>On March 28th, 2011, 8:50 p.m., <b>mlehner</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The checks I removed caused the eventtype field to always be NULL when the column was an integer. This is not an issue of a default value, nor does this patch revert revision 236847. The extraneous checks for integer values were added (to the best of my searching) when the file was renamed from cel_adpative_odbc.c to cel_odbc.c. There is no comment in the SVN history as to why the extra checks were added at that time. My patch removes these checks and actually brings the code closer to revision 236847.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">No, the checks you removed cause the field, if unspecified, to take the default value. In your case, it seems that the default value is NULL. If you change the default value, the insert will insert that value, instead.
I've already pointed you to the exact contents of revision 236847. I'll link to the log here, since you seem to be unwilling to accept that your patch reverts that revision:
$ svn pg --revprop -r 236847 svn:log http://svn.asterisk.org/svn/asterisk
When the field is blank, don't warn about the field being unable to be coerced, just skip the column.
(closes http://lists.digium.com/pipermail/asterisk-dev/2009-December/041362.html)
Reported by Nic Colledge on the -dev list, fixed by me.
SVNView is having trouble this evening, but the info can usually be found here: http://svnview.digium.com/svn/asterisk/trunk/cel/cel_adaptive_odbc.c?view=log&pathrev=236847</pre>
<br />
<p>- Tilghman</p>
<br />
<p>On March 26th, 2011, 11:43 a.m., mlehner wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By mlehner.</div>
<p style="color: grey;"><i>Updated 2011-03-26 11:43:38</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">event_type could not be an integer datatype in a ODBC database because an extra ast_strlen_zero() check was wrapped around the code that did the work. ast_strlen_zero() check is done in the second branch of the if statement, after the check for eventtype is done. Changes are very simple, just removing the "overzealous" ast_strlen_zero() check, while keeping braces to prevent compilation errors and warnings with variable re-definitions. </pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested on a local install of Asterisk 1.8.3.2. Compiles and creates the expected records in the database.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/tags/1.8.3.2/cel/cel_odbc.c <span style="color: grey">(311686)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1149/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>