Gossamer Forum
Home : Products : Gossamer Links : Development, Plugins and Globals :

bug in dir Converter

(Page 1 of 2)
> >
Quote Reply
bug in dir Converter
Hello webmaster33

I found a small bug in your dir Concerter plugin on line 299

message:

Can't call method "rows" on an undefined value at /path/to/url/cgi-bin/links/admin/Plugins/DirName_converter.pm line 299


i checked & noticed that the variable $cat_table_name could not be used in the query defined on line 299
Quote Reply
Re: [ridesworld] bug in dir Converter In reply to
Could you enable debugging, and show what it says? Also, what is line 299?

Cheers

Andy (mod)
andy@ultranerds.co.uk
Want to give me something back for my help? Please see my Amazon Wish List
GLinks ULTRA Package | GLinks ULTRA Package PRO
Links SQL Plugins | Website Design and SEO | UltraNerds | ULTRAGLobals Plugin | Pre-Made Template Sets | FREE GLinks Plugins!
Quote Reply
Re: [ridesworld] bug in dir Converter In reply to
Ok, sorry I noticed some potential problems that I feel need pointing out. Here goes:

1) "use lib" is used in the perl module but it isn't needed.

2) Data::Dumper debugging code is left in the code. That's not a major issue but it is loading a module that isn't needed. The reason for pointing this out too is that GT::Dumper is a better option for compatiblity sake (yes I know DAta::Dumper is standard but still).

3) Excessive use of "my" at the top. my() is actually a sub-routine so there about 13 sub-routine calls when this could be reduced to one or two by declaring more than one variable at once.

4) "not" used where "!" would be more appropriate.

5) Unnecessary escaping of single quotes inside qq||

6) Incorrect use of "ne" when comparing numerics. Should use "!="

7) Incorrect use of eq when comparing numerics. Should use "=="

8) Unnecessary use of parenthesis in regexes when no capturing is needed (this affects speed).

9) Escpaing of forward slashes in regexes could be avoided by changing the substitution delimiter.

10) Use of my() outside a sub-routine (thanks Yogi).

That's it for now - there may be other things I've missed. I think the plugin would fail under mod_perl.

This is intended as constructive criticizm.

Last edited by:

Paul: Mar 12, 2003, 8:51 AM
Quote Reply
Re: [ridesworld] bug in dir Converter In reply to
Hi Ridesworld,

1) Please enable debugging, and post the text here. It would be helpful.
2) Also, what database do you use?
3) What plugin version do you use?
4) What Links SQL version do you use?

Best regards,
Webmaster33


Paid Support
from Webmaster33. Expert in Perl programming & Gossamer Threads applications. (click here for prices)
Webmaster33's products (upd.2004.09.26) | Private message | Contact me | Was my post helpful? Donate my help...

Last edited by:

webmaster33: Mar 12, 2003, 10:14 AM
Quote Reply
Re: [webmaster33] bug in dir Converter In reply to
here the details with debug mode on:

A fatal error has occured:


Can't call method "rows" on an undefined value at /path/to/cgi-bin/links/admin/Plugins/DirName_converter.pm line 299.

Please enable debugging in setup for more details.

Stack Trace
======================================
Links (28550): Links::environment called at /path/to/cgi-bin/links/admin/Links.pm line 431 with no arguments.
Links (28550): Links::fatal called at /path/to/cgi-bin/links/admin/Plugins/DirName_converter.pm line 299 with arguments
(Can't call method "rows" on an undefined value at /path/to/cgi-bin/links/admin/Plugins/DirName_converter.pm line 299.
).
Links (28550): Plugins::DirName_converter::convert called at admin.cgi line 216 with no arguments.
Links (28550): main::plugin called at admin.cgi line 52 with no arguments.
Links (28550): main::main called at admin.cgi line 24 with no arguments.


System Information
======================================
Perl Version: 5.006001
Links SQL Version: 2.1.2
DBI.pm Version: 1.21
Persistant Env: mod_perl (1) SpeedyCGI (0)
GT::SQL::error = Failed to execute query: 'UPDATE Category SET Name = SUBSTRING_INDEX(Full_Name,'/',-1);' Reason: Table 'links.category' doesn't exist
@INC =
z:/WORK/WWW.INDYGO.HU/CGI-BIN/lsql/admin
/path/to/cgi-bin/links/admin
/usr/local/lib/perl5/5.6.1/i686-linux
/usr/local/lib/perl5/5.6.1
/usr/local/lib/perl5/site_perl/5.6.1/i686-linux
/usr/local/lib/perl5/site_perl/5.6.1
/usr/local/lib/perl5/site_perl
.

