Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2.6.1 StringPrimary no longer accepts aggregate functions as argument(AVG, SUM, COUNT, MIN and MAX) #7286

Closed
NothingWeAre opened this issue Jul 2, 2018 · 4 comments
Assignees
Milestone

Comments

@NothingWeAre
Copy link

BC Break Report

Q A
BC Break yes
Version 2.6.1

Summary

After upgrading from 2.5.14 to 2.6.1 StringPrimary no longer accepts aggregate functions as argument(AVG, SUM, COUNT, MIN and MAX).
Which affects any functions depending on StringPrimary, for example CONCAT

Previous behavior

CONCAT(teable.field1, MAX(table.field2)) => '{text from field1}{maximum value of field2}'

Current behavior

CONCAT(teable.field1, MAX(table.field2)) => ERROR: Expected StateFieldPathExpression | string | InputParameter | FunctionsReturningStrings | AggregateExpression, got 'MAX'

How to reproduce

pass any aggregate function to CONCAT (for example) as argument

PS

Old StringPrimary function looked like:

            case Lexer::T_CASE:
            case Lexer::T_COALESCE:
            case Lexer::T_NULLIF:
                return $this->CaseExpression();

            default:
                if ($this->isAggregateFunction($lookaheadType)) {
                    return $this->AggregateExpression();
                }
        }
        $this->syntaxError(
            'StateFieldPathExpression | string | InputParameter | FunctionsReturningStrings | AggregateExpression'
        );

while new one missing part responsible for accepting aggregate expressions:

            case Lexer::T_CASE:
            case Lexer::T_COALESCE:
            case Lexer::T_NULLIF:
                return $this->CaseExpression();
        }
        $this->syntaxError(
            'StateFieldPathExpression | string | InputParameter | FunctionsReturningStrings | AggregateExpression'
        );

Creating this as separate issue from #7205, because this concerns any function relient on StringPrimary

@NothingWeAre NothingWeAre changed the title StringPrimary no longer accepts aggregate functions as argument(AVG, SUM, COUNT, MIN and MAX) 2.6.1 StringPrimary no longer accepts aggregate functions as argument(AVG, SUM, COUNT, MIN and MAX) Jul 2, 2018
@stof
Copy link
Member

stof commented Jul 2, 2018

866418e#diff-3aff39fbb5b16ba9be6df33f2fcde380 is the culprit

@NothingWeAre
Copy link
Author

NothingWeAre commented Jul 2, 2018

I can make pull request, but should we use old approach

            default:
                if ($this->isAggregateFunction($lookaheadType)) {
                    return $this->AggregateExpression();
                }

or specify each concerned type inside StringPrimary

            case Lexer::T_COUNT:
            case Lexer::T_AVG:
            case Lexer::T_MIN:
            case Lexer::T_MAX:
            case Lexer::T_SUM:
                return $this->AggregateExpression();

@Majkl578 Majkl578 added this to the 2.6.2 milestone Jul 4, 2018
Majkl578 added a commit to Majkl578/doctrine-orm that referenced this issue Jul 4, 2018
Majkl578 added a commit to Majkl578/doctrine-orm that referenced this issue Jul 4, 2018
@Majkl578
Copy link
Contributor

Majkl578 commented Jul 4, 2018

#7296

Majkl578 added a commit to Majkl578/doctrine-orm that referenced this issue Jul 5, 2018
Majkl578 added a commit to Majkl578/doctrine-orm that referenced this issue Jul 9, 2018
Majkl578 added a commit to Majkl578/doctrine-orm that referenced this issue Jul 9, 2018
lcobucci added a commit that referenced this issue Jul 9, 2018
Fix #7286: StringPrimary no longer accepts aggregate functions as argument
@Majkl578
Copy link
Contributor

Fixed via #7296 (for 2.6.2).

@Majkl578 Majkl578 self-assigned this Jul 10, 2018
Majkl578 added a commit to Majkl578/doctrine-orm that referenced this issue Sep 23, 2018
Majkl578 added a commit to Majkl578/doctrine-orm that referenced this issue Sep 23, 2018
maglnet pushed a commit to maglnet/doctrine2 that referenced this issue Oct 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants