[Dbix-class] '+select'/'+as' is broken when chaining resultsets

Peter Rabbitson rabbit+list at rabbit.us
Tue Oct 28 10:21:25 GMT 2008


BUCHMULLER Norbert wrote:
> So, the patch for t/88_result_set_column.t contains modifications related
> to 3 different issues:
> 
> 1. a new test for chained search() and '+select'/'+as'
> 2. a new test for a $rs->get_column() for a nonexistent column
> 3. $rs->get_column() calls replaced with $rs->first->get_column() calls

After looking further into this your testing approach is wrong.
t/88_result_set_column.t is _only_ for aclling get_column on a
resutlset, nothing less or more. So rewrite your tests as outlined below:

> Regarding 1.: I think that should be OK.

These tests go into t/76retrieve.t (new file)

> Regarding 2.: we should have such a test IMHO, otherwise we cannot trust
> the other test cases (as we cannot be sure that a column really exists -
> which the state is currently).

This belongs in t/76retrieve.t too
> Regarding 3.: maybe I misunderstood something, in that case pls give me
> some pointer how to solve it better. (First I looked into
> DBIx::Class::ResultSetColumn, but found that in C<new> I should not check
> if the column exists in the resultset as C<new> should not touch the
> database - as far as I see the design. Then I looked into
> DBIx::Class::ResultSet, but found that C<get_column> does not look like
> the ideal place for such a change, too. But here I'm possibly wrong.)

Apparently you should be able to call get_column on a nonexistent
resultset column, with the tests in t/88_result_set_column.t
safeguarding this. All in all your new patch should make no
modifications to t/88_result_set_column.t at all.

Cheers!




More information about the DBIx-Class mailing list