Alex Vandiver via RT wrote:
Show quoted text> <URL:
https://rt.cpan.org/Ticket/Display.html?id=96902 >
>
> On 07/02/2014 10:31 PM, Michelle Sullivan via RT wrote:
>
>> Ok when testing RT 4.0 with RT servers on a different network from the
>> DB servers it's unusably slow.
>>
>> The problem is down to calling ->Fields()
>>
>> Here's a patch that makes Fields load only the fields for tables that
>> are requested - rather than from every schema (which can be huge -
>> someone recently told me that they have 119 schemas in the same database!)
>>
>
> Thanks for the patch. I've applied it to master, and DBIx-SearchBuilder
> 1.65_01 is on its way to CPAN with it.
>
I can't actually see it in the source - though I can see the revision bump.
You know I have been thinking about the patch (and the docs) ... There
is a general problem we are missing...
DBIx::SearchBuilder was originally build by Jesse, and originally
against MySQL to make things easier for people (particularly those that
didn't know what they were doing)
No I don't know how many DBs are case insensitive, but I suspect it's
very few (notable: MySQL and derivatives.) However, the whole function
is written to lowercase everything (indicating case insensitivity is the
expected 'norm'.. which isn't the case.) I wrote a second patch for it
for the FreeBSD ports (here:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=191734 ) which removed
all case-sensitivity from it... I think that would actually be the
correct way to go as a 'half way house' that it's in with my initial
patch could cause more confusion and certainly will result in more
people using it in an incorrect way. Thoughts?
Here is my 'complete fix' patch that removes the case sensitivity from
DBIx::SearchBuilder->Fields (actually:
DBIx::SearchBuilder::Handle->Fields()) and an update to the documentation.
diff -Nru DBIx-SearchBuilder-1.65.orig/lib/DBIx/SearchBuilder/Handle.pm
DBIx-SearchBuilder-1.65/lib/DBIx/SearchBuilder/Handle.pm
--- DBIx-SearchBuilder-1.65.orig/lib/DBIx/SearchBuilder/Handle.pm
2014-07-13 06:51:05.000000000 -0500
+++ DBIx-SearchBuilder-1.65/lib/DBIx/SearchBuilder/Handle.pm
2014-07-13 06:42:11.000000000 -0500
@@ -1430,16 +1430,16 @@
my $self = shift;
my $table = shift;
- unless ( keys %FIELDS_IN_TABLE ) {
- my $sth = $self->dbh->column_info( undef, '', '%', '%' )
+ unless ( keys %FIELDS_IN_TABLE && exists $FIELDS_IN_TABLE{$table} ) {
+ my $sth = $self->dbh->column_info( undef, '', $table, '%' )
or return ();
my $info = $sth->fetchall_arrayref({});
foreach my $e ( @$info ) {
- push @{ $FIELDS_IN_TABLE{ lc $e->{'TABLE_NAME'} } ||= [] },
lc $e->{'COLUMN_NAME'};
+ push @{ $FIELDS_IN_TABLE{ $e->{'TABLE_NAME'} } ||= [] },
$e->{'COLUMN_NAME'};
}
}
- return @{ $FIELDS_IN_TABLE{ lc $table } || [] };
+ return @{ $FIELDS_IN_TABLE{ $table } || [] };
}
diff -Nru DBIx-SearchBuilder-1.65.orig/lib/DBIx/SearchBuilder.pm
DBIx-SearchBuilder-1.65/lib/DBIx/SearchBuilder.pm
--- DBIx-SearchBuilder-1.65.orig/lib/DBIx/SearchBuilder.pm
2014-07-13 06:51:05.000000000 -0500
+++ DBIx-SearchBuilder-1.65/lib/DBIx/SearchBuilder.pm 2014-07-13
06:50:28.000000000 -0500
@@ -1785,9 +1785,13 @@
=head2 Fields TABLE
-Return a list of fields in TABLE, lowercased.
+Return a list of fields in TABLE
-TODO: Why are they lowercased?
+Note: Previous versions returned all column names as lowercase, this
+call is now case sensitive in both the table name and the column names
+returned. If your database is not case sensitive (eg: MySQL) this
+will have no effect. If your database is case sensitive (eg: Oracle and
+PostgreSQL) you have to use the correct Case at all times!
=cut
@@ -1801,6 +1805,8 @@
Returns true if TABLE has field FIELD.
Return false otherwise
+Note: Both TABLE and FIELD are case-sensitive (See: L</Fields>)
+
=cut
sub HasField {
Show quoted text>
>> Second bug (which this patch mostly exaggerates rather than fixing) is
>> it is valid to have tables of "Table", "table", "taBLE" which are all
>> different by using the lowercase table name in the hash only the last
>> table will be recorded, and worse any application wanting fields on
>> 'Table' where 'taBLE' was stored would get the wrong fields.
>>
>
> Noted in the commit message. I would be surprised if
> DBIx::SearchBuilder did not make that assumption already -- and it is
> certainly asking for confusion, if nothing else. :)
>
There was actually a knock-on effect... Before the patch if you called
'HasField' with *non* lowercased table and *non* lowercased field
(column) names it would always return FALSE (this was undocumented.)
Eg before patching (using PostgreSQL) these will all return FALSE:
CREATE TABLE "Table1" ("Column1" BIGINT DEFAULT NOT NULL);
DBIx::SearchBuilder->HasField(TABLE => "Table1", FIELD => "column1");
DBIx::SearchBuilder->HasField(TABLE => "table1", FIELD => "column1");
DBIx::SearchBuilder->HasField(TABLE => "Table1", FIELD => "Column1");
This, however, would return true:
DBIx::SearchBuilder->HasField(TABLE => "table1", FIELD => "Column1");
(This is because Fields() cached everything lowercase, but HasFields()
did not lowercase the input parameters.)
Show quoted text>
>> Performance increase for running RT4 with just 3 schemas on PostgreSQL
>> is a single ticket load will go from 4-6 minutes (without the patch) to
>> 7 seconds (with the patch) - the RT front end to DB server round trip
>> time was just 20ms during this testing.
>>
>
> Local testing (with a pg host 20ms away) shows the first page load time
> drops from 130s to 7s. Subsequent page loads are under 3s; if you are
> still seeing slowness after the first query to each process, it implies
> that your mod_perl children are never being reused -- which may yield
> additional speedups if fixed.
>
This seems correct - however I have followed all the docs and used my
knowledge and can't find the issue (SORBS uses ModPerl2 extensively, so
I know how I *should* do it - however I am also not using Mason, or many
of the public modules, so it could be I missed something.)
Show quoted text>
>> The response on the RT Users
>> mailing list was a less than helpful "it's not designed to run on a
>> different network" and no further responses even when submitting the
>> patch suggestion.
>>
>
> Apologies -- I had your email earmarked to respond, but hadn't had time
> to do so.
>
No problem, and am sorry, I (like many) want answers immediately,
however unlike most I do know what it's like (especially running SORBS)
so I give people a week or 2 and if I haven't even got an ack I treat as
"They missed my post" - resend - then a week later "Ok, now I'm just
being ignored" and this has been frustrating me a lot, I have a chance
on making all public support for $employer (redacted as this post will
be in the public space) into RT, and if I don't get this all fixed and
working, it'll all go to SalesForce (which shows they are looking at
paying for something...! ie not just another Freebie)
Regards,
Michelle
--
Michelle Sullivan
http://www.mhix.org/
