Previous MySQL fix only half correct - Corrected MySQL syntax to = NULL

[Chris Moules]

I have created git patches for these two things.

Looks like there was only one error in the 243983349ba6d831da15677c46ff1f07fe977d68 commit. On initial examination I though 
there was a few more.

Regards

Chris

Chris Moules wrote:
> Sorry for not supplying decent patch files, I don't have time right now.
> 
> Chris
> 
> Looks like the follow commit fixed some issues but broke other:
> 
> commit 243983349ba6d831da15677c46ff1f07fe977d68
> Author: Alan T. DeKok <aland at freeradius.org>
> Date:   Wed Jan 28 14:59:42 2009 +0100
> 
>     Corrected MySQL syntax to = NULL
> 
> 
> -- 
> This patch seems to have blindly turned any instance of "IS NULL" into a 
> "= NULL". This is wrong.
> 
> The "IS NULL" syntax is correct for anything after the "WHERE" 
> statement. You MUST use this syntax for checking for the NULL
> value. See http://dev.mysql.com/doc/refman/5.0/en/working-with-null.html
> 
> The fix was correct in changing the assignment part of "UPDATE" 
> statements from "IS NULL" to "= NULL".
> 
> For example (snips from the commit log):
> Good
> ----
> @@ -19,7 +19,7 @@
>   allocate-clear = "UPDATE ${ippool_table} \
>    SET nasipaddress = '', pool_key = 0, \
>    callingstationid = '', username = '', \
> -  expiry_time IS NULL \
> +  expiry_time = NULL \
>    WHERE expiry_time <= NOW() - INTERVAL 1 SECOND
>    AND nasipaddress = '%{Nas-IP-Address}'"
> ----
> This is an assignment therefore we use the '=' assignment operator.
> 
> Bad
> ----
> @@ -59,7 +59,7 @@ allocate-update = "UPDATE ${ippool_table} \
>   SET nasipaddress = '%{NAS-IP-Address}', pool_key = '${pool-key}', \
>   callingstationid = '%{Calling-Station-Id}', username = '%{User-Name}', \
>   expiry_time = NOW() + INTERVAL ${lease-duration} SECOND \
> - WHERE framedipaddress = '%I' AND expiry_time IS NULL"
> + WHERE framedipaddress = '%I' AND expiry_time = NULL"
> ----
> This is a comparison and therefore we need to use the "IS NULL" syntax.
> 
> *-*-*-*-*-*-*-*-*
> 
> If you are still reading at this point, here is a mini-patch from myself 
> for the "sql/mysql/ippool.conf".
> 
>  ## The ORDER BY clause of this query tries to allocate the same IP-address
>  ## which user had last session...
>  allocate-find = "SELECT framedipaddress FROM ${ippool_table} \
> - WHERE pool_name = '%{control:Pool-Name}' AND expiry_time < NOW() \
> + WHERE pool_name = '%{control:Pool-Name}' AND (expiry_time < NOW() OR 
> expiry_time IS NULL) \
>   ORDER BY (username <> '%{User-Name}'), \
>   (callingstationid <> '%{Calling-Station-Id}'), \
>   expiry_time \
> 
> If you do not check _explicitly_ for NULL then nothing will match (if 
> expiry_time == NULL).
> As the 'ippool.sql' schema file sets 'default NULL' for expiry_time, 
> this is likely to be the case.
> 
> Example:
> 
> mysql> select COUNT(*) from radippool WHERE pool_name = 'dummy' AND 
> expiry_time < NOW();
> +----------+
> | COUNT(*) |
> +----------+
> |        0 |
> +----------+
> 1 row in set (0.00 sec)
> 
> mysql> select COUNT(*) from radippool WHERE pool_name = 'dummy' AND 
> (expiry_time < NOW() OR expiry_time IS NULL);
> +----------+
> | COUNT(*) |
> +----------+
> |      200 |
> +----------+
> 1 row in set (0.00 sec)
> 
> -
> List info/subscribe/unsubscribe? See 
> http://www.freeradius.org/list/devel.html
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-typo-in-MySQL-ippool.conf-and-revert-change-from.patch
Type: text/x-patch
Size: 1376 bytes
Desc: not available
URL: <http://lists.freeradius.org/pipermail/freeradius-devel/attachments/20090218/ac2bde33/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Patch-so-that-the-allocate-find-ippool-lookup-will.patch
Type: text/x-patch
Size: 1082 bytes
Desc: not available
URL: <http://lists.freeradius.org/pipermail/freeradius-devel/attachments/20090218/ac2bde33/attachment-0001.bin>