On March 17, 2014, 9:28 a.m., zvision wrote:
> > So, the changes look good. But that regression thing is .. not so nice.
> >
> > For trunk and up, we could change the behaviour. But we can't for 1.8..12.
> >
> > The options are:
> >
> > - leave the bug in place, only fix in trunk
> >
> > - change the behaviour mid-release
> >
> > - check which database we're using during connect-time
> >
> >
> > For that last option, we could use this:
> >
> > mysql> select cast('' as decimal) as num;
> > +-----+
> > | num |
> > +-----+
> > | 0 |
> > +-----+
> >
> > postgres=> select cast('' as decimal) as num;
> > ERROR: invalid input syntax for type numeric: ""
> > LINE 1: select cast('' as decimal) as num;
> >
> > And hope that this extra query doesn't cause any regressions anywhere.
> >
> > And then we'd have to choose between keeping this behaviour in trunk or
> > "fixing" it to write NULLs when dealing with numeric types.
> >
> >
> > My vote:
> > - add the above check or any other which consistently declares how the
> > database reacts
> > - apply this patch to trunk (removing the check again)
> >
> > But a second opinion would be nice.
>
> zvision wrote:
> I have uploaded a patch for trunk version. Please see my comments
> attached to the patch.
>
> Another option for non-trunk versions would be to introduce some config
> variable to avoid the check
> on every connnect. But I guess it is not an elegant solution anyway.
>
> zvision wrote:
> Actually, having a config option in res_odbc (something like
> backslash_is_escape) may be not so bad:
> - by default it would be disabled, not changing the current behavior,
> - enabling it would give a chance to PostgreSQL (and maybe other RDBMSes)
> users to run realtime SIP peers with no problems
> (I know they could have defined columns as text, but sample
> configurations provided use integers),
> - a SELECT CAST check is not neccessary - it will generate errors in
> database logs which may be confusing
> and may have other side effect we are not aware of.
>
> wdoekes wrote:
> I'm down with a patch that adds an option for that.
>
> zvision wrote:
> Great, I will prepare a patch and submit it with another review request.
> Any votes for the option name? Maybe:
> nullable_columns,
> null_support,
> enable_null,
> enable_null_values?
I do have an opinion about that.
Since MySQL is usually the odd one here, the option should reflect that.
Something like this:
allow_empty_string_in_nontext = yes ; mysql compatability default, set to no
for more strict databases
That should hopefully not give someone ideas that full NULL-support is now an
option.
- wdoekes
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3335/#review11245
-----------------------------------------------------------
On March 17, 2014, 11:12 a.m., zvision wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3335/
> -----------------------------------------------------------
>
> (Updated March 17, 2014, 11:12 a.m.)
>
>
> Review request for Asterisk Developers.
>
>
> Bugs: ASTERISK-23459
> https://issues.asterisk.org/jira/browse/ASTERISK-23459
>
>
> Repository: Asterisk
>
>
> Description
> -------
>
> This patch fixes setting nullable integer columns to NULL instead of an empty
> string, which fails for PostgreSQL, for example.
> The current code is supposed to do so, but the check is broken. The patch
> also allows the first column in the list to be a nullable integer.
> Also, the check for existence of a mandatory column checked for the first
> column in the list instead of the key field lookup column.
>
>
> Diffs
> -----
>
> http://svn.asterisk.org/svn/asterisk/trunk/res/res_config_odbc.c 410663
>
> Diff: https://reviewboard.asterisk.org/r/3335/diff/
>
>
> Testing
> -------
>
> Tested by me. Use case scenario: Asterisk + res_odbc + PostgreSQL backend,
> SIP realtime peers + regs.
> When a 'port' column in SIP regs (I assume this also applies when using
> sippeers only) is a nullable integer,
> Asterisk tries to write an empty string here during SIP endpoint
> deregistration.
>
>
> Thanks,
>
> zvision
>
>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
