tywalch / electrodb

A DynamoDB library to ease the use of modeling complex hierarchical relationships and implementing a Single Table Design while keeping your query code readable.
MIT License
993 stars 62 forks source link

Is this a feature or a bug? #408

Open andrestone opened 2 months ago

andrestone commented 2 months ago

When running lt conditions, electrodb builds the sk but doesn't care about adding a FilterExpression (which seems to me like the correct behavior).

  lt: {
    name: "lt",
    action(entity, state, facets = {}) {
      if (state.getError() !== null) {
        return state;
      }
      try {
        return state.setType(QueryTypes.lt).ifSK((state) => {
          const { pk, sk } = state.getCompositeAttributes();
          const { composites } = state.identifyCompositeAttributes(
            facets,
            sk,
            pk,
          );
          state.setSK(composites);
        });
      } catch (err) {
        state.setError(err);
        return state;
      }
    },
    children: ["go", "params"],
  },

When running lte conditions, it adds a FilterExpression on top of the ConditionExpression:

  lte: {
    name: "lte",
    action(entity, state, facets = {}) {
      if (state.getError() !== null) {
        return state;
      }
      try {
        return state.setType(QueryTypes.lte).ifSK((state) => {
          const { pk, sk } = state.getCompositeAttributes();
          const { composites } = state.identifyCompositeAttributes(
            facets,
            sk,
            pk,
          );
          state.setSK(composites);
          const accessPattern =
            entity.model.translations.indexes.fromIndexToAccessPattern[
              state.query.index
            ];
          if (!entity.model.indexes[accessPattern].sk.isFieldRef) {
            state.filterProperties(FilterOperationNames.lte, composites); // <---- HERE
          }
        });
      } catch (err) {
        state.setError(err);
        return state;
      }
    },
    children: ["go", "params"],
  },

The same happens with greater than conditions (differently though, it's gt that gets the FilterExpression while gte doesn't).

Is this intentional? If so what's the rationale behind this decision? Why not allow users to decide their FilterExpression using the where command?

As it stands, there's no way to build a lt or gte query with a composite SK without a FilterExpression (as far as I'm aware).

tywalch commented 2 months ago

Good question! Feel free to weigh in here: https://github.com/tywalch/electrodb/issues/228