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

when using selectRaw, array bindings are flattened #39554

Closed
atymic opened this issue Nov 9, 2021 · 10 comments
Closed

when using selectRaw, array bindings are flattened #39554

atymic opened this issue Nov 9, 2021 · 10 comments
Labels

Comments

@atymic
Copy link

atymic commented Nov 9, 2021

  • Laravel Version:8.70.1
  • PHP Version: 8.0
  • Database Driver & Version: Mysql 8.0

Description:

Take the following query:

DB::table('whatever')
    ->selectRaw("SUM('case(`status` in (?)) as total_pending", [[1,2,3]])
    ->where('user', 1)
    ->groupBy('something')
    ->get()

When laravel runs this query, the bindings are flattened, which results in

SELECT SUM('case(`status` in (1)) as total_pending from whatever where user = 2 group by something

Possibly a security issue if query params are passed into a select raw? see #35865

Anyhow, I don't see why array bindings flattened, as they are supported by the database.

@RahulDey12
Copy link

RahulDey12 commented Nov 10, 2021

You could use like this.

DB::table('whatever')
    ->selectRaw("SUM('case(`status` in (?)) as total_pending", [json_encode([1,2,3])])
    ->where('user', 1)
    ->groupBy('something')
    ->get()

@bastien-phi
Copy link
Contributor

@atymic Did you try with laravel 8.70.2 ? I think this was fixed with 74dcc02

@RahulDey12
Copy link

@bastien-phi no it is not fixed

@nuernbergerA
Copy link
Contributor

nuernbergerA commented Nov 10, 2021

This seems to be related to #39553 and #39492

@taylorotwell I assume it's the same issue on this line

Replace ->toArray() with ->all().

@atymic / @RahulDey12 can you test this and confirm if this is the issue?

@RahulDey12
Copy link

@nuernbergerA Still same

@jakubthedeveloper
Copy link

jakubthedeveloper commented Nov 13, 2021

There is a mistake in the query:

    ->selectRaw("SUM('case(`status` in (?)) as total_pending", [[1,2,3]])

The apostrophe before "case" is not needed.

Of course, removing it will not solve the problem, but may help debugging it.

I think the test for this case is:

    public function testBindInValuesIntoSelectRaw()
    {
        $expectedSql = 'select SUM(case(`status` in (?)) as total_pending from "whatever" where "user" = ? group by "something"';
        $expectedBindings = [
            "1,2,3",
            1
        ];

        $builder = $this->getBuilder();

        $builder->selectRaw("SUM(case(`status` in (?)) as total_pending", [[1,2,3]])
            ->from('whatever')
            ->where('user', 1)
            ->groupBy('something');

        $this->assertEquals($expectedSql, $builder->toSql());
        $this->assertEquals($expectedBindings, $builder->getBindings());
    }

@driesvints
Copy link
Member

Should be fixed with da7aa38. Thanks

@atymic
Copy link
Author

atymic commented Jan 4, 2022

@driesvints apologies for the delay. I don't think this is fixed for sum/case in statements.

The issue seems to be that when I bind an wherein, the query builder is smart enough to put the correct amount of placeholders in, but it can't tell that when doing a selectraw with bound arrays.

Will try to repro and find a workaround.

@jimktrains
Copy link

There is a similar issue when doing something like whereIn(DB::raw('(a,b)', [[1,2]]), as it only generates where (a,b) in (?) instead of where (a,b) in ((?,?)). (Or otherwise keep the single ? and bind it to '(1,2)').

Should I open a separate ticket for that?

@driesvints
Copy link
Member

@jimktrains best if you can send in a PR with a failing test so we can verify the bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants