result source metadata (RT#107462)
- Fix incorrect SQL generated with invalid {rows} on complex resultset
operations, generally more robust handling of rows/offset attrs
+ - Fix silent failure to retrieve a primary key (RT#80283) or worse:
+ returning an incorrect value (RT#115381) in case a rdbms-side autoinc
+ column is declared as PK with the is_auto_increment attribute unset
- Fix incorrect $storage state on unexpected RDBMS disconnects and
other failure events, preventing clean reconnection (RT#110429)
- Make sure exception objects stringifying to '' are properly handled
# they can be fused once again with the final return
$to_insert = { %$to_insert, %$prefetched_values };
- # FIXME - we seem to assume undef values as non-supplied. This is wrong.
- # Investigate what does it take to s/defined/exists/
my %pcols = map { $_ => 1 } $source->primary_columns;
+
my (%retrieve_cols, $autoinc_supplied, $retrieve_autoinc_col);
+
for my $col ($source->columns) {
+
+ # first autoinc wins - this is why ->columns() in-order iteration is important
+ #
+ # FIXME - there ought to be a sanity-check for multiple is_auto_increment settings
+ # or something...
+ #
if ($col_infos->{$col}{is_auto_increment}) {
+
+ # FIXME - we seem to assume undef values as non-supplied.
+ # This is wrong.
+ # Investigate what does it take to s/defined/exists/
+ # ( fails t/cdbi/copy.t amoong other things )
$autoinc_supplied ||= 1 if defined $to_insert->{$col};
+
$retrieve_autoinc_col ||= $col unless $autoinc_supplied;
}
# nothing to retrieve when explicit values are supplied
next if (
- defined $to_insert->{$col} and ! is_literal_value($to_insert->{$col})
+ # FIXME - we seem to assume undef values as non-supplied.
+ # This is wrong.
+ # Investigate what does it take to s/defined/exists/
+ # ( fails t/cdbi/copy.t amoong other things )
+ defined $to_insert->{$col}
+ and
+ (
+ # not a ref - cheaper to check before a call to is_literal_value()
+ ! length ref $to_insert->{$col}
+ or
+ # not a literal we *MAY* need to pull out ( see check below )
+ ! is_literal_value( $to_insert->{$col} )
+ )
);
# the 'scalar keys' is a trick to preserve the ->columns declaration order
);
};
+ # corner case of a non-supplied PK which is *not* declared as autoinc
+ if (
+ ! $autoinc_supplied
+ and
+ ! defined $retrieve_autoinc_col
+ and
+ # FIXME - first come-first serve, suboptimal...
+ ($retrieve_autoinc_col) = ( grep
+ {
+ $pcols{$_}
+ and
+ ! $col_infos->{$_}{retrieve_on_insert}
+ and
+ ! defined $col_infos->{$_}{is_auto_increment}
+ }
+ sort
+ { $retrieve_cols{$a} <=> $retrieve_cols{$b} }
+ keys %retrieve_cols
+ )
+ ) {
+ carp_unique(
+ "Missing value for primary key column '$retrieve_autoinc_col' on "
+ . "@{[ $source->source_name ]} - perhaps you forgot to set its "
+ . "'is_auto_increment' attribute during add_columns()? Treating "
+ . "'$retrieve_autoinc_col' implicitly as an autoinc, and attempting "
+ . 'value retrieval'
+ );
+ }
+
local $self->{_autoinc_supplied_for_op} = $autoinc_supplied;
local $self->{_perform_autoinc_retrieval} = $retrieve_autoinc_col;
use warnings;
use Test::More;
+use Test::Warn;
use DBICTest;
my $schema = DBICTest->init_schema();
# and inserts?
my $see_spot;
-$see_spot = eval { $owner->books->find_or_create({ title => "See Spot Run" }) };
-if ($@) { print $@ }
-ok(!$@, 'find_or_create on resultset with attribute for non-existent entry did not throw');
+$see_spot = eval {
+ warnings_exist {
+ $owner->books->find_or_create({ title => "See Spot Run" })
+ } qr/Missing value for primary key column 'id' on BooksInLibrary - perhaps you forgot to set its 'is_auto_increment'/;
+};
+is ($@, '', 'find_or_create on resultset with attribute for non-existent entry did not throw');
ok(defined $see_spot, 'successfully did insert on resultset with attribute for non-existent entry');
my $see_spot_rs = $owner->books->search({ title => "See Spot Run" });
__PACKAGE__->column_info_from_storage(1);
__PACKAGE__->set_primary_key('id');
+ # FIXME - for some reason column_info_from_storage does not properly find
+ # the is_auto_increment setting...
+ __PACKAGE__->column_info('id')->{is_auto_increment} = 1;
}
SKIP: {
skip "Need DBD::Pg 2.9.2 or newer for array tests", 4 if $DBD::Pg::VERSION < 2.009002;
use Test::Warn;
use Try::Tiny;
-
use DBICTest;
my ($dsn, $user, $pass) = @ENV{map { "DBICTEST_MSSQL_ODBC_${_}" } qw/DSN USER PASS/};
'updated money value to NULL round-trip';
}
}
+
+# Test leakage of PK on implicit retrieval
+ {
+
+ my $next_owner = $schema->resultset('Owners')->get_column('id')->max + 1;
+ my $next_book = $schema->resultset('BooksInLibrary')->get_column('id')->max + 1;
+
+ cmp_ok(
+ $next_owner,
+ '!=',
+ $next_book,
+ 'Preexisting auto-inc PKs staggered'
+ );
+
+ my $yet_another_owner = $schema->resultset('Owners')->create({ name => 'YAO' });
+ my $yet_another_book;
+ warnings_exist {
+ $yet_another_book = $yet_another_owner->create_related( books => { title => 'YAB' })
+ } qr/Missing value for primary key column 'id' on BooksInLibrary - perhaps you forgot to set its 'is_auto_increment'/;
+
+ is(
+ $yet_another_owner->id,
+ $next_owner,
+ 'Expected Owner id'
+ );
+
+ is(
+ $yet_another_book->id,
+ $next_book,
+ 'Expected Book id'
+ );
+ }
}
}
__PACKAGE__->set_table('Movies');
__PACKAGE__->columns(All => qw(id title));
+ # Disables the implicit autoinc-on-non-supplied-pk behavior
+ # (and the warning that goes with it)
+ # This is the same behavior as it was pre 0.082900
+ __PACKAGE__->column_info('id')->{is_auto_increment} = 0;
+
sub create_sql {
return qq{
id INTEGER PRIMARY KEY AUTOINCREMENT,
__PACKAGE__->columns(TEMP => qw/ nonpersistent /);
__PACKAGE__->add_constructor(salary_between => 'salary >= ? AND salary <= ?');
+# Disables the implicit autoinc-on-non-supplied-pk behavior
+# (and the warning that goes with it)
+# This is the same behavior as it was pre 0.082900
+__PACKAGE__->column_info('id')->{is_auto_increment} = 0;
+
sub mutator_name_for { "set_$_[1]" }
sub create_sql {
__PACKAGE__->has_a( actor => 'Actor' );
__PACKAGE__->has_a( alias => 'Actor' );
+# Disables the implicit autoinc-on-non-supplied-pk behavior
+# (and the warning that goes with it)
+# This is the same behavior as it was pre 0.082900
+__PACKAGE__->column_info('id')->{is_auto_increment} = 0;
+
sub create_sql {
return qq{
id INTEGER PRIMARY KEY,
Class::DBI::Column->new('columnb' => {mutator => 'columnb_as_write'}),
));
+# Disables the implicit autoinc-on-non-supplied-pk behavior
+# (and the warning that goes with it)
+# This is the same behavior as it was pre 0.082900
+__PACKAGE__->column_info('id')->{is_auto_increment} = 0;
+
sub create_sql {
return qq{
id INTEGER PRIMARY KEY,
__PACKAGE__->columns('Directors', qw( Director CoDirector ));
__PACKAGE__->columns('Other', qw( Rating NumExplodingSheep HasVomit ));
+# Disables the implicit autoinc-on-non-supplied-pk behavior
+# (and the warning that goes with it)
+# This is the same behavior as it was pre 0.082900
+__PACKAGE__->column_info('title')->{is_auto_increment} = 0;
+
sub create_sql {
return qq{
title VARCHAR(255),
update_datetime => 'MyDateStamp',
);
+
+# Disables the implicit autoinc-on-non-supplied-pk behavior
+# (and the warning that goes with it)
+# This is the same behavior as it was pre 0.082900
+__PACKAGE__->column_info('id')->{is_auto_increment} = 0;
+
sub create_sql {
# SQLite doesn't support Datetime datatypes.
return qq{
deflate => 'mysql_datetime'
);
+# Disables the implicit autoinc-on-non-supplied-pk behavior
+# (and the warning that goes with it)
+# This is the same behavior as it was pre 0.082900
+__PACKAGE__->column_info('id')->{is_auto_increment} = 0;
+
__PACKAGE__->add_trigger(before_create => \&set_dts);
__PACKAGE__->add_trigger(before_update => \&set_dts);
inflate => sub { Date::Simple->new(shift) },
deflate => 'format',
);
+
+# Disables the implicit autoinc-on-non-supplied-pk behavior
+# (and the warning that goes with it)
+# This is the same behavior as it was pre 0.082900
+__PACKAGE__->column_info('myid')->{is_auto_increment} = 0;
+
#__PACKAGE__->find_column('tdate')->placeholder("IF(1, CURDATE(), ?)");
sub create_sql {
__PACKAGE__->table('books');
__PACKAGE__->add_columns(
'id' => {
+ # part of a test (auto-retrieval of PK regardless of autoinc status)
+ # DO NOT define
+ #is_auto_increment => 1,
+
data_type => 'integer',
- is_auto_increment => 1,
},
'source' => {
data_type => 'varchar',
'id' => {
data_type => 'timestamp',
default_value => \'current_timestamp',
+ retrieve_on_insert => 1,
},
);
__PACKAGE__->table('users');
__PACKAGE__->add_columns(
- qw/user_id email password
- firstname lastname active
- admin/
+ user_id => { retrieve_on_insert => 1 },
+ qw( email password firstname lastname active admin ),
);
__PACKAGE__->set_primary_key('user_id');
projects / dbsrgits/DBIx-Class.git / commitdiff