[asterisk-dev] [Code Review] 3346: [res_config_pgsql] Correct handling of nullable int fields in update_realtime

[Tilghman Lesher]

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


One issue is that this patch is against a released version, Asterisk 11, and not against trunk.  Since we've never previously supported NULL columns as such, I think we'd want this change in trunk only.

Also, while you've changed the update function, we'd also want the update2 function changed at the same time, since it has similar, though more versatile, functionality.


http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_pgsql.c
<https://reviewboard.asterisk.org/r/3346/#comment20845>

    This code appears to only work for integer types, not character types (char, varchar, bpchar, text) or even the numeric type.  I would think if we wanted support for NULL-able columns, we'd want to be able to support ALL column types, not just integers.


- Tilghman Lesher


On March 13, 2014, 10:06 a.m., zvision wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3346/
> -----------------------------------------------------------
> 
> (Updated March 13, 2014, 10:06 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23351
>     https://issues.asterisk.org/jira/browse/ASTERISK-23351
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch fixes handling of nullable int columns in update_realtime function. It checks if a value is empty and sets the column to NULL instead of '', which raises an error.
> Additionally, it checks for existence of the keyfield column instead of the first parameter column.
> 
> 
> Diffs
> -----
> 
>   http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_pgsql.c 410508 
> 
> Diff: https://reviewboard.asterisk.org/r/3346/diff/
> 
> 
> Testing
> -------
> 
> Only tested for successful compilation. Someone needs to confirm that the patch works fine.
> 
> 
> Thanks,
> 
> zvision
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140314/835de93a/attachment-0001.html>