Previous MySQL fix only half correct - Corrected MySQL syntax to = NULL
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@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)
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@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
Chris Moules wrote:
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.
Applied, thanks. Alan DeKok.
participants (2)
-
Alan DeKok -
Chris Moules