Skip to content

fix: m2m filtering on joined queries#4549

Merged
Weakky merged 1 commit intomainfrom
fix/m2m-filtering
Dec 11, 2023
Merged

fix: m2m filtering on joined queries#4549
Weakky merged 1 commit intomainfrom
fix/m2m-filtering

Conversation

@Weakky
Copy link
Copy Markdown
Contributor

@Weakky Weakky commented Dec 8, 2023

Overview

fixes prisma/prisma#22311
fixes prisma/prisma#22299

  • No parent alias was passed to m2m filters, resulting in an unknown table. eg: WHERE Table.id = 1 instead of WHERE t4.id = 1
  • Relational filters weren't selected in the parent select of the select where filters are rendered, resulting in columns not existing on the filtered table.

@Weakky Weakky requested a review from a team as a code owner December 8, 2023 17:21
@Weakky Weakky requested review from Druue and jkomyno and removed request for a team December 8, 2023 17:21
.left_join(m2m_join_data) // join m2m table
.and_where(join_conditions) // adds join condition to the child table
.with_ordering(&rs.args, Some(join_alias_name(&rs.field)), ctx) // adds ordering stmts
.with_filters(rs.args.filter.clone(), None, ctx) // adds query filters // TODO: avoid clone filter
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Parent alias passed to the filters

.value(Column::from((m2m_join_alias.to_table_string(), JSON_AGG_IDENT)))
.left_join(m2m_join_data) // join m2m table
.and_where(join_conditions) // adds join condition to the child table
.with_ordering(&rs.args, Some(join_alias_name(&rs.field)), ctx) // adds ordering stmts
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated ordering parent alias (same as filtering)

fn build_m2m_join<'a>(&mut self, rs: &RelationSelection, parent_alias: Alias, ctx: &Context<'_>) -> JoinData<'a> {
let rf = rs.field.clone();
let m2m_alias = m2m_join_alias_name(&rf);
let m2m_table_alias = self.next_alias();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We know use Alias instead of the generated string. This is to enable passing the alias to the FilterVisitor which expects an Alias and not a random string.

@Weakky Weakky self-assigned this Dec 8, 2023
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Dec 8, 2023

CodSpeed Performance Report

Merging #4549 will not alter performance

Comparing fix/m2m-filtering (e25b2e9) with main (e7a7287)

Summary

✅ 11 untouched benchmarks

@Weakky Weakky merged commit 47a3fbd into main Dec 11, 2023
@Weakky Weakky deleted the fix/m2m-filtering branch December 11, 2023 09:30
Weakky added a commit that referenced this pull request Dec 18, 2023
@aqrln aqrln added this to the 5.8.0 milestone Jan 6, 2024
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.

relationJoins: fails when filtering includes by isNot: null relationJoins: "The table (not available) does not exist in the current database."

3 participants