[Dbix-class] binding variables to CASE WHEN

[Augustus Saunders]

On Apr 8, 2015, at 1:47 PM, Peter Rabbitson <rabbit+dbic at 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