Peter - a couple of things about the MySQL stuff: 1) I just noticed that the ./docs/examples/mysql.sql schema in the 2.0 HEAD doesn't look right: # # Table structure for table 'radippool' # CREATE TABLE radippool ( id int(11) unsigned NOT NULL auto_increment, pool_name varchar(30) NOT NULL, FramedIPAddress varchar(15) NOT NULL default , NASIPAddress varchar(15) NOT NULL default , CalledStationId VARCHAR(30) NOT NULL, CallingStationID VARCHAR(30) NOT NULL, expiry_time DATETIME NOT NULL default '0000-00-00 00:00:00', username varchar(64) NOT NULL default , pool_key varchar(30) NOT NULL, PRIMARY KEY (id) ); Note the missing default values. The 1.1.7 branch has a more correct looking version. 2) Also, I just noticed in the 2.0 mysql-ippool-dialup.conf, the allocate-clear query has ... allocate-clear = "UPDATE ${ippool_table} \ SET NASIPAddress = '', pool_key = 0, \ CallingStationID = '', username = '', \ expiry_time = '0000-00-00 00:00:00' \ WHERE expiry_time <= NOW() - INTERVAL 1 SECOND AND nasipaddress = '%{Nas-IP-Address}'" Note the lower case 'nasipaddress'. Pretty please for to remember that MySQL on Windows is cASe SenSITiVe when it comes to column names. Well, by default. Yes, one can make the names case insensitive, but that can cause problems. And yeah, there aren't many of us using MySQL on Windows behind FR, but I happen to be one of them. Don't ask. :) FYI, slippool.conf in 1.1.7 has the correct case-ification. -- hugh
On Fri 20 Jul 2007, Hugh Messenger wrote:
Peter - a couple of things about the MySQL stuff:
1) I just noticed that the ./docs/examples/mysql.sql schema in the 2.0 HEAD doesn't look right:
# # Table structure for table 'radippool' # CREATE TABLE radippool ( id int(11) unsigned NOT NULL auto_increment, pool_name varchar(30) NOT NULL, FramedIPAddress varchar(15) NOT NULL default , NASIPAddress varchar(15) NOT NULL default , CalledStationId VARCHAR(30) NOT NULL, CallingStationID VARCHAR(30) NOT NULL, expiry_time DATETIME NOT NULL default '0000-00-00 00:00:00', username varchar(64) NOT NULL default , pool_key varchar(30) NOT NULL, PRIMARY KEY (id) );
Note the missing default values. The 1.1.7 branch has a more correct looking version.
Ahh, yes. I caught and fixed this in 1.1.7 and forgot HEAD.. Grrr.. I hate working on multiple branches :-(
2) Also, I just noticed in the 2.0 mysql-ippool-dialup.conf, the allocate-clear query has ...
allocate-clear = "UPDATE ${ippool_table} \ SET NASIPAddress = '', pool_key = 0, \ CallingStationID = '', username = '', \ expiry_time = '0000-00-00 00:00:00' \ WHERE expiry_time <= NOW() - INTERVAL 1 SECOND AND nasipaddress = '%{Nas-IP-Address}'"
Note the lower case 'nasipaddress'. Pretty please for to remember that MySQL on Windows is cASe SenSITiVe when it comes to column names. Well, by default. Yes, one can make the names case insensitive, but that can cause problems.
And yeah, there aren't many of us using MySQL on Windows behind FR, but I happen to be one of them. Don't ask. :)
FYI, slippool.conf in 1.1.7 has the correct case-ification.
ok. Its fixed. I am beginning to think that for 2.0, maybe we should just make EVERYTHING lowercase for ALL database backends. It would be one less compatibility issue to deal with. Would anyone take issue with that? (I realise it has backwards compatibility issues to MySQL on windows users. Does it affect anyone else though?) Cheers -- Peter Nixon http://peternixon.net/
Peter Nixon wrote:
ok. Its fixed. I am beginning to think that for 2.0, maybe we should just make EVERYTHING lowercase for ALL database backends. It would be one less compatibility issue to deal with. Would anyone take issue with that? (I realise it has backwards compatibility issues to MySQL on windows users. Does it affect anyone else though?)
I hATe ThE usE oF ExesSiVe Cap... Ah, the heck with it. Make it all lowercase. Alan DeKok.
Alan DeKok said:
Sent: Monday, July 23, 2007 9:03 AM To: FreeRadius users mailing list Subject: Re: 2.0 mysql.sql
Peter Nixon wrote:
ok. Its fixed. I am beginning to think that for 2.0, maybe we should just make EVERYTHING lowercase for ALL database backends. It would be one less compatibility issue to deal with. Would anyone take issue with that? (I realise it has backwards compatibility issues to MySQL on windows users. Does it affect anyone else though?)
I hATe ThE usE oF ExesSiVe Cap...
Working 24*7 is RADIUS-Attribute-Land must drive you crazy!
Ah, the heck with it. Make it all lowercase.
As long as we do some checking first, as Peter mentions. Make sure it's not just MySQL on Windows which has the case issue. And as long as we provide a simple update_case.sql to modify the tables appropriately when upgrading to 2.x. Personally I'd be very loath to change things just for cosmetic reasons. It's not like db col names are things we have to type very often. And the risk of unforeseen back compat issues is very real. The only real wrinkle for me is I'll have to run a non-standard 1.1.7 config, as I'm about to set up 2.0 as my secondary, so I'll have 1.1.7 and 2.0 talking to the same (case sensitive) db. No biggie, just an example of one of those back compat issues.
Alan DeKok.
-- hugh
On Mon 23 Jul 2007, Hugh Messenger wrote:
Alan DeKok said:
Sent: Monday, July 23, 2007 9:03 AM To: FreeRadius users mailing list Subject: Re: 2.0 mysql.sql
Peter Nixon wrote:
ok. Its fixed. I am beginning to think that for 2.0, maybe we should
just
make EVERYTHING lowercase for ALL database backends. It would be one
less
compatibility issue to deal with. Would anyone take issue with that? (I realise it has backwards compatibility issues to MySQL on windows users. Does it affect anyone else though?)
I hATe ThE usE oF ExesSiVe Cap...
Working 24*7 is RADIUS-Attribute-Land must drive you crazy!
Ah, the heck with it. Make it all lowercase.
As long as we do some checking first, as Peter mentions. Make sure it's not just MySQL on Windows which has the case issue. And as long as we provide a simple update_case.sql to modify the tables appropriately when upgrading to 2.x.
Given that we already HAD some case issues (that you asked me to fix) I am nt sure that its a big deal.
Personally I'd be very loath to change things just for cosmetic reasons. It's not like db col names are things we have to type very often. And the risk of unforeseen back compat issues is very real.
Well, we decided about 6 months ago that we were going to make every cosmetic change we could think of for the 2.0 release in the hope that we wouldn't need to make any more in the future :-)
The only real wrinkle for me is I'll have to run a non-standard 1.1.7 config, as I'm about to set up 2.0 as my secondary, so I'll have 1.1.7 and 2.0 talking to the same (case sensitive) db. No biggie, just an example of one of those back compat issues.
Oh my god it burns..... Why would you do that to yourself??? :-D -- Peter Nixon http://peternixon.net/
While we are on the subject of schemas (schemi?) ... Would it be worth adding some indexing to the basic sqlippool table? At the moment, only the 'id' is indexed, by virtue of being the primary key. I should imagine that even a medium sized provider could end up with quite large radippool tables, at a thousand or so entries for every four class C's. I'm just a corner shop ISP, but even I'll end up with about 10k entries by the time I'm done migrating everything from Funk. With 3 queries per login, 1 each for every stop, start and update, plus whatever work the backend provisioning is doing to check and update the pools, that's a lot of extra work for the db with no indexing. Also, unless there is some bizarre circumstance I haven't thought of where the same IP would appear twice, make FrameIPAddress a 'unique' index, to prevent accidentally adding the same IP twice when provisioning the table. -- hugh
On Mon, 2007-07-23 at 10:11 -0500, Hugh Messenger wrote:
While we are on the subject of schemas (schemi?) ...
Would it be worth adding some indexing to the basic sqlippool table? At the moment, only the 'id' is indexed, by virtue of being the primary key.
I should imagine that even a medium sized provider could end up with quite large radippool tables, at a thousand or so entries for every four class C's. I'm just a corner shop ISP, but even I'll end up with about 10k entries by the time I'm done migrating everything from Funk.
With 3 queries per login, 1 each for every stop, start and update, plus whatever work the backend provisioning is doing to check and update the pools, that's a lot of extra work for the db with no indexing.
Also, unless there is some bizarre circumstance I haven't thought of where the same IP would appear twice, make FrameIPAddress a 'unique' index, to prevent accidentally adding the same IP twice when provisioning the table.
VRFs with overlapping address space are one possible use I can think of; handing out special IPs which redirect to "you are banned" or null route is another. Don't make it unique. Do index it.
On Mon 23 Jul 2007, Phil Mayers wrote:
On Mon, 2007-07-23 at 10:11 -0500, Hugh Messenger wrote:
While we are on the subject of schemas (schemi?) ...
Would it be worth adding some indexing to the basic sqlippool table? At the moment, only the 'id' is indexed, by virtue of being the primary key.
I should imagine that even a medium sized provider could end up with quite large radippool tables, at a thousand or so entries for every four class C's. I'm just a corner shop ISP, but even I'll end up with about 10k entries by the time I'm done migrating everything from Funk.
With 3 queries per login, 1 each for every stop, start and update, plus whatever work the backend provisioning is doing to check and update the pools, that's a lot of extra work for the db with no indexing.
Also, unless there is some bizarre circumstance I haven't thought of where the same IP would appear twice, make FrameIPAddress a 'unique' index, to prevent accidentally adding the same IP twice when provisioning the table.
VRFs with overlapping address space are one possible use I can think of; handing out special IPs which redirect to "you are banned" or null route is another.
Don't make it unique. Do index it.
I disagree. It DOES need to be unique and indexed. In the case where you are virtualising things with VRF (Which I do in production) you need to add an extra VRF key to the queries (In my case I use calledstationid) and you would adjust the index/constraints accordingly. -- Peter Nixon http://peternixon.net/
On Mon 23 Jul 2007, Alan DeKok wrote:
Peter Nixon wrote:
ok. Its fixed. I am beginning to think that for 2.0, maybe we should just make EVERYTHING lowercase for ALL database backends. It would be one less compatibility issue to deal with. Would anyone take issue with that? (I realise it has backwards compatibility issues to MySQL on windows users. Does it affect anyone else though?)
I hATe ThE usE oF ExesSiVe Cap...
Ah, the heck with it. Make it all lowercase.
Done :-) -- Peter Nixon http://peternixon.net/
participants (4)
-
Alan DeKok -
Hugh Messenger -
Peter Nixon -
Phil Mayers