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

Scan used instead of query #149

Closed
sahilsharma011 opened this issue Aug 31, 2018 · 7 comments
Closed

Scan used instead of query #149

sahilsharma011 opened this issue Aug 31, 2018 · 7 comments

Comments

@sahilsharma011
Copy link
Contributor

Describe the bug
Eloquent queries are using 'scan' whenever I use where function.

Schema

use BaoPham\DynamoDb\DynamoDbModel;

class DistributorInvoices extends DynamoDbModel
{
    protected $table = 'mytable';
    protected $dynamoDbIndexKeys = [
        'GSIndex' => [
            'hash' => 'hashkey',
            'range' => 'rangekey',
        ]
    ];

My Query

DynamoModel::where('hashkey',$hash)
                ->where('rangekey','between',[$range1,$range2])
                ->withIndex('GSIndex')
                ->toDynamoDbQuery();

Debug info
And this is what is returned when I dump the DynamoDbQuery.

RawDynamoDbQuery {#575
  +op: "Scan"
  +query: array:5 [
    "FilterExpression" => "#hash = :a1 AND (#range BETWEEN :a2 AND :a3)"
    "ExpressionAttributeNames" => array:2 [
      "#hash" => "hashkey"
      "#range" => "rangekey"
    ]
    "ExpressionAttributeValues" => array:3 [
      ":a1" => array:1 [
        "N" => "2"
      ]
      ":a2" => array:1 [
        "N" => "1531938600"
      ]
      ":a3" => array:1 [
        "N" => "1532024999"
      ]
    ]
    "TableName" => "mytable"
    "IndexName" => "GSIndex"
  ]
}
@baopham
Copy link
Owner

baopham commented Sep 1, 2018

Good catch. Thanks for reporting. This should be a quick fix

baopham added a commit that referenced this issue Sep 1, 2018
@baopham
Copy link
Owner

baopham commented Sep 1, 2018

hm... what version are you on? I'm not getting the same result: a244362 - the test passed for me.

@sahilsharma011
Copy link
Contributor Author

sahilsharma011 commented Sep 1, 2018

I dig into the code a little bit and I found out that array_first laravel helper function was being used in Analyzer.php
This function was modified in laravel 5.3 where order of its parameters was reversed and that is why it is not working with my code because I have Laravel 5.2
Arrays - Key / Value Order Change

Currently I have fixed this in my project by overriding laravel's helper method

@baopham
Copy link
Owner

baopham commented Sep 2, 2018

Ugh, this is the third time with laravel's helpers. I'll update that so it can be compatible with Laravel 5.2. Thanks for digging in the code!

baopham added a commit that referenced this issue Sep 4, 2018
baopham added a commit that referenced this issue Sep 4, 2018
baopham added a commit that referenced this issue Sep 4, 2018
baopham added a commit that referenced this issue Sep 4, 2018
baopham added a commit that referenced this issue Sep 4, 2018
@baopham baopham closed this as completed in a01d471 Sep 4, 2018
@baopham
Copy link
Owner

baopham commented Sep 4, 2018

This should be fixed now. Could you take a look? Thanks!

@sahilsharma011
Copy link
Contributor Author

Yes it works fine now. Thank you.
Which laravel versions do you plan to support and will you be abandoning support from Laravel 5.2 any time soon?

@baopham
Copy link
Owner

baopham commented Sep 5, 2018

I'm not planning to abandon support for Laravel 5.2 anytime soon. It's just that I'm actively developing in the latest(?) Laravel version so I didn't catch this bug. I'm going to have a task to migrate all the Laravel helpers I've used to the new class. PRs are welcome!

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

No branches or pull requests

2 participants