Skip to content

RecordQueryFirstOrDefaultPlan does not handle out-of-band stopping reasons from inner correctly #3220

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

Open
alecgrieser opened this issue Mar 3, 2025 · 0 comments
Labels
bug Something isn't working

Comments

@alecgrieser
Copy link
Collaborator

Issue #3219 points out that we were incorrectly feeding back the continuation in RecordQueryFirstOrDefaultPlans to the inner continuation when resuming it. That meant that we were not resuming the continuation from the right place. However, in reality, there are three states that the cursor needs to consider:

  1. The inner cursor returned a result. In this case, the first value is returned
  2. The inner cursor was empty. In this case, the default value is returned
  3. The inner cursor hit an out of band limit. In this case, we are unable to determine if the underlying cursor is empty or not

In both of the first two cases, because we've returned a result, we no longer need to record any state about the inner cursor, and we should just return a continuation that amounts to "we are done after this". In the third case, we should provide the inner cursor its previous continuation and continue on. That implies a continuation like:

message ForOrDefaultContinuation {
   optional bytes innerContinuation = 1;
   optional bool returnedValue = 2;
}

Technically, we could get by with something like just the inner continuation, and if the field is empty, then that means that we have already exhausted the continuation, but one would have to be careful around not returning an empty array.

Note: this was originally the scope of #3219. However, that issue has been repurposed to fixing the fact that the continuation wasn't being properly handled at all by the plan, which meant it was always resuming from the wrong place. This is to fix the other issue identified in a follow up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant