We can't get away with this - the alternative is a cryptic 'Use of uninitialized value'
exception - good luck debugging that
- Invoking get_inflated_columns() no longer fires get_columns() but
instead retrieves data from individual non-inflatable columns via
get_column()
+ - Limited checks are performed on whether columns without declared
+ is_nullable => 1 metadata do in fact sometimes fetch NULLs from
+ the database (the check is currently very limited and is performed
+ only on resultset collapse when the alternative is rather worse)
* Fixes
- Fix _dbi_attrs_for_bind() being called befor DBI has been loaded
}
}
- my @extra_collapser_args;
- if ($attrs->{collapse} and ! $did_fetch_all ) {
-
- @extra_collapser_args = (
- # FIXME SUBOPTIMAL - we can do better, cursor->next/all (well diff. methods) should return a ref
- sub { my @r = $cursor->next or return; \@r }, # how the collapser gets more rows
- ($self->{_stashed_rows} = []), # where does it stuff excess
- );
- }
-
# hotspot - skip the setter
my $res_class = $self->_result_class;
);
}
}
- # Special-case multi-object HRI (we always prune, and there is no $inflator_cref pass)
- elsif ($self->{_result_inflator}{is_hri}) {
+ else {
+ my $parser_type =
+ $self->{_result_inflator}{is_hri} ? 'hri'
+ : $self->{_result_inflator}{is_core_row} ? 'classic_pruning'
+ : 'classic_nonpruning'
+ ;
# $args and $attrs to _mk_row_parser are seperated to delineate what is
# core collapser stuff and what is dbic $rs specific
- ( $self->{_row_parser}{hri} ||= $rsrc->_mk_row_parser({
+ @{$self->{_row_parser}{$parser_type}}{qw(cref nullcheck)} = $rsrc->_mk_row_parser({
eval => 1,
inflate_map => $infmap,
collapse => $attrs->{collapse},
premultiplied => $attrs->{_main_source_premultiplied},
- hri_style => 1,
- prune_null_branches => 1,
- }, $attrs) )->($rows, @extra_collapser_args);
+ hri_style => $self->{_result_inflator}{is_hri},
+ prune_null_branches => $self->{_result_inflator}{is_hri} || $self->{_result_inflator}{is_core_row},
+ }, $attrs) unless $self->{_row_parser}{$parser_type}{cref};
+
+ # column_info metadata historically hasn't been too reliable.
+ # We need to start fixing this somehow (the collapse resolver
+ # can't work without it). Add an explicit check for the *main*
+ # result, hopefully this will gradually weed out such errors
+ #
+ # FIXME - this is a temporary kludge that reduces perfromance
+ # It is however necessary for the time being
+ my ($unrolled_non_null_cols_to_check, $err);
+
+ if (my $check_non_null_cols = $self->{_row_parser}{$parser_type}{nullcheck} ) {
+
+ $err =
+ 'Collapse aborted due to invalid ResultSource metadata - the following '
+ . 'selections are declared non-nullable but NULLs were retrieved: '
+ ;
+
+ my @violating_idx;
+ COL: for my $i (@$check_non_null_cols) {
+ ! defined $_->[$i] and push @violating_idx, $i and next COL for @$rows;
+ }
+
+ $self->throw_exception( $err . join (', ', map { "'$infmap->[$_]'" } @violating_idx ) )
+ if @violating_idx;
+
+ $unrolled_non_null_cols_to_check = join (',', @$check_non_null_cols);
+ }
+
+ my $next_cref =
+ ($did_fetch_all or ! $attrs->{collapse}) ? undef
+ : defined $unrolled_non_null_cols_to_check ? eval sprintf <<'EOS', $unrolled_non_null_cols_to_check
+sub {
+ # FIXME SUBOPTIMAL - we can do better, cursor->next/all (well diff. methods) should return a ref
+ my @r = $cursor->next or return;
+ if (my @violating_idx = grep { ! defined $r[$_] } (%s) ) {
+ $self->throw_exception( $err . join (', ', map { "'$infmap->[$_]'" } @violating_idx ) )
}
- # Regular multi-object
- else {
- my $parser_type = $self->{_result_inflator}{is_core_row} ? 'classic_pruning' : 'classic_nonpruning';
+ \@r
+}
+EOS
+ : sub {
+ # FIXME SUBOPTIMAL - we can do better, cursor->next/all (well diff. methods) should return a ref
+ my @r = $cursor->next or return;
+ \@r
+ }
+ ;
- # $args and $attrs to _mk_row_parser are seperated to delineate what is
- # core collapser stuff and what is dbic $rs specific
- ( $self->{_row_parser}{$parser_type} ||= $rsrc->_mk_row_parser({
- eval => 1,
- inflate_map => $infmap,
- collapse => $attrs->{collapse},
- premultiplied => $attrs->{_main_source_premultiplied},
- prune_null_branches => $self->{_result_inflator}{is_core_row},
- }, $attrs) )->($rows, @extra_collapser_args);
+ $self->{_row_parser}{$parser_type}{cref}->(
+ $rows,
+ $next_cref ? ( $next_cref, $self->{_stashed_rows} = [] ) : (),
+ );
- $_ = $inflator_cref->($res_class, $rsrc, @$_) for @$rows;
+ # Special-case multi-object HRI - there is no $inflator_cref pass
+ unless ($self->{_result_inflator}{is_hri}) {
+ $_ = $inflator_cref->($res_class, $rsrc, @$_) for @$rows
+ }
}
# The @$rows check seems odd at first - why wouldn't we want to warn
},
);
- my $src = $args->{collapse}
- ? assemble_collapsing_parser({
+ my $check_null_columns;
+
+ my $src = (! $args->{collapse} ) ? assemble_simple_parser(\%common) : do {
+ my $collapse_map = $self->_resolve_collapse ({
+ # FIXME
+ # only consider real columns (not functions) during collapse resolution
+ # this check shouldn't really be here, as fucktards are not supposed to
+ # alias random crap to existing column names anyway, but still - just in
+ # case
+ # FIXME !!!! - this does not yet deal with unbalanced selectors correctly
+ # (it is now trivial as the attrs specify where things go out of sync
+ # needs MOAR tests)
+ as => { map
+ { ref $attrs->{select}[$common{val_index}{$_}] ? () : ( $_ => $common{val_index}{$_} ) }
+ keys %{$common{val_index}}
+ },
+ premultiplied => $args->{premultiplied},
+ });
+
+ $check_null_columns = $collapse_map->{-identifying_columns}
+ if @{$collapse_map->{-identifying_columns}};
+
+ assemble_collapsing_parser({
%common,
- collapse_map => $self->_resolve_collapse ({
- # FIXME
- # only consider real columns (not functions) during collapse resolution
- # this check shouldn't really be here, as fucktards are not supposed to
- # alias random crap to existing column names anyway, but still - just in
- # case
- # FIXME !!!! - this does not yet deal with unbalanced selectors correctly
- # (it is now trivial as the attrs specify where things go out of sync
- # needs MOAR tests)
- as => { map
- { ref $attrs->{select}[$common{val_index}{$_}] ? () : ( $_ => $common{val_index}{$_} ) }
- keys %{$common{val_index}}
- },
- premultiplied => $args->{premultiplied},
- })
- })
- : assemble_simple_parser(
- \%common
- )
- ;
-
- return $args->{eval}
- ? ( eval "sub $src" || die $@ )
- : $src
- ;
+ collapse_map => $collapse_map,
+ });
+ };
+
+ return (
+ $args->{eval} ? ( eval "sub $src" || die $@ ) : $src,
+ $check_null_columns,
+ );
}
/];
is_same_src (
- $schema->source ('CD')->_mk_row_parser({
+ ($schema->source ('CD')->_mk_row_parser({
inflate_map => $infmap,
- }),
+ }))[0],
'$_ = [
{ year => $_->[1] },
{ single_track => ( ! defined( $_->[0]) )
/];
is_same_src (
- $schema->source ('CD')->_mk_row_parser({
+ ($schema->source ('CD')->_mk_row_parser({
inflate_map => $infmap,
- }),
+ }))[0],
'$_ = [
{ artist => $_->[5], title => $_->[4], year => $_->[2] },
{
);
is_same_src (
- $schema->source ('CD')->_mk_row_parser({
+ ($schema->source ('CD')->_mk_row_parser({
prune_null_branches => 1,
inflate_map => $infmap,
- }),
+ }))[0],
'$_ = [
{ artist => $_->[5], title => $_->[4], year => $_->[2] },
{
);
is_same_src (
- $schema->source ('CD')->_mk_row_parser({
+ ($schema->source ('CD')->_mk_row_parser({
hri_style => 1,
prune_null_branches => 1,
inflate_map => $infmap,
- }),
+ }))[0],
'$_ = {
artist => $_->[5], title => $_->[4], year => $_->[2],
);
is_same_src (
- $schema->source ('CD')->_mk_row_parser({
+ ($schema->source ('CD')->_mk_row_parser({
inflate_map => $infmap,
collapse => 1,
- }),
+ }))[0],
' my $rows_pos = 0;
my ($result_pos, @collapse_idx, $cur_row_data, %cur_row_ids);
);
is_same_src (
- $schema->source ('CD')->_mk_row_parser({
+ ($schema->source ('CD')->_mk_row_parser({
inflate_map => $infmap,
collapse => 1,
hri_style => 1,
prune_null_branches => 1,
- }),
+ }))[0],
' my $rows_pos = 0;
my ($result_pos, @collapse_idx, $cur_row_data);
);
is_same_src (
- $schema->source ('CD')->_mk_row_parser({
+ ($schema->source ('CD')->_mk_row_parser({
inflate_map => $infmap,
collapse => 1,
- }),
+ }))[0],
' my $rows_pos = 0;
my ($result_pos, @collapse_idx, $cur_row_data, %cur_row_ids);
);
is_same_src (
- $schema->source ('CD')->_mk_row_parser({
+ ($schema->source ('CD')->_mk_row_parser({
inflate_map => $infmap,
collapse => 1,
prune_null_branches => 1,
- }),
+ }))[0],
' my $rows_pos = 0;
my ($result_pos, @collapse_idx, $cur_row_data);
);
is_same_src (
- $schema->source ('CD')->_mk_row_parser({
+ ($schema->source ('CD')->_mk_row_parser({
inflate_map => $infmap,
collapse => 1,
- }),
+ }))[0],
' my $rows_pos = 0;
my ($result_pos, @collapse_idx, $cur_row_data, %cur_row_ids);
);
is_same_src (
- $schema->source ('CD')->_mk_row_parser({
+ ($schema->source ('CD')->_mk_row_parser({
inflate_map => $infmap,
collapse => 1,
hri_style => 1,
prune_null_branches => 1,
- }),
+ }))[0],
' my $rows_pos = 0;
my ($result_pos, @collapse_idx, $cur_row_data, %cur_row_ids);