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 function returns a list of primary keys and no other item attributes #51

Open
DomenicoCammarota opened this issue Aug 21, 2015 · 9 comments

Comments

@DomenicoCammarota
Copy link

When calling SCAN on a table, the data returned is an array of items but only the primary key attribute is included. Other item attributes are ignored.

@rowanu
Copy link
Contributor

rowanu commented Sep 27, 2015

Just noticed this myself.

I can see SPECIFIC_ATTRIBUTES is being used instead of the default behaviour. Is there an explicit reason for that @victorquinn?

I'd be interested in sending through a PR to enable the default behaviour and return all fields unless specific fields are specified (using ProjectionExpression which replaces AttributesToGet). How do you think this should be implemented?

@victorquinn
Copy link
Owner

@rowanu I can't seem to remember any explicit reason for that being there though I don't think I was the one to implement scan so that's probably why I don't remember. However, that does seems rather silly and limiting!

I agree with you on all points. To put some more details on the topic, I feel like it should probably:

  • Return all fields by default
  • Have an optional key in params called attributes which specifies which attributes to grab. This would set the ProjectionExpression and set the Dynamo Select attribute to SPECIFIC_ATTRIBUTES
  • Use the ProjectionExpression as you suggest as AttributesToGet is now deprecated
  • Add a count flag which, when set, returns only the count setting the Select parameter to COUNT

Something like the following code snippets, using the existing scan example:

// Example 1: no attributes specified, return all
lands
    .scan()
    .then(function(allLands) {
        // Returns all lands and all attributes of said lands since none were specified
    });

// Example 2: attributes specified, return only those attributes
lands
    .scan({ attributes: ['population', 'capital'] })
    .then(function(allLands) {
        // Returns all lands and only the `population` and `capital` attributes of said lands
    });

// Example 3: optionally specify a count flag to just get a count
lands
    .scan({ count: true })
    .then(function(numLands) {
        // Returns just the number of lands and no attributes of said lands since the count flag was set
    });

Anyway, those are my thoughts, would love a PR with this if you've got the time @rowanu otherwise I'll add it to my ever growing todo list.

@rowanu
Copy link
Contributor

rowanu commented Sep 29, 2015

Thanks for the detail.

I'm hoping to send a PR for it, but it won't be until next week at the earliest.

@calvinlfer
Copy link

You can specify what you want using (as a temporary fix):

    selectedTable.scan({
      // https://github.com/victorquinn/dynasty/blob/master/src/lib/aws-translators.coffee#L110
      attrsGet: ['description', 'id', 'username'], 
      Limit: 10
    });
  }

@rowanu
Copy link
Contributor

rowanu commented Sep 29, 2015

Yeah, I saw that in the code when I started looking. It's a bit confusing, because it's different from the default/underlying DynamoDB behavior.

I like the proposed functionality, and it will have the benefit of not using deprecated features.

@camjackson
Copy link

Is there a timeframe on this? Is it considered a high priority issue?

( By the way, I just discovered this library, and it seems awesome so far!)

@rowanu
Copy link
Contributor

rowanu commented Jan 7, 2016

Sorry, no ETA yet - I haven't been using DDB recently, but am planning to use it on my next project so should have a chance to get back to this.

@victorquinn
Copy link
Owner

Apologies, this is high priority, I just don't have a use case personally and I haven't had the time to build a sample dataset in order to build and test this.

I also got sidetracked figuring out how to better test Dynasty generally since it's currently rather dependent on Dynamo and some sample data existing and the way it's currently built it's kind of a pain. I have a local branch in which I've tried to install and utilize a local version of Dynamo and seed it with a bunch of test data which can be spun up and torn down with the ultimate goal of making testing more viable than it currently is. Ideally I'll get it working and hooked up to CI so tests can be run whenever code is pushed.

Anyway, that tangent has soaked up most of my time but once I finalize it I'll be able to circle back and more easily (and confidently) implement things like this scan.

@rowanu
Copy link
Contributor

rowanu commented Jan 7, 2016

Anything to make testing easier is definitely worth the time taken.

FYI: I didn't have a great experience with the official local version of Dyanmo when I was trying to do something similar, but there is a (nodejs-based) alternative: dynalite.

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

5 participants