Skip to content

Allows non ancestor queries inside a transaction #366

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
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,6 @@ public PreparedQueryImpl(Query query, Transaction txn, QueryRunner queryRunner)
this.txn = txn;
this.queryRunner = queryRunner;

// TODO Move this check and the one that follows into the
// LocalDatastoreService (it may already be there).
checkArgument(
txn == null || query.getAncestor() != null,
"Only ancestor queries are allowed inside transactions.");
TransactionImpl.ensureTxnActive(txn);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,6 @@ private void validateQuery() {
}
}

// Transaction requires ancestor
if (query.hasTransaction() && !query.hasAncestor()) {
throw new IllegalQueryException(
"Only ancestor queries are allowed inside transactions.",
IllegalQueryType.TRANSACTION_REQUIRES_ANCESTOR);
}

// Filters and sort orders require kind.
if (!query.hasKind()) {
for (Filter filter : query.filters()) {
Expand Down Expand Up @@ -309,7 +302,6 @@ enum IllegalQueryType {
FILTER_WITH_MULTIPLE_PROPS,
MULTIPLE_INEQ_FILTERS,
FIRST_SORT_NEQ_INEQ_PROP,
TRANSACTION_REQUIRES_ANCESTOR,
ILLEGAL_VALUE,
ILLEGAL_PROJECTION,
ILLEGAL_GROUPBY,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1207,29 +1207,23 @@ public QueryResult runQuery(Status status, Query query) {
String app = query.getApp();
Profile profile = getOrCreateProfile(app);

// The real datastore supports executing ancestor queries in transactions.
// For now we're just going to make sure the entity group of the ancestor
// is the same entity group with which the transaction is associated and
// skip providing a transactionally consistent result set.

synchronized (profile) {
// Having a transaction implies we have an ancestor, but having an
// ancestor does not imply we have a transaction.
if (query.hasTransaction() || query.hasAncestor()) {
// Query can only have a txn if it is an ancestor query. Either way we
// know we've got an ancestor.

if (query.hasTransaction()) {
if (!app.equals(query.getTransaction().getApp())) {
throw newError(
ErrorCode.INTERNAL_ERROR,
"Can't query app "
+ app
+ "in a transaction on app "
+ query.getTransaction().getApp());
}
}

if (query.hasAncestor()) {
Path groupPath = getGroup(query.getAncestor());
Profile.EntityGroup eg = profile.getGroup(groupPath);
if (query.hasTransaction()) {
if (!app.equals(query.getTransaction().getApp())) {
throw newError(
ErrorCode.INTERNAL_ERROR,
"Can't query app "
+ app
+ "in a transaction on app "
+ query.getTransaction().getApp());
}

LiveTxn liveTxn = profile.getTxn(query.getTransaction().getHandle());
// this will throw an exception if we attempt to read from
// the wrong entity group
Expand All @@ -1238,12 +1232,10 @@ public QueryResult runQuery(Status status, Query query) {
profile = eg.getSnapshot(liveTxn);
}

if (query.hasAncestor()) {
if (query.hasTransaction() || !query.hasFailoverMs()) {
// Either we have a transaction or the user has requested strongly
// consistent results. Either way, we need to apply jobs.
eg.rollForwardUnappliedJobs();
}
if (query.hasTransaction() || !query.hasFailoverMs()) {
// Either we have a transaction or the user has requested strongly
// consistent results. Either way, we need to apply jobs.
eg.rollForwardUnappliedJobs();
}
}

Expand Down
Loading