Skip to content

Commit 3bf1bbc

Browse files
fix(datastore): Switch to BFS in Join Builder for Alias, fixes - #2488 (#2693)
Co-authored-by: Tyler Roach <[email protected]>
1 parent 14acc3b commit 3bf1bbc

File tree

12 files changed

+2076
-64
lines changed

12 files changed

+2076
-64
lines changed

aws-datastore/src/androidTest/java/com/amplifyframework/datastore/storage/sqlite/SQLiteStorageAdapterQueryTest.java

Lines changed: 358 additions & 0 deletions
Large diffs are not rendered by default.

aws-datastore/src/main/java/com/amplifyframework/datastore/storage/sqlite/SQLiteCommandFactory.java

Lines changed: 133 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,11 @@
4848
import java.util.HashMap;
4949
import java.util.HashSet;
5050
import java.util.Iterator;
51+
import java.util.LinkedList;
5152
import java.util.List;
5253
import java.util.Map;
5354
import java.util.Objects;
55+
import java.util.Queue;
5456
import java.util.Set;
5557

5658
/**
@@ -178,7 +180,7 @@ public SqlCommand queryFor(@NonNull ModelSchema modelSchema,
178180
tableCount.put(tableName, 1);
179181

180182
// Joins the foreign keys
181-
recursivelyBuildJoins(table, columns, joinStatement, tableCount, tableName);
183+
buildJoinsUsingBFS(table, columns, joinStatement, tableCount, tableName);
182184

183185
// Convert columns to comma-separated column names
184186
boolean firstTable = true;
@@ -499,67 +501,6 @@ private List<Object> extractFieldValues(@NonNull Model model) throws DataStoreEx
499501
return bindings;
500502
}
501503

502-
/**
503-
* Recursively build joins for multilevel nested joins.
504-
*
505-
*/
506-
private void recursivelyBuildJoins(SQLiteTable table, Map<String, List<SQLiteColumn>> columns,
507-
StringBuilder joinStatement, Map<String, Integer> tableCount,
508-
String tableAlias) {
509-
// Joins the foreign keys
510-
// LEFT JOIN if foreign key is optional, INNER JOIN otherwise.
511-
final Iterator<SQLiteColumn> foreignKeyIterator = table.getForeignKeys().iterator();
512-
while (foreignKeyIterator.hasNext()) {
513-
final SQLiteColumn foreignKey = foreignKeyIterator.next();
514-
final String ownedTableName = foreignKey.getOwnedType();
515-
final ModelSchema ownedSchema = schemaRegistry.getModelSchemaForModelClass(ownedTableName);
516-
final SQLiteTable ownedTable = SQLiteTable.fromSchema(ownedSchema);
517-
518-
int newOwnedTableCount = 1;
519-
String ownedTableAlias = ownedTableName;
520-
if (tableCount.containsKey(ownedTableName)) {
521-
Integer currentOwnedTableCount = tableCount.get(ownedTableName);
522-
newOwnedTableCount += currentOwnedTableCount == null ? 0 : currentOwnedTableCount;
523-
ownedTableAlias += newOwnedTableCount;
524-
}
525-
tableCount.put(ownedTableName, newOwnedTableCount);
526-
columns.put(ownedTableAlias, ownedTable.getSortedColumns());
527-
528-
SqlKeyword joinType = foreignKey.isNonNull()
529-
? SqlKeyword.INNER_JOIN
530-
: SqlKeyword.LEFT_JOIN;
531-
532-
joinStatement.append(joinType)
533-
.append(SqlKeyword.DELIMITER)
534-
.append(Wrap.inBackticks(ownedTableName))
535-
.append(SqlKeyword.DELIMITER);
536-
537-
if (!ownedTableName.equals(ownedTableAlias)) {
538-
joinStatement.append(SqlKeyword.AS)
539-
.append(SqlKeyword.DELIMITER)
540-
.append(Wrap.inBackticks(ownedTableAlias))
541-
.append(SqlKeyword.DELIMITER);
542-
}
543-
544-
// Reference the foreign key and primary key using the corresponding table's alias.
545-
String foreignKeyName = foreignKey.getQuotedColumnName().replaceFirst(table.getName(), tableAlias);
546-
String ownedTablePrimaryKeyName = ownedTable.getPrimaryKeyColumnName().replaceFirst(ownedTableName,
547-
ownedTableAlias);
548-
joinStatement.append(SqlKeyword.ON)
549-
.append(SqlKeyword.DELIMITER)
550-
.append(foreignKeyName)
551-
.append(SqlKeyword.EQUAL)
552-
.append(ownedTablePrimaryKeyName);
553-
554-
if (foreignKeyIterator.hasNext()) {
555-
joinStatement.append(SqlKeyword.DELIMITER);
556-
}
557-
558-
// important that this comes last to maintain the order of the joins
559-
recursivelyBuildJoins(ownedTable, columns, joinStatement, tableCount, ownedTableAlias);
560-
}
561-
}
562-
563504
// Utility method to parse columns in CREATE TABLE
564505
private StringBuilder parseColumns(SQLiteTable table) {
565506
final StringBuilder builder = new StringBuilder();
@@ -656,4 +597,134 @@ private boolean shouldCreateIndex(ModelIndex modelIndex, Map<String, ModelAssoci
656597
}
657598
return true;
658599
}
600+
601+
/**
602+
* Builds a SQL JOIN statement using Breadth-First Search (BFS) to explore table relationships.
603+
* This method iteratively explores tables and their foreign keys to construct a comprehensive
604+
* join statement. It handles tables with multiple foreign key references to the same table and
605+
* avoids infinite loops in cyclic relationships by maintaining a set of visited combination keys.
606+
*
607+
* <p>Implementation Notes:</p>
608+
* <ul>
609+
* <li><strong>Multiple Foreign Key References:</strong> This method can process multiple
610+
* references from a single table to another by incorporating the foreign key name into the
611+
* unique combination key. This allows the method to differentiate between different join paths
612+
* and process each unique path accordingly.</li>
613+
*
614+
* <li><strong>Preventing Infinite Loops:</strong> To avoid infinite loops in the presence of
615+
* cyclic relationships, the method tracks processed joins using a combination of the current
616+
* table alias, the target table name, and the foreign key name.
617+
* This ensures that each join operation is processed only once, even in complex schemas
618+
* with potential cycles.</li>
619+
* </ul>
620+
*
621+
* @param rootTable The starting table for join construction.
622+
* @param columns A map to maintain a list of columns for each table encountered during BFS
623+
* traversal. Updated as new tables are processed.
624+
* @param joinStatement The StringBuilder to append JOIN clauses to. This will be modified to
625+
* include the generated JOIN statements.
626+
* @param tableCount A map to track the occurrence count of each table, used for generating
627+
* unique table aliases.
628+
* @param rootTableAlias The alias for the root table, used in the initial JOIN statement.
629+
* @implNote In highly nested or complex cyclic schemas, additional safeguards such as
630+
* traversal depth limits may be advisable to ensure optimal performance and prevent excessively
631+
* deep traversal.
632+
*/
633+
private void buildJoinsUsingBFS(final SQLiteTable rootTable, final Map<String, List<SQLiteColumn>> columns,
634+
StringBuilder joinStatement, final Map<String, Integer> tableCount,
635+
final String rootTableAlias) {
636+
Queue<TableInfo> queue = new LinkedList<>();
637+
queue.add(new TableInfo(rootTable, rootTableAlias));
638+
639+
// Use a Set to track visited (table, foreign key) combinations to allow multiple references.
640+
Set<String> visitedCombinations = new HashSet<>();
641+
642+
while (!queue.isEmpty()) {
643+
final TableInfo currentInfo = queue.poll();
644+
final SQLiteTable table = currentInfo.getSQLiteTable();
645+
final String tableAlias = currentInfo.getAlias();
646+
647+
final Iterator<SQLiteColumn> foreignKeyIterator = table.getForeignKeys().iterator();
648+
while (foreignKeyIterator.hasNext()) {
649+
final SQLiteColumn foreignKey = foreignKeyIterator.next();
650+
final String ownedTableName = foreignKey.getOwnedType();
651+
final String combinationKey = tableAlias + "->" + ownedTableName + ":" + foreignKey.getName();
652+
653+
// Skip if this table-foreignKey combination has been processed.
654+
if (!visitedCombinations.add(combinationKey)) {
655+
continue;
656+
}
657+
658+
final ModelSchema ownedSchema = schemaRegistry.getModelSchemaForModelClass(ownedTableName);
659+
final SQLiteTable ownedTable = SQLiteTable.fromSchema(ownedSchema);
660+
661+
int newOwnedTableCount = 1;
662+
String ownedTableAlias = ownedTableName;
663+
if (tableCount.containsKey(ownedTableName)) {
664+
Integer currentOwnedTableCount = tableCount.get(ownedTableName);
665+
newOwnedTableCount += currentOwnedTableCount == null ? 0 : currentOwnedTableCount;
666+
ownedTableAlias += newOwnedTableCount;
667+
}
668+
tableCount.put(ownedTableName, newOwnedTableCount);
669+
columns.put(ownedTableAlias, ownedTable.getSortedColumns());
670+
671+
SqlKeyword joinType = foreignKey.isNonNull()
672+
? SqlKeyword.INNER_JOIN
673+
: SqlKeyword.LEFT_JOIN;
674+
675+
joinStatement.append(joinType)
676+
.append(SqlKeyword.DELIMITER)
677+
.append(Wrap.inBackticks(ownedTableName))
678+
.append(SqlKeyword.DELIMITER);
679+
680+
if (!ownedTableName.equals(ownedTableAlias)) {
681+
joinStatement.append(SqlKeyword.AS)
682+
.append(SqlKeyword.DELIMITER)
683+
.append(Wrap.inBackticks(ownedTableAlias))
684+
.append(SqlKeyword.DELIMITER);
685+
}
686+
687+
String foreignKeyName = foreignKey.getQuotedColumnName().replaceFirst(table.getName(), tableAlias);
688+
String ownedTablePrimaryKeyName = ownedTable.getPrimaryKeyColumnName().replaceFirst(
689+
ownedTableName, ownedTableAlias);
690+
joinStatement.append(SqlKeyword.ON)
691+
.append(SqlKeyword.DELIMITER)
692+
.append(foreignKeyName)
693+
.append(SqlKeyword.EQUAL)
694+
.append(ownedTablePrimaryKeyName);
695+
696+
if (!queue.isEmpty() || foreignKeyIterator.hasNext()) {
697+
joinStatement.append(SqlKeyword.DELIMITER);
698+
}
699+
queue.add(new TableInfo(ownedTable, ownedTableAlias));
700+
}
701+
}
702+
}
703+
704+
/**
705+
* Represents information about a table in the context of building SQL JOIN statements.
706+
* This class holds a reference to a SQLiteTable instance and its associated alias.
707+
* It is primarily used in the context of BFS traversal for SQL join generation,
708+
* where it is necessary to keep track of each table and its alias while processing the graph of tables.
709+
*
710+
* @implNote This class is a simple container used for organizing table data and its corresponding alias
711+
* during the BFS traversal in the join building process. It includes basic getter methods for both fields.
712+
*/
713+
private static class TableInfo {
714+
private final SQLiteTable table;
715+
private final String alias;
716+
717+
TableInfo(SQLiteTable table, String alias) {
718+
this.table = Objects.requireNonNull(table, "Table cannot be null");
719+
this.alias = Objects.requireNonNull(alias, "Alias cannot be null");
720+
}
721+
722+
public SQLiteTable getSQLiteTable() {
723+
return table;
724+
}
725+
726+
public String getAlias() {
727+
return alias;
728+
}
729+
}
659730
}

testmodels/src/main/java/com/amplifyframework/testmodels/commentsblog/AmplifyModelProvider.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@ public static AmplifyModelProvider getInstance() {
4949
@Override
5050
public Set<Class<? extends Model>> models() {
5151
final Set<Class<? extends Model>> modifiableSet = new HashSet<>(
52-
Arrays.<Class<? extends Model>>asList(Blog.class, Post.class, Comment.class, Author.class, BlogOwner.class, OtherBlog.class, BlogOwnerWithCustomPK.class)
52+
Arrays.<Class<? extends Model>>asList(Blog.class, Post.class, Comment.class, Author.class,
53+
BlogOwner.class, OtherBlog.class, BlogOwnerWithCustomPK.class, BlogOwner3.class,
54+
Blog3.class, Post2.class)
5355
);
5456

5557
return Immutable.of(modifiableSet);

0 commit comments

Comments
 (0)