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

Is this a feature or a bug? #408

Closed
andrestone opened this issue Jul 20, 2024 · 2 comments
Closed

Is this a feature or a bug? #408

andrestone opened this issue Jul 20, 2024 · 2 comments

Comments

@andrestone
Copy link

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
Copy link
Owner

tywalch commented Jul 20, 2024

Good question! Feel free to weigh in here: #228

@tywalch
Copy link
Owner

tywalch commented Oct 20, 2024

Hi @andrestone 👋

I just published version 3.0.0 which removes the character shifting and extra filters by default. Let me know if you have any questions!

@tywalch tywalch closed this as completed Oct 20, 2024
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

2 participants