victorquinn/dynasty

SCAN function returns a list of primary keys and no other item attributes

DomenicoCammarota opened this issue · 9 comments

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.

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?

@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.

Thanks for the detail.

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

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
    });
  }

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.

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!)

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.

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.

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.