Fix table name usage in clauses

[maxwkf]

Fixes #55

[maxwkf]

By using the function setApplyColumnCheck, it then bypass the checking on column and apply original where clause.

$query = Entry::query()
        ->setApplyColumnCheck(false)

[ryanmitchell]

Would a better fix not be to change the !Str::startsWith('data->' to be Str::contains('data-> ?

[maxwkf]

Would a better fix not be to change the !Str::startsWith('data->' to be Str::contains('data-> ?

This line was not introduced by me.

[ryanmitchell]

You are misunderstanding... you could do the same effect of what your changes have done by simpyl checking if Str::contains('data-> instead of the additional functions and logic you have put in place

[maxwkfcc]

contains

Do you mean like this?

    protected function column($column)
    {
        if (! is_string($column)) {
            return $column;
        }

        if ($column == 'origin') {
            $column = 'origin_id';
        }

        if (in_array($column, self::COLUMNS)) {
            if (! Str::contains($column, 'data->')) {
                $column = 'data->'.$column;
            }
        }

        return $column;
    }

[ryanmitchell]

Yep - basically... one small change (maybe a copy/paste issue on your side).

    protected function column($column)
    {
        if (! is_string($column)) {
            return $column;
        }

        if ($column == 'origin') {
            $column = 'origin_id';
        }

        if (! in_array($column, self::COLUMNS)) {
            if (! Str::contains($column, 'data->')) {
                $column = 'data->'.$column;
            }
        }

        return $column;
    }

does that change fix your issue?

[maxwkfcc]

startsWith

It looks you are right. Let me check it up. Thanks

[maxwkf]

Yep - basically... one small change (maybe a copy/paste issue on your side).

    protected function column($column)
    {
        if (! is_string($column)) {
            return $column;
        }

        if ($column == 'origin') {
            $column = 'origin_id';
        }

        if (! in_array($column, self::COLUMNS)) {
            if (! Str::contains($column, 'data->')) {
                $column = 'data->'.$column;
            }
        }

        return $column;
    }

does that change fix your issue?

This would generate the following SQL in which the location part is incorrect.

select * from "entries"
inner join "entries" as "e" on "e"."id" = "entries"."id" and "e"."collection" = ?
left join "entries" as "locations" on "locations"."collection" = ? and "locations"."id" = json_extract("e"."data", '$."location"')
where json_extract("e"."data", '$."title"') like ? and json_extract("data", '$."locations.slug"') = ?

[ryanmitchell]

What about adding a check for the table alias... something like:

    protected function column($column)
    {
        if (! is_string($column)) {
            return $column;
        }
        
        $table = Str::contains($column, '.') ? Str::before($column, '.') : '';
        $column = Str::after($column, '.');

        if ($column == 'origin') {
            $column = 'origin_id';
        }

        if (! in_array($column, self::COLUMNS)) {
            if (! Str::contains($column, 'data->')) {
                $column = 'data->'.$column;
            }
        }

        return ($table ? $table.'.' : '').$column;
    }

[maxwkf]

What about adding a check for the table alias... something like:

    protected function column($column)
    {
        if (! is_string($column)) {
            return $column;
        }
        
        $table = Str::contains($column, '.') ? Str::before($column, '.') : '';
        $column = Str::after($column, '.');

        if ($column == 'origin') {
            $column = 'origin_id';
        }

        if (! in_array($column, self::COLUMNS)) {
            if (! Str::contains($column, 'data->')) {
                $column = 'data->'.$column;
            }
        }

        return ($table ? $table.'.' : '').$column;
    }

Thanks a lot. You logic is right. Do you think this is simplier?

    protected function column($column)
    {
        if (! is_string($column)) {
            return $column;
        }

        if ($column == 'origin') {
            $column = 'origin_id';
        }

        if (! in_array($column, self::COLUMNS)) {
            if (! Str::contains($column, 'data->')) {
                if (! Str::contains($column, '.') ) {
                    $column = 'data->'.$column;
                    
                }
            }
        }

        return $column;
    }

[ryanmitchell]

Simpler yes but it wouldn’t work the same. Stick with what I did :)

…

On 5 Sep 2022, at 14:49, maxwkf ***@***.***> wrote:  What about adding a check for the table alias... something like: protected function column($column) { if (! is_string($column)) { return $column; } $table = Str::contains($column, '.') ? Str::before($column, '.') : ''; $column = Str::after($column, '.'); if ($column == 'origin') { $column = 'origin_id'; } if (! in_array($column, self::COLUMNS)) { if (! Str::contains($column, 'data->')) { $column = 'data->'.$column; } } return ($table ? $table.'.' : '').$column; } Thanks a lot. You logic is right. Do you think this is simplier? protected function column($column) { if (! is_string($column)) { return $column; } if ($column == 'origin') { $column = 'origin_id'; } if (! in_array($column, self::COLUMNS)) { if (! Str::contains($column, 'data->')) { if (! Str::contains($column, '.') ) { $column = 'data->'.$column; } } } return $column; } — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.

[maxwkf]

Simpler yes but it wouldn’t work the same. Stick with what I did :)
…
On 5 Sep 2022, at 14:49, maxwkf @.***> wrote:  What about adding a check for the table alias... something like: protected function column($column) { if (! is_string($column)) { return $column; } $table = Str::contains($column, '.') ? Str::before($column, '.') : ''; $column = Str::after($column, '.'); if ($column == 'origin') { $column = 'origin_id'; } if (! in_array($column, self::COLUMNS)) { if (! Str::contains($column, 'data->')) { $column = 'data->'.$column; } } return ($table ? $table.'.' : '').$column; } Thanks a lot. You logic is right. Do you think this is simplier? protected function column($column) { if (! is_string($column)) { return $column; } if ($column == 'origin') { $column = 'origin_id'; } if (! in_array($column, self::COLUMNS)) { if (! Str::contains($column, 'data->')) { if (! Str::contains($column, '.') ) { $column = 'data->'.$column; } } } return $column; } — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.

Hi Ryan,

Thanks for your help. Updated according to our discussion. Do I need to close the pull request or leave it as open?

[ryanmitchell]

Leave it open :)

[what-the-diff]

• Added a new test to check if entries can be retrieved on join table conditions
• Fixed the column method in EntryQueryBuilder class so that it works with joins as well