Skip to content

Allows non ancestor queries inside a transaction in remote api context #383

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

Closed
wants to merge 1 commit into from

Conversation

step76
Copy link
Contributor

@step76 step76 commented Jun 23, 2025

In the pull request #366 I've left behind a change inside the remote api servlets and so, in remote api context, the non ancestor queries insieme a transaction fails.

In this pull request I've removed the check inside the two remote api servlets.

@ludoch
Copy link
Collaborator

ludoch commented Jun 23, 2025

Interesting, this code was added in 2012 without much context other than
"Consistent transactional queries in the java remote_api when talking
to a high-replication datastore.

Also fix ID allocation in a transaction to not write entities."

I'll try to chat with the author. Maybe a better change would be to introduce a system property that would control this behavior and you would turn it on for your app.

if (!query.hasAncestor()) {
throw new ApiProxy.ApplicationException(
BAD_REQUEST.getNumber(), "No ancestor in transactional query.");
}
// Make __entity_group__ key
OnestoreEntity.Reference.Builder egKey =
result.getEntityGroupKeyBuilder().mergeFrom(query.getAncestor());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, we use getAncestor(), so we must have the optional field, so we must have query.hasAncestor() being true.
Can you explain how it works for you?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, I'm sorry, but I thought that the fix was trivial, but this is not the case.

Moreover, that code enforces the optimistic with entity group lock of the old datastore.

I'll try to understand if and how to fix it; in the meantime, you can trash this PR.

@ludoch
Copy link
Collaborator

ludoch commented Jun 24, 2025

no pb.

@ludoch ludoch closed this Jun 24, 2025
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

Successfully merging this pull request may close these issues.

2 participants