DDC-1071: CASE expressions do not work as documented in EBNF #1664

Closed
doctrinebot opened this Issue Mar 15, 2011 · 9 comments

2 participants

@doctrinebot

Jira issue originally created by user dalvarez:

The EBNF definition of DQL on the website (http://www.doctrine-project.org/docs/orm/2.0/en/reference/dql-doctrine-query-language.html) defines the SELECT expression as follows:

SelectExpression ::= IdentificationVariable | PartialObjectExpression | (AggregateExpression | "(" Subselect ")" | FunctionDeclaration | ScalarExpression) [["AS"] AliasResultVariable]

A ScalarExpression, in turn, is defined as follows:

ScalarExpression ::= SimpleArithmeticExpression | StringPrimary | DateTimePrimary | StateFieldPathExpression BooleanPrimary | CaseExpression | EntityTypeExpression

There is already a missing ''|" between "StateFieldPathExpression" and "BooleanPrimary" here.

But ignoring that detail, this definition states that case expressions can show up anywhere in the list of values to be selected in a query, which, of course, would come in handy.

Contrary to this definition, queries like this one will not work:

SELECT someEntity.a, CASE someEntity.b WHEN 1 THEN 'A' ELSE 'B' END FROM \someEntity;

Whereas queries like this one will:

SELECT CASE someEntity.b WHEN 1 THEN 'A' ELSE 'B' END FROM \someEntity;

Both queries should be legitimate, according to the EBNF definition.

In addition, CASE expressions are not allowed in subselects either, although the EBNF definition of the SimpleSelectExpression states that they should be.

@doctrinebot

Comment created by @beberlei:

Case is not fully implemented yet. Its on the roaodmap for 2.1

@doctrinebot

Comment created by dalvarez:

I reopen this as a doc issue, as long as the feature is not yet implemented.

At the moment, the Doctrine 2 online documentation is patently misleading with respect to the case statement., It documents the case expression as being supported, when in fact it is not even implemented yet. How is a user supposed to know, by reading the docs, that the feature is not yet implemented? I mean, if something in the EBNF definitions, the normal thing is to expect that it is actually supported. People will choose Doctrine 2, because they believe it does what it is documented to do, and instead end up with something that does not. I know Doctrine 2 comes with no warranty. But this is almost deliberate misdocumentation. This was the case with path expressions, and now with the case statement. It is becoming a bad habit.

Anyway, I am looking forward to full support of case expressions. Until then, I will have to patch my code.

But, PLEASE, PLEASE, fix the docs, to save prospective users some serious pain. It is just unfair to document features that are not there.

@doctrinebot

Comment created by @beberlei:

The problem here is that it was planned for 2.0 and its really hard to keep the EBNF and the actual working stuff in sync. I know its not perfect, i will add a note to the docs soonish, but CASE and path expression were the only things that changed and/or didnt get finished for 2.0

@doctrinebot

Comment created by dalvarez:

Thanks for taking care.

An option would be to use a parser generator and generate the parser from an EBNF syntax spec, which saves you the process of deriving the EBNF manually from implementation code. As you already opted for maintaining a formal syntax definition, you could as well reap some consequential benefits. Then again, it would probably be a completely different approach. I also do not know a parser gerator capable of generating PHP 5 code.

Another viable option would be making it a habit to update the EBNF definition first, then using it as a starting point for the actual implementation. Then, with each release, simply publish both code and documentation as you normally would. This way the EBNF definition does not run any risk of ever becoming outdated. Unless you find other ways of screwing it, of course ;-)

@doctrinebot

Comment created by @beberlei:

There is no parser generator for PHP that generates a top-down recursive descent walker as the one we are using. There is a parser generator for bottom-up, but that is targeting php4 and produces ugly code.

We have a convention to always update EBNF and code in conjunction, but given the large EBNF this can just be forgotten sometimes.

@doctrinebot

Comment created by dalvarez:

Fixed typo ("EBDF" instead of "EBNF")

@doctrinebot

Comment created by @guilhermeblanco:

In this commit: 816ce41
And documented in this commit: doctrine/orm-documentation@189c729

This support was FINALLY included. =)

@doctrinebot

Issue was closed with resolution "Fixed"

@doctrinebot doctrinebot added this to the 2.2 milestone Dec 6, 2015
@doctrinebot doctrinebot closed this Dec 6, 2015
@doctrinebot doctrinebot added the Bug label Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment