[Bps-public-commit] dbix-searchbuilder branch, quote-table-names, repushed
Aaron Trevena
ast at bestpractical.com
Fri Sep 4 15:19:11 EDT 2020
The branch quote-table-names was deleted and repushed:
was 3def714b3743b9435b3c2da9b2c92d13093cb9fe
now f77f2b07393142284921ee929f758274f02a2dca
1: c402dff ! 1: f79ae6c Add support for automatic quoting of table names
@@ -1,8 +1,14 @@
Author: Aaron Trevena <aaron at aarontrevena.co.uk>
- Add table name quoting option for mysql
+ Add support for automatic quoting of table names
- Add table name quoting option, initially for mysql
+ MySQL 8 adds new reserved and keywords, that will cause queries with tables or
+ columns of those names to fail with syntax errors.
+ https://dev.mysql.com/doc/mysqld-version-reference/en/keywords-8-0.html
+
+ The new QuoteTableNames option will automatically quote tables in SQL created
+ by SearchBuilder, and will enabled if MySQL version 8 is detected. New helpers
+ simplify quoting table or column names.
diff --git a/lib/DBIx/SearchBuilder.pm b/lib/DBIx/SearchBuilder.pm
--- a/lib/DBIx/SearchBuilder.pm
@@ -24,8 +30,8 @@
Host => 'hostname',
User => 'dbuser',
- Password => 'dbpassword');
-+ Password => 'dbpassword',
-+ QuoteTableNames => 1 );
++ Password => 'dbpassword' );
++
# now $handle isa DBIx::SearchBuilder::Handle::mysql
=head1 DESCRIPTION
@@ -53,7 +59,7 @@
the handle will be automatically "upgraded" into that subclass.
+QuoteTableNames option will force all table names to be quoted if the driver subclass has a method
-+for quoting implemented.
++for quoting implemented. The mysql subclass will detect mysql version 8 and set the flag.
+
=cut
@@ -70,17 +76,27 @@
# So we need to explicitly call it
$self->{'DisconnectHandleOnDestroy'} = $args{'DisconnectHandleOnDestroy'};
-+ # Enable quotes table names
-+ $self->{'QuoteTableNames'} = $args{QuoteTableNames} if (defined $args{QuoteTableNames});
++ # Enable optional quoted table names
++ $self->{'QuoteTableNames'} = delete $args{QuoteTableNames} if (defined $args{QuoteTableNames});
+
my $old_dsn = $self->DSN || '';
my $new_dsn = $self->BuildDSN( %args );
@@
+ # Cache version info
+ $self->DatabaseVersion;
+
++ # force quoted tables for mysql 8
++ $self->{'QuoteTableNames'} = 1 if ($self->_RequireQuotedTables);
++
+ return 1;
+ }
+
+@@
push @bind, shift @pairs;
}
-+ $table = $self->QuoteName($table) if ($self->{'QuoteTableNames'});
++ $table = $self->QuoteName($table) if ($self->QuoteTableNames);
my $QueryString = "INSERT INTO $table";
$QueryString .= " (". join(", ", @cols) .")";
$QueryString .= " VALUES (". join(", ", @vals). ")";
@@ -134,6 +150,58 @@
@@
+ if ( $old_alias =~ /^(.*?) (\Q$args{'ALIAS2'}\E)$/ ) {
+ $args{'TABLE2'} = $1;
+ $alias = $2;
++ $args{'TABLE2'} =~ s/`//g;
+ }
+ else {
+ push @new_aliases, $old_alias;
+@@
+ if ( $old_alias =~ /^(.*?) ($args{'ALIAS2'})$/ ) {
+ $args{'TABLE2'} = $1;
+ $alias = $2;
+-
++ $args{'TABLE2'} =~ s/`//g;
+ }
+ else {
+ push @new_aliases, $old_alias;
+@@
+ # XXX: this situation is really bug in the caller!!!
+ return ( $self->_NormalJoin(%args) );
+ }
++ $args{TABLE2} = $self->QuoteName($args{TABLE2}) if ($self->QuoteTableNames);
+ $args{'SearchBuilder'}->{'aliases'} = \@new_aliases;
+ } elsif ( $args{'COLLECTION2'} ) {
+ # We're joining to a pre-limited collection. We need to take
+@@
+ # alias, apply them locally, then proceed as usual.
+ my $collection = delete $args{'COLLECTION2'};
+ $alias = $args{ALIAS2} = $args{'SearchBuilder'}->_GetAlias( $collection->Table );
+- $args{TABLE2} = $collection->Table;
++ my $table2 = $collection->Table;
++ $table2 = $self->QuoteName($table2) if ($self->QuoteTableNames);
++ $args{TABLE2} = $table2;
+
+ eval {$collection->_ProcessRestrictions}; # RT hate
+
+@@
+ $args{SearchBuilder}{restrictions}{$_} = $restrictions->{$_} for keys %{$restrictions};
+ } else {
+ $alias = $args{'SearchBuilder'}->_GetAlias( $args{'TABLE2'} );
++ $args{TABLE2} = $self->QuoteName($args{TABLE2}) if ($self->QuoteTableNames);
+ }
+
+ my $meta = $args{'SearchBuilder'}->{'left_joins'}{"$alias"} ||= {};
+@@
+ if ( $args{'TYPE'} =~ /LEFT/i ) {
+ my $alias = $sb->_GetAlias( $args{'TABLE2'} );
+ my $meta = $sb->{'left_joins'}{"$alias"} ||= {};
++ $args{TABLE2} = $self->QuoteName($args{TABLE2}) if ($self->QuoteTableNames);
+ $meta->{'alias_string'} = " LEFT JOIN $args{'TABLE2'} $alias ";
+ $meta->{'depends_on'} = $args{'ALIAS1'};
+ $meta->{'type'} = 'LEFT';
+@@
my $sb = shift;
$self->OptimizeJoins( SearchBuilder => $sb );
@@ -145,6 +213,26 @@
$processed{'main'} = 1;
@@
+
+ sub Fields {
+ my $self = shift;
+- my $table = lc shift;
++ my $tablename = shift;
++ my $table = lc $tablename;
+
+ unless ( $FIELDS_IN_TABLE{$table} ) {
+ $FIELDS_IN_TABLE{ $table } = [];
+- my $sth = $self->dbh->column_info( undef, '', $table, '%' )
+- or return ();
++ my $sth = $self->dbh->column_info( undef, '', $tablename, '%' );
++ unless ($sth) {
++ warn "couldn't get column info for $tablename / $table " . $sth->errstr;
++ return ();
++ }
+ my $info = $sth->fetchall_arrayref({});
+ foreach my $e ( @$info ) {
+ push @{ $FIELDS_IN_TABLE{ $table } }, $e->{'COLUMN_NAME'};
+@@
}
@@ -158,10 +246,16 @@
+
+# over-ride in subclass
+sub QuoteName {
-+ my $self = shift;
-+ return shift;
-+}
-+
++ my ($self, $name) = @_;
++ # use dbi built in quoting if we have a connection,
++ if ($self->dbh) {
++ return $self->dbh->quote_identifier($name);
++ }
++ warn "QuoteName called without a db handle";
++ return $name;
++}
++
++sub _RequireQuotedTables { return 0 };
+
=head2 DESTROY
@@ -208,9 +302,22 @@
+# over-rides inherited method
+sub QuoteName {
+ my ($self, $name) = @_;
++ # use dbi built in quoting if we have a connection,
++ if ($self->dbh) {
++ return $self->dbh->quote_identifier($name);
++ }
++
+ return sprintf('`%s`', $name);
+}
+
++
++sub _RequireQuotedTables {
++ my $self = shift;
++ if ( substr($self->DatabaseVersion, 0, 1) == 8 ) {
++ return 1;
++ }
++ return 0;
++}
+
1;
@@ -263,16 +370,20 @@
+=head2 QuotedTableName
+
-+Returns the name of current Table, including any quoting
++Returns the name of current Table, or the table provided as an argument, including any quoting
++ based on yje Handle's QuoteTableNames flag and driver method.
+=cut
+
+sub QuotedTableName {
-+ my $self = shift;
-+ return $self->{'_quoted_table'} if (defined $self->{'_quoted_table'});
-+ $self->{'_quoted_table'} = ( $self->_Handle->QuoteTableNames ) ?
-+ $self->_Handle->QuoteName($self->Table) : $self->Table ;
-+ return $self->{'_quoted_table'}
++ my ($self, $name) = @_;
++ unless ($name) {
++ return $self->{'_quoted_table'} if (defined $self->{'_quoted_table'});
++ $self->{'_quoted_table'} = ( $self->_Handle->QuoteTableNames ) ?
++ $self->_Handle->QuoteName($self->Table) : $self->Table ;
++ return $self->{'_quoted_table'}
++ }
++ return ( $self->_Handle->QuoteTableNames ) ? $self->_Handle->QuoteName($name) : $name ;
+}
=head2 _Handle
2: 89c850a ! 2: a5ea8fe Update tests for mysql 8 and table quoting option
@@ -1,9 +1,90 @@
Author: Aaron Trevena <ast at bestpractical.com>
- Add tests for tablename quoting in mysql
+ Update tests for mysql 8 and table quoting option
- Add tests for with/without tablename quoting flag set
+ Update tests to work with mysql 8 reserved words.
Add host/port ENV var options for testing database
+
+diff --git a/t/02searches_function.t b/t/02searches_function.t
+--- a/t/02searches_function.t
++++ b/t/02searches_function.t
+@@
+ GroupId integer
+ ) },
+ q{
+-CREATE TEMPORARY TABLE Groups (
++CREATE TEMPORARY TABLE `Groups` (
+ id integer primary key AUTO_INCREMENT,
+ Name varchar(36)
+ ) },
+
+diff --git a/t/02searches_joins.t b/t/02searches_joins.t
+--- a/t/02searches_joins.t
++++ b/t/02searches_joins.t
+@@
+ unless( should_test( $d ) ) {
+ skip "ENV is not defined for driver '$d'", TESTS_PER_DRIVER;
+ }
+-
+ my $handle = get_handle( $d );
+ connect_handle( $handle );
+ isa_ok($handle->dbh, 'DBI::db');
+@@
+ cleanup_schema( 'TestApp', $handle );
+
+ }} # SKIP, foreach blocks
+-
+ 1;
+
+
+@@
+ GroupId integer
+ ) },
+ q{
+-CREATE TEMPORARY TABLE Groups (
++CREATE TEMPORARY TABLE `Groups` (
+ id integer primary key AUTO_INCREMENT,
+ Name varchar(36)
+ ) },
+
+diff --git a/t/03cud_from_select.t b/t/03cud_from_select.t
+--- a/t/03cud_from_select.t
++++ b/t/03cud_from_select.t
+@@
+ skip "ENV is not defined for driver '$d'", TESTS_PER_DRIVER;
+ }
+
++ my $groups_table = ($d eq 'mysql') ? '`Groups`' : 'Groups';
+ my $handle = get_handle( $d );
+ connect_handle( $handle );
+ isa_ok($handle->dbh, 'DBI::db');
+@@
+ {
+ my $res = $handle->InsertFromSelect(
+ 'UsersToGroups' => ['UserId', 'GroupId'],
+- 'SELECT u.id as col1, g.id as col2 FROM Users u, Groups g WHERE u.Login LIKE ? AND g.Name = ?',
++ "SELECT u.id as col1, g.id as col2 FROM Users u, $groups_table g WHERE u.Login LIKE ? AND g.Name = ?",
+ '%a%', 'Support'
+ );
+ is( $res, 2 );
+@@
+ local $SIG{__WARN__} = sub {};
+ $handle->InsertFromSelect(
+ 'UsersToGroups' => ['UserId', 'GroupId'],
+- 'SELECT u.id, g.id FROM Users u, Groups g WHERE u.Login LIKE ? AND g.Name = ?',
++ "SELECT u.id, g.id FROM Users u, $groups_table g WHERE u.Login LIKE ? AND g.Name = ?",
+ '%a%', 'Support'
+ );
+ };
+@@
+ GroupId integer
+ ) },
+ q{
+-CREATE TEMPORARY TABLE Groups (
++CREATE TEMPORARY TABLE `Groups` (
+ id integer primary key AUTO_INCREMENT,
+ Name varchar(36)
+ ) },
diff --git a/t/11schema_records.t b/t/11schema_records.t
--- a/t/11schema_records.t
@@ -13,53 +94,25 @@
our (@AvailableDrivers);
-use constant TESTS_PER_DRIVER => 63;
-+use constant TESTS_PER_DRIVER => 66 * 4;
++use constant TESTS_PER_DRIVER => 66;
my $total = scalar(@AvailableDrivers) * TESTS_PER_DRIVER;
plan tests => $total;
-
-+foreach my $quote_tablenames (0,1) {
-+
- foreach my $d ( @AvailableDrivers ) {
- SKIP: {
- unless( has_schema( 'TestApp', $d ) ) {
-@@
- skip "ENV is not defined for driver '$d'", TESTS_PER_DRIVER;
- }
-
-- my $handle = get_handle( $d );
-+ my $handle = get_handle( $d, QuoteTableNames => $quote_tablenames );
- connect_handle( $handle );
- isa_ok($handle->dbh, 'DBI::db', "Got handle for $d");
-
@@
is($phone_collection->Next, undef);
}
-
--
+ ok($phone3->Delete, "Deleted phone $p3_id");
+
-+ # check mysql version
-+ my $version = $handle->DatabaseVersion(Short=>1);
-+ if ($quote_tablenames == 0 and lc($d) eq 'mysql' and $version =~ m/^8\./) {
-+ ok(1, 'skipping quoted reserved word tests for mysql 8');
-+ ok(1, 'skipping quoted reserved word tests for mysql 8');
-+ }
-+ else {
-+ my $group = TestApp::Group->new($handle);
-+ my $g_id = $group->Create( Name => 'Employees' );
-+ ok($g_id, "Got an id for the new group: $g_id");
-+ $group->Load($g_id);
-+ is($group->id, $g_id, "loaded group ok");
-+ }
++ my $group = TestApp::Group->new($handle);
++ my $g_id = $group->Create( Name => 'Employees' );
++ ok($g_id, "Got an id for the new group: $g_id");
++ $group->Load($g_id);
++ is($group->id, $g_id, "loaded group ok");
+
cleanup_schema( 'TestApp', $handle );
}} # SKIP, foreach blocks
-
-+}
- 1;
-
-
@@
id integer primary key,
Employee integer NOT NULL,
3: c282044 ! 3: f77f2b0 Update Changes with quoted table names update
@@ -13,4 +13,3 @@
1.68 2020-07-06
- Avoid segmentation faults on disconnect on MariaDB 10.2+
-
4: b293d7d < -: ------- Update join/alias logic to use optional table name quoting
5: 3def714 < -: ------- Update join/alias tests to use optional table name quoting
More information about the Bps-public-commit
mailing list