CGI INPUT
======================================
do => plugin
do_convert => Execute selected steps
func => convert
plugin => DirName_converter
step2 => 2



I use LinksSQL 2.1.2

and Plugin version 1.0.4

I see on the log that the script will use the table category, but it is defined as Category in the script & in my database.

I use MySQL.
Quote Reply
Re: [ridesworld] bug in dir Converter In reply to
This is a bug in the code I missed. Webmaster33 is using do_query which is not advised as it does not handle table prefixes properly. This is probably related to your problem.
Quote Reply
Re: [Paul] bug in dir Converter In reply to
No, the problem is with mod_perl environment!
I will explain in my answers to your long post.

Best regards,
Webmaster33


Paid Support
from Webmaster33. Expert in Perl programming & Gossamer Threads applications. (click here for prices)
Webmaster33's products (upd.2004.09.26) | Private message | Contact me | Was my post helpful? Donate my help...
Quote Reply
Re: [ridesworld] bug in dir Converter In reply to
The problem is that you use mod_perl environment.
I did not plan it for mod_perl usage, but I will correct that problem in next version.

Would you be open test the beta version of DirName converter plugin under mod_perl, on a test configuration?
This would help me to make it mod_perl compatible.

Best regards,
Webmaster33


Paid Support
from Webmaster33. Expert in Perl programming & Gossamer Threads applications. (click here for prices)
Webmaster33's products (upd.2004.09.26) | Private message | Contact me | Was my post helpful? Donate my help...
Quote Reply
Re: [webmaster33] bug in dir Converter In reply to
The problem is not mod_perl, but it's on line 269, where you use 'category' instead of 'Category'.

Ivan
-----
Iyengar Yoga Resources / GT Plugins
Quote Reply
Re: [yogi] bug in dir Converter In reply to
...and it also doesn't handle prefixes like I said originally :)
Quote Reply
Re: [Paul] bug in dir Converter In reply to
Paul, did you ever note this code at 269?:
Code:
my $cat_table_name = "$prefix" . "category";
If Yogi would quote it, you would not say, that prefixes are not handled... Tongue

Anyway likely Yogi is right, that "Category", should be used instead "category", but I'm still investigating the problem, because in my database all table names are lowercase (under Windows).

Best regards,
Webmaster33


Paid Support
from Webmaster33. Expert in Perl programming & Gossamer Threads applications. (click here for prices)
Webmaster33's products (upd.2004.09.26) | Private message | Contact me | Was my post helpful? Donate my help...
Quote Reply
Re: [webmaster33] bug in dir Converter In reply to
Quote:
Paul, did you ever note this code at 269?:

No I missed it, thanks for pointing it out.

I'm still unclear as to why you are not using the update() method which automatically handles prefixes.

Last edited by:

Paul: Mar 12, 2003, 2:10 PM
Quote Reply
Re: [Paul] bug in dir Converter In reply to
Well, likely that one with update method would also work:
Code:
my $cat_sth = $cat_db->update ( \"Name = SUBSTRING_INDEX(Full_Name,'/',-1)" );

Best regards,
Webmaster33


Paid Support
from Webmaster33. Expert in Perl programming & Gossamer Threads applications. (click here for prices)
Webmaster33's products (upd.2004.09.26) | Private message | Contact me | Was my post helpful? Donate my help...
Quote Reply
Re: [webmaster33] bug in dir Converter In reply to
I'm not sure that's the correct syntax.

I expect you'd need a condition.
Quote Reply
Re: [Paul] bug in dir Converter In reply to
I'm not sure either, but logically that syntax should work.
Anyway using a condition object is also possible, if that syntax is not working.
I'm sure your suggestion to use update, would make it elegant solution, but I don't feel the need to change to update, until that solution works correctly.

Best regards,
Webmaster33


Paid Support
from Webmaster33. Expert in Perl programming & Gossamer Threads applications. (click here for prices)
Webmaster33's products (upd.2004.09.26) | Private message | Contact me | Was my post helpful? Donate my help...
Quote Reply
Re: [yogi] bug in dir Converter In reply to
In Reply To:
The problem is not mod_perl, but it's on line 269, where you use 'category' instead of 'Category'.
Possible, but I'm still investigating the problem, because in my database all table names are lowercase (under Windows).

You are using Linux. Could you check the table names (if are lowercase or not) under Linux?
I think it's an OS issue, that under Windows the database file names of MySQL, are treated as all lower case. On Linux the filename is case sensitive, that's why it likely does not works under Linux, and why works for me under Windows.

