Skip to content

Make GSI index binding explicit instead of inferred #11

Description

@msull

Problem

Two things about a GSI are currently inferred rather than declared, and both inferences are avoidable.

1. Which config entry belongs to which index. The outer key of a gsi_config entry is a label, not an identifier — _apply_gsi_configuration only ever iterates .values(), so the label is free text. In practice it is used three different ways: as the deployed index name, as a descriptive access-pattern name ("by-owner"), and occasionally as a name that identifies one index while the entry declares another's attributes. Nothing connects an entry to the index it feeds.

2. Which declared attribute is the partition key and which is the sort key. The library infers this from the pk/sk suffix in the attribute name.

Neither inference matters on the write path — it just writes every declared attribute. Both matter in build_lek_data, which has to reconstruct an index's key to synthesize a LastEvaluatedKey when client-side filtering truncates a page. #7 was the result: an index named gsi-1 (keeping the index name distinct from its gsi1pk/gsi1sk attributes) either raised or silently produced a key that restarted pagination.

#7 is fixed, but the fix is inference on top of inference: normalize the index name, try the naming convention, fall back to a label match, special-case the legacy gsi1/gsi2/gsi3 names so their original meaning is preserved. It is correct and it is covered by tests, but it reads as a pile of heuristics, and it still cannot resolve one legitimate setup: an index whose attributes do not follow the gsi<N>pk convention and whose entry carries a descriptive label. Nothing identifies the index in that case, so the library guesses.

Proposal

Make both facts explicit and delete the inference. Sketch, not a settled API:

@classmethod
def get_gsi_config(cls) -> dict:
    return {
        "gsi-1": GsiIndex(                      # keyed by the DEPLOYED index name
            pk=("gsi1pk", lambda self: f"things#{self.owner_id}"),
            sk=("gsi1sk", lambda self: self.resource_id),
            description="lookup by owner",      # where a descriptive label goes
        ),
    }

With that, build_lek_data becomes total and trivial: look the index up by name, read the two declared attribute names, copy them off the item. No normalization, no naming convention, no pk/sk suffix sniffing, no legacy special case. It also makes a sparse index's None contract expressible in one place rather than depending on whether a lambda returns None or (None, None).

Open questions:

  • Keyed by index name (above), or keep a free label and add an explicit index_name= field? Keying by index name means the dict key finally means something, at the cost of losing descriptive labels as the primary reading.
  • Does the built-in gsitype index become an ordinary declared entry, or stay special?
  • Is db_get_gsi1pk / db_get_gsi2pk / db_get_gsi3pk_and_sk removed in the same release, or kept as a shim? These carry the same implied index binding.

Migration

This is breaking, so it wants a major release. An audit of the largest downstream consumers gives some confidence about scope:

  • Every indexed attribute observed follows the gsi<N>pk/gsi<N>sk convention — no consumer uses custom attribute names. Migration is mechanical.
  • Both index-name conventions are in real use, undashed (gsi1) and dashed (gsi-1), so both must survive the transition.
  • Some consumers define no gsi_config at all and rely entirely on the legacy db_get_gsiNpk overrides.
  • Some use tuple-declared pairs (("gsi3pk", "gsi3sk")); at least one uses a fourth and fifth GSI beyond the three the library originally assumed.
  • At least one consumer has a latent label/index mismatch that is dormant only because that GSI is never queried — exactly the class of bug explicit naming would surface at import time.

Suggested sequence:

  1. Add the explicit form; accept both. Emit a DeprecationWarning when an entry cannot be resolved to an index without inference.
  2. Ship a validator — memory.validate_gsi_config(ResourceClass) — that compares the declared indexes and attributes against DescribeTable output. This is the piece that turns a runtime pagination bug into a CI failure, and it is worth having regardless of whether the rest of this proposal lands.
  3. Remove the inference and the legacy overrides in the next major.

Not urgent

#7 is fixed and the behavior is pinned by a compatibility matrix plus real-world pattern shapes. This is a cleanup to consider deliberately, not a bug.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions