Correct DynamoDB's Count/ScannedCount when paginating over numeric result keys (#9653)

Author: ashovlin

.changes/next-release/bugfix-dynamodb-60935.json

@@ -0,0 +1,5 @@
+{
+  "type": "bugfix",
+  "category": "``dynamodb``",
+  "description": "Correct Scan and Scanned Count values when resuming a scan or query with a ``--starting-token``"
+}

awscli/botocore/paginate.py

@@ -411,7 +411,12 @@ def _handle_first_request(
             elif isinstance(sample, str):
                 empty_value = ''
             elif isinstance(sample, (int, float)):
-                empty_value = 0
+                # Even though we may be resuming from a truncated page, we
+                # still start from the actual numeric secondary result. For
+                # DynamoDB's Count/ScannedCount, this will still show how many
+                # items the server evaluated, even if the client is truncating
+                # due to a StartingToken.
+                empty_value = sample
             else:
                 empty_value = None
             set_value_from_jmespath(parsed, token.expression, empty_value)

tests/functional/botocore/test_paginator_config.py

@@ -131,6 +131,10 @@
     'xray.GetTraceSummaries.TracesProcessedCount',
     'xray.GetTraceSummaries.ApproximateTime',
 ]
+KNOWN_PAGINATORS_WITH_INTEGER_OUTPUTS = (
+    ('dynamodb', 'Query'),
+    ('dynamodb', 'Scan'),
+)
 
 
 def _pagination_configs():
@@ -150,11 +154,12 @@ def _pagination_configs():
 )
 def test_lint_pagination_configs(operation_name, page_config, service_model):
     _validate_known_pagination_keys(page_config)
-    _valiate_result_key_exists(page_config)
+    _validate_result_key_exists(page_config)
     _validate_referenced_operation_exists(operation_name, service_model)
     _validate_operation_has_output(operation_name, service_model)
     _validate_input_keys_match(operation_name, page_config, service_model)
     _validate_output_keys_match(operation_name, page_config, service_model)
+    _validate_new_numeric_keys(operation_name, page_config, service_model)
 
 
 def _validate_known_pagination_keys(page_config):
@@ -165,7 +170,7 @@ def _validate_known_pagination_keys(page_config):
             )
 
 
-def _valiate_result_key_exists(page_config):
+def _validate_result_key_exists(page_config):
     if 'result_key' not in page_config:
         raise AssertionError(
             "Required key 'result_key' is missing "
@@ -260,6 +265,30 @@ def _validate_output_keys_match(operation_name, page_config, service_model):
         )
 
 
+def _validate_new_numeric_keys(operation_name, page_config, service_model):
+    output_shape = service_model.operation_model(operation_name).output_shape
+    for key in _get_list_value(page_config, 'result_key'):
+        current_shape = output_shape
+        if '.' in key:  # result_key is a JMESPath expression
+            for part in key.split('.'):
+                current_shape = current_shape.members[part]
+        elif key in output_shape.members:
+            current_shape = output_shape.members[key]
+
+        if (
+            getattr(current_shape, 'type_name', None) == 'integer'
+            and (service_model.service_name, operation_name)
+            not in KNOWN_PAGINATORS_WITH_INTEGER_OUTPUTS
+        ):
+            raise AssertionError(
+                f'There is a new operation {operation_name} for service '
+                f'{service_model.service_name} that is configured to sum '
+                'integer outputs across pages. Verify that this behavior is '
+                'correct before allow-listing, since whether or not it is '
+                'appropriate to sum depends on the subject matter.'
+            )
+
+
 def _looks_like_jmespath(expression):
     if all(ch in MEMBER_NAME_CHARS for ch in expression):
         return False

tests/functional/ddb/test_select.py

@@ -27,7 +27,7 @@ def assert_yaml_response_equal(self, response, expected):
 
 class TestSelect(BaseSelectTest):
     def setUp(self):
-        super(TestSelect, self).setUp()
+        super().setUp()
         self.parsed_response = {
             "Count": 1,
             "Items": [{"foo": {"S": "spam"}}],
@@ -509,7 +509,7 @@ def test_select_parsing_error_rc(self):
 
 class TestSelectPagination(BaseSelectTest):
     def setUp(self):
-        super(TestSelectPagination, self).setUp()
+        super().setUp()
         self.parsed_responses = [
             {
                 "Count": 1,
@@ -617,8 +617,8 @@ def test_starting_token(self):
             command, expected_params, expected_rc=0
         )
         expected_response = {
-            'Count': 0,
+            'Count': 1,
             'Items': [{'foo': 2}],
-            'ScannedCount': 0,
+            'ScannedCount': 1,
         }
         self.assert_yaml_response_equal(stdout, expected_response)

tests/unit/botocore/test_paginate.py

@@ -1089,7 +1089,7 @@ def test_resume_with_secondary_result_as_string(self):
         # they were in the original (first) response.
         self.assertEqual(complete, {"Users": ["User2"], "Groups": ""})
 
-    def test_resume_with_secondary_result_as_integer(self):
+    def test_resume_with_secondary_result_does_not_reset_integer(self):
         self.method.return_value = {"Users": ["User1", "User2"], "Groups": 123}
         starting_token = encode_token(
             {"Marker": None, "boto_truncate_amount": 1}
@@ -1098,9 +1098,9 @@ def test_resume_with_secondary_result_as_integer(self):
             PaginationConfig={'MaxItems': 1, 'StartingToken': starting_token}
         )
         complete = pages.build_full_result()
-        # Note that the secondary keys ("Groups") becomes zero because
-        # they were in the original (first) response.
-        self.assertEqual(complete, {"Users": ["User2"], "Groups": 0})
+        # Note that we don't reset numeric secondary keys, for DynamoDB's
+        # Count/ScannedCount since the server still evaluates those items
+        self.assertEqual(complete, {"Users": ["User2"], "Groups": 123})
 
 
 class TestMultipleInputKeys(unittest.TestCase):