Best regards,
Webmaster33


Paid Support
from Webmaster33. Expert in Perl programming & Gossamer Threads applications. (click here for prices)
Webmaster33's products (upd.2004.09.26) | Private message | Contact me | Was my post helpful? Donate my help...
Quote Reply
Re: [webmaster33] bug in dir Converter In reply to
The table names in Links SQL are all upper case, e.g. Category.

Ivan
-----
Iyengar Yoga Resources / GT Plugins
Quote Reply
Re: [yogi] bug in dir Converter In reply to
As I wrote, under Windows, in database table names are all lowercase.
Otherwise I know, that the first uppercase should be used (Category).

Best regards,
Webmaster33


Paid Support
from Webmaster33. Expert in Perl programming & Gossamer Threads applications. (click here for prices)
Webmaster33's products (upd.2004.09.26) | Private message | Contact me | Was my post helpful? Donate my help...

Last edited by:

webmaster33: Mar 12, 2003, 3:03 PM
Quote Reply
Re: [webmaster33] bug in dir Converter In reply to
 

Last edited by:

Paul: Mar 12, 2003, 5:00 PM
Quote Reply
Re: [Paul] bug in dir Converter In reply to
Sigh! Shocked

Paul, you should read MySQL docs (v3.23):
Quote:
Database names and table names are case sensitive in MySQL on operating systems that have case sensitive filenames (like most Unix systems). If you have a problem remembering table names, adopt a consistent convention, such as always creating databases and tables using lowercase names.
Hopefully next time you read the docs, before you post such opinion you said: "they are uppercase on *all* platforms"...

Best regards,
Webmaster33


Paid Support
from Webmaster33. Expert in Perl programming & Gossamer Threads applications. (click here for prices)
Webmaster33's products (upd.2004.09.26) | Private message | Contact me | Was my post helpful? Donate my help...
Quote Reply
Re: [webmaster33] bug in dir Converter In reply to
Hello

I've tested Links-SQL on a local webserver with windows 2000, apache, perl, php & mysql

the problem is that table names will be renamed lowercase!

this will also happen with using prefixes.


I think you should use an update() statement in your scripting.

this will work properly:

my $cat_sth = $cat_db->do_query ("UPDATE Category SET Name = SUBSTRING_INDEX(Full_Name,'/',-1);"); on my Linux machine.
Quote Reply
Re: [ridesworld] bug in dir Converter In reply to
Try something like this:

Code:
my $cat_sth = $cat_db->update({ Name => \"SUBSTRING_INDEX(Full_Name, '/', -1)" });

Not tested :)
Quote Reply
Re: [Paul] bug in dir Converter In reply to
Seems logical. IMO should work.

Best regards,
Webmaster33


Paid Support
from Webmaster33. Expert in Perl programming & Gossamer Threads applications. (click here for prices)
Webmaster33's products (upd.2004.09.26) | Private message | Contact me | Was my post helpful? Donate my help...
Quote Reply
Re: [ridesworld] bug in dir Converter In reply to
Quote:
I've tested Links-SQL on a local webserver with windows 2000, apache, perl, php & mysql
the problem is that table names will be renamed lowercase!
this will also happen with using prefixes.
Exactly. I wrote the same in post #20.

I will send you a small bugfix update of the plugin to your email address.

Best regards,
Webmaster33


Paid Support
from Webmaster33. Expert in Perl programming & Gossamer Threads applications. (click here for prices)
Webmaster33's products (upd.2004.09.26) | Private message | Contact me | Was my post helpful? Donate my help...
Quote Reply
Re: [Paul] bug in dir Converter In reply to
Paul,
Here are my answers to your comments in post #3:

Quote:
1) "use lib" is used in the perl module but it isn't needed.
It is needed in my development process. Otherwise when checking for syntax with -c option I will get error. It can be commented out after release, though.
But causes NO problems.

Quote:
2) Data::Dumper debugging code is left in the code. That's not a major issue but it is loading a module that isn't needed. The reason for pointing this out too is that GT::Dumper is a better option for compatiblity sake (yes I know DAta::Dumper is standard but still).
Data::Dumper is used for debugging in development phase. I don't care about GT::Dumper (which module is not standard in Perl, but Data::Dumpler is). It can be commented out after release, tough. But since Data::Dumper is standard module in Perl releases will not cause any problems.
In overall, it causes NO problems.

