rlm_sql: default Acct-On/Off query for all backends is somewhat bogus

[Peter Nixon]

Stefan is absolutely correct about this. 

This query is also broken because it does "WHERE acctstoptime = NULL" when it 
should do "WHERE acctstoptime IS NULL". This bug does not exist in the 
Postgresql or Oracle backends (which I use in favour of MySQL) so either I 
fixed them at some point in the past or someone else broke MySQL...

Because I run heavily customised queries in production it has been a while 
since I took a close look at what we are shipping. I guess I should do so 
again.

I have committed a fix for the MySQL NULL issue..

-Peter

On Tue 29 Apr 2008, Stefan Winter wrote:
> Hi,
>
> the query states (in variants):
>
> ~        accounting_onoff_query = "\
> ~          UPDATE ${acct_table1} \
> ~          SET \
> ~             acctstoptime       =  '%S', \
> ~             acctsessiontime    =  unix_timestamp('%S') - \
> ~                                   unix_timestamp(acctstarttime), \
> ~             acctterminatecause =  '%{Acct-Terminate-Cause}', \
> ~             acctstopdelay      =  %{%{Acct-Delay-Time}:-0} \
> ~          WHERE acctsessiontime =  0 \
> ~          AND acctstoptime      =  NULL \
> ~          AND nasipaddress      =  '%{NAS-IP-Address}' \
> ~          AND acctstarttime     <= '%S'"
>
>
> and the acctsessiontime = 0 is bad: acctsessiontime gets updated
> whenever an interim record updates the session.
> The job of the query should be to close all open sessions, not just all
> open sessions that have been opened recently enough to not have received
> an update yet. I suggest to change the WHERE to
>
> ~          WHERE acctstoptime      =  NULL \
> ~          AND nasipaddress      =  '%{NAS-IP-Address}' \
> ~          AND acctstarttime     <= '%S'"
>
> (leaving out the acctsessiontime = 0 constraint)
>
> Greetings,
>
> Stefan Winter
> -
> List info/subscribe/unsubscribe? See
> http://www.freeradius.org/list/devel.html



-- 

Peter Nixon
http://peternixon.net/