On Apr 8, 2015, at 1:47 PM, Peter Rabbitson <rabbit+d...@rabbit.us> wrote:
>> It works, but {'column_expr' => $val}} or {'column_expr' => {op => $val}} is
>> the more natural and obvious way
>
> ... which however is a lie in terms of the API contract - you are telling
> DBIC "use the column named 'column_expr', nevermind the fact the column
> doesn't exist". Additionally you are relying on the side-effect of lack of
> quote_names, in which case most (but not all!) of the machinery does not
> check a column for existence.
We use this method all over the place, and it works great. Not everybody is
going to read all the documentation, and if the obvious thing people try just
works to solve their problem, you should probably consider mitigating whatever
is bad about it--they're going to do it regardless. We have several DBIx
developers here and we've all read substantial parts of the documentation and
the source code, and I don't think any of us could tell you the potential
pitfalls with this technique. We would be interested in understanding them,
however, so we can help fix them or avoid them.
> So I could not disagree more with your statement "I would recommend that you
> don't refer people to that link". This is the one and only place people need
> to be pointed to when trying to do what the original poster asked.
On this one, I am actually the originator of the discussion. The link wasn't
helpful for me, and I don't see it being helpful to others, which is why I
recommended you don't reference it. If the obvious solution is bad (especially
even though it works!) you should probably add an explanation there.
> "but this is ugly in perl" is not a valid argument to drive people away from
> the only universally correct solution. Please refrain from arguing this
> further in the context of new code.
I don't recall anybody talking about ugliness in Perl. However you should
recognize that your "correct" solution has some deficiencies compared to the
"obvious" solution. The mental burden of learning exceptional corner cases is
real, but more importantly it pushes complexity into user code. It would add a
significant amount of complexity to our code to handle simple columns and
column expressions differently. We could of course never use {col => $val} and
replace all usage with \[] constructs, but that makes query analysis harder
since structural details have been moved inside a string.
It may be that \[] is correct in the general case (which for whatever reason
doesn't apply to us) due to some technical limitation, but I think {col =>
$val} is better from a programmer standpoint. Perhaps the {RHS=>{}, op =>
whatever, RHS => {}} construct below can provide a mechanism that is
technically safe in all scenarios while preserving the syntactic structure of
the query. If the LHS is a hashref, then you could provide an actual AST-type
structure for the column expression. Cleanly dealing with DISTINCT, ORDER BY,
FILTER, OVER etc in aggregate/window functions without being forced to make
string literals would be great.
>> If DBIx could expose an alternate structure like:
>>
>> {
>> LHS => \[],
>> op => whatever,
>> RHS => \[]
>> }
>>
>
> This has been proposed and couldn't be made to work consistently at the time.
> However the state of the art moved on, so it may be worth taking another
> look. Noted.
Cool. Thanks!
>> I understand that you want people to use \[], but I am unclear on why you
>> would not want bind to work correctly in the cases it is still required.
>>
>
> It already works correctly... or am I misunderstanding the issue you are
> having...?
>
> Can you please make a small test case using as a model
> https://github.com/dbsrgits/dbix-class/blob/current/blead/t/count/group_by_func.t#L12-L21
>
>
> Please do that *before* spending more time on attempting a fix as you see
> fit. Because if we do not agree on what needs fixing, your work will end up
> being discarded, which will be a shame.
As just one example, as far as I understand it, it is currently not possible to
achieve LATERAL joins directly. Instead, you must create a view (the DBIx
construct, ie virtual, not an actual DB level view). When you want to search on
this view, you must pass bind variables for the ?s inside the view definition
SQL.
Thanks-
Augustus
_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/dbix-class@lists.scsys.co.uk