Quote:
3) Excessive use of "my" at the top. my() is actually a sub-routine so there about 13 sub-routine calls when this could be reduced to one or two by declaring more than one variable at once.
You are right, that they could be declared in one step.
But usage of 13 "my" declarations does simply not matter in fact of the speed. The optimization you suggest is unnecessary, since in the full execution time these 13 "my"-s are executed in less than 1%. Optimization is unnecessary for such small speed gain.
Did you done profiling for DirName converter? I didn't, but I could say I'm 99% sure, that these 13 "my"s execution time, will be even not noted in the profiler result...
In overall the "my"s does NOT cause problems.

Quote:
4) "not" used where "!" would be more appropriate.
This does not mean any problems...
It's my programming style, and I like using not instead of ! for easier readibility.
Anyway it causes NO problems.

Quote:
5) Unnecessary escaping of single quotes inside qq||
Yeah, that's true. They are unnecessary. Earlier it was printed within '' quotes, then qq||; was added around the text later. Also I copied text from install.pm, where as you know '' quotes are used originally. I could correct all of them, but it does not cause any problems, so I don't bother about it currently.
In overall, it causes NO problems.

Quote:
6) Incorrect use of "ne" when comparing numerics. Should use "!="
Not incorrect. Both gives the same result, when comparing 2 integers.
When using 'eq' or 'ne' for number comparison, numbers are converted to strings automatically, then comparison is done as string.

Tough ne is likely slower a bit, but gives more readibility of the code (at least for me).
Finally since the speed decrease in under millisec, you don't have to bother about it. Also this plugin is not speed dependent, so speed does not matter.
In overall, it causes NO problems.

Quote:
7) Incorrect use of eq when comparing numerics. Should use "=="
Same as above, with the addition, that there are NO number comparisions using 'eq'!!!
You meant this code part?: $IN->param("step1") eq "1" .... also in html sourse: <INPUT TYPE="CHECKBOX" NAME="step1" VALUE="1">
All input parameters you get from the form are strings, so you statement is simply not true.
So it causes NO problems.

Quote:
8) Unnecessary use of parenthesis in regexes when no capturing is needed (this affects speed).
I checked the code, and yeah you are right, those parenthesis are unnecessary.
Those parentheses was used for grouping purpose, not for capturing reasons. I should use (?:) style groupings to avoid capturing or remove the parentheses from those regexps. Both would be saticfactory.
However yeah, the parenthesis capture affects the speed. But it does not mean so much speed decrease to bother with. Also DirName converter plugin has no speed demand at the moment.
I will place comments to the code to correct the captures, if there will be need for speed related optimizations in the plugin.

Quote:
9) Escpaing of forward slashes in regexes could be avoided by changing the substitution delimiter.
Yes, substitution delimiter could be changed. The code could be more readable.
However it causes NO problems in the code execution.

Quote:
10) Use of my() outside a sub-routine (thanks Yogi).
perldoc perlfunc says: "A my declares the listed variables to be local (lexically) to the enclosing block, file, or eval."
perdoc perlsub says: "You may declare my variables at the outermost scope of a file to hide any such identifiers from the world outside that file."
Having a my variable used in a module file is correct.
Paul, Yogi: you should try out this small example Tongue:

Code:
my $text = "Test my variable in file\n";
package Foo::Bar;
use strict;
print "Content-Type: text/plain\n\n";
print $text;
my $text = "Test my variable in function\n";
print $text;
So your comment was NOT valid in CGI mode. Unfortunately under mod_perl the my() declaration usage on file level will not work if it's used under mod_perl. The mod_perl is the only reason to correct it, and avoid usage of my() declarations on file level.

Quote:
I think the plugin would fail under mod_perl.
I went & read some docs, and it seems you are right.
But mod_perl is only reason not to use my() declarations on file level.


Final conclusions:
Paul,
Thanks very much for taking your precious time to find bugs in my plugin!
In overall, I aggreed you that may be problems in 1.5 points. In point 8) and (in case the plugin is executed under mod_perl) in point 10).
The point 8) affects the speed a bit, but does not affect the correct work of the plugin.
I would say, the only one point, which has potential problems is the point 10), which affects the mod_perl usage.
Anyway, I appreciate your constructive criticizm!

Go ahead Paul, do hundreds of tests on my plugin, discover all the possible problems it has, and tell me all the problems!
I always appreciate if somebody does my work for free. Cool

Best regards,
Webmaster33


Paid Support
from Webmaster33. Expert in Perl programming & Gossamer Threads applications. (click here for prices)
Webmaster33's products (upd.2004.09.26) | Private message | Contact me | Was my post helpful? Donate my help...

Last edited by:

webmaster33: Mar 13, 2003, 5:10 AM
> >