Skip to content

Commit

Permalink
qe: fix prisma/20799 take 2
Browse files Browse the repository at this point in the history
  • Loading branch information
tomhoule committed Aug 30, 2023
1 parent 0a64c82 commit d1de298
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 1 deletion.
31 changes: 30 additions & 1 deletion quaint/src/ast/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,36 @@ impl<'a> Table<'a> {
/// Add unique index definition.
pub fn add_unique_index(mut self, i: impl Into<IndexDefinition<'a>>) -> Self {
let definition = i.into();
self.index_definitions.push(definition.set_table(self.clone()));

// FIXME: the whole circular dependency (Table -> IndexDefinition -> Column), and cloning
// of tables inside each column.
//
// We can't clone `self` here, as that would lead to cycles. The following happened:
// `add_unique_index()` clones the table, including all previous index definitions, and
// adds the cloned table to the new index definition. On models with multiple unique
// indexes/criterias (including PKs), we repeatedly called `add_unique_index()`. Each time,
// we clone one more index, that itself contains copies of all previous indexes. Each
// column in each of these previous indexes contains a partial copy of the `Table` with
// the indexes on the table at the point of the `add_unique_index()` call that created that
// copy.
//
// If we make the simplifying assumption that all the indexes have a single column, one
// call to `add_unique_index()` would cause `(index_definitions.len() + 1)!` clones and
// allocations of `Table`s with `IndexDefinition` arrays. That quickly leads to exhausting
// available memory. With multiple columns per index, that adds a factor to each step in
// the factorial.
//
// For symptoms of the previous naive clone, see
// https://github.com/prisma/prisma/issues/20799 and the corresponding regression test in
// connector-test-kit.
let table = Table {
typ: self.typ.clone(),
alias: self.alias.clone(),
database: self.database.clone(),
index_definitions: Vec::new(),
};

self.index_definitions.push(definition.set_table(table));
self
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ mod prisma_15607;
mod prisma_16760;
mod prisma_17103;
mod prisma_18517;
mod prisma_20799;
mod prisma_5952;
mod prisma_6173;
mod prisma_7010;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
use query_engine_tests::*;

// before the fix to https://github.com/prisma/prisma/issues/20799, this test would consistently
// run for multiple minutes and crash with an OOM error on a fast desktop machine with 32GB of RAM.
#[test_suite(schema(schema), only(Sqlite))]
mod regression {
fn schema() -> String {
r#"
model Connection {
id Int @id @default(autoincrement())
uid String @unique
ownerId Int
atlassianOnPremiseOAuthCredentialsId Int? @unique
bitbucketCloudOAuthCredentialsId Int? @unique
genericAppCredentialsId Int? @unique
gitlabOAuthCredentialsId Int? @unique
googleSheetsOAuthCredentialsId Int? @unique
githubOAuthCredentialsId Int? @unique
mondayOAuthCredentialsId Int? @unique
serviceNowOAuthCredentialsId Int? @unique
bitbucketOnPremiseOAuthCredentialsId Int? @unique
salesforceOAuthCredentialsId Int? @unique
tempoCloudOAuthCredentialsId Int? @unique
slackCredentialsId Int?
jsmCloudAssetsApiKeyCredentialsId Int? @unique
googleCalendarOAuthCredentialsId Int? @unique
microsoftOAuthCredentialsId Int? @unique
zoomOAuthCredentialsId Int? @unique
statuspageApiKeyCredentialsId Int? @unique
trelloApiKeyCredentialsId Int? @unique
opsgenieApiKeyCredentialsId Int? @unique
one Int? @unique
two Int? @unique
three Int? @unique
four Int? @unique
five Int? @unique
six Int? @unique
seven Int? @unique
eight Int? @unique
nine Int? @unique
ten Int? @unique
}
"#
.to_owned()
}

#[connector_test]
async fn repro(runner: Runner) -> TestResult<()> {
let query = r#"
query {
findManyConnection(
where: {
ownerId: 100,
},
) { id }
}
"#;
runner.query(query).await?.assert_success();
Ok(())
}
}

0 comments on commit d1de298

Please sign in to comment.