[Dbix-class] Announcing 0.08099_07 (0.08100_RC1)

[Ash Berlin]

On 28 Feb 2009, at 04:15, Rob Kinyon wrote:

> On Fri, Feb 27, 2009 at 22:37, Guillermo Roditi <groditi at gmail.com>  
> wrote:
>> ->table(\ 'otherschema.table');
>>
>> no longer works. This is kind of odd, but I have started to track  
>> it down.
>> The first issue is an unless(ref $table)
>>  in &DBIx::Class::ResoultSourceProxy::Table::table (line 78).  
>> Changing it to
>> test for blessed instead of ref started looking like it was going  
>> to work,
>> but unfortunately ->from was spitting a ref, which made the tests  
>> puke
>> during populate as "0x88BEEF" became interpolated into the queries.  
>> A quick
>> dive through Storage::DBI and DBIC::SQLA seems to indicate that the  
>> table is
>> passed as a ref to SQLA, and further digging seems to indicate that  
>> the ref
>> is not being accurately decoded by _table which handles the case  
>> fine in
>> SQLA. So this leaves DBIC::SQLA as the guilty party. I am at a loss  
>> for what
>> the fuck to do now though. I found where the bug is, I can't figure  
>> out how
>> to fix it.
>>
>> It looks like it could be as simple as adding a new case to the if/ 
>> else in
>> _table, or we could just call SUPER instead of returning $from but  
>> I'm not
>> really sure. it gets too hairy form me here
>
> groditi and I discussed this in channel tonight. I think there's
> actually two bugs here. The first is the ->table(\'artist') one
> described above. I think this is easily solved by changing the
> else-clause in _table() from "else { return $from }" to "else { return
> $self->_make_as( $from ); }". But, I think _make_as() is incorrect
> when it handles ->table({ foo => \'other_schema.bar' }). But,
> apparently, we don't have tests for these cases. So, I think the first
> step is to add the following test cases:
>    1) ->table(\'foo')
>    2) ->table( [ \'foo', 'bar' ] )
>    3) ->table( [ 'foo', \'bar' ] )
>    4) ->table( { bar => \'schema.table' } )
>    5) ->table( { bar => 'schema.table' } )
> Assuming, of course, that I got the hashref usecase done right. These
> cases could be something as simple as converting existing tests to use
> the \'' form vs. the '' form.
>
> We also had a bit of a discussion wrt why the \'' form is used in
> ->table. The three cases he described were:
>    1) Views
>    2) Subqueries
>    3) Multiple Schemas
> Cases #1 and #2 are being addressed in 08100, so that's good. However,
> case #3 isn't handled by DBIC at all. So, I'd like to propose the
> following changes for 08200:

#1 and #2 are being addressed, but there's always back compatibility  
to maintain.

What's your definition of schema here? What mysql calls a database,  
and Pg calls a schema?

If so at work we use ->table("schema.foo") just fine with 0.0801x on Pg.


>
>    1) We add a ->schema() method.
>    2) ->schema() would default to the schema connected to in the DSN.
>    3) We always add the schema to the tablename. So, it would always
> be "FROM <schema>.<table> <alias>" instead of "FROM <table> <alias>".
> (Subject, of course, to RDBMS vagaries and quoting.)
>    4) Unrelated to above, but we should also always add the table
> alias to the columns in the various clauses. Since this is generated
> SQL, I don't see the harm.

We already do, dont we: "SELECT me.id ....";

>
> The explicit schema and table names are practices that the DBAs I've
> respected have all done, no exceptions. I think that DBIC should also
> do the same.
>
> A somewhat related question: Is there a reason why quoting isn't on  
> by default?
>
> Rob