Skip to content
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

Joined tables add all columns to column set #477

Open
jdmcd opened this issue Dec 13, 2021 · 2 comments
Open

Joined tables add all columns to column set #477

jdmcd opened this issue Dec 13, 2021 · 2 comments
Assignees
Labels
bug Something isn't working wontfix This will not be worked on

Comments

@jdmcd
Copy link
Member

jdmcd commented Dec 13, 2021

Describe the bug

When running a Fluent query with the following join:

let courseSections = try await CourseSection.query(on: conn)
    .join(DistrictCourse.self, on: \CourseSection.$course.$id == \DistrictCourse.$course.$id)
    .filter(DistrictCourse.self, \.$district.$id == district.safeId)
    .all()

The generated query is:

SELECT
	`course_sections`.`id` AS `course_sections_id`,
	`course_sections`.`course_id` AS `course_sections_course_id`,
	`course_sections`.`name` AS `course_sections_name`,
	`course_sections`.`timeDescription` AS `course_sections_timeDescription`,
	`course_sections`.`sourcedId` AS `course_sections_sourcedId`,
	`course_sections`.`createdAt` AS `course_sections_createdAt`,
	`course_sections`.`updatedAt` AS `course_sections_updatedAt`,
	`course_sections`.`deletedAt` AS `course_sections_deletedAt`,
	`district_courses`.`id` AS `district_courses_id`,
	`district_courses`.`district_id` AS `district_courses_district_id`,
	`district_courses`.`course_id` AS `district_courses_course_id`,
	`district_courses`.`createdAt` AS `district_courses_createdAt`,
	`district_courses`.`updatedAt` AS `district_courses_updatedAt`,
	`district_courses`.`deletedAt` AS `district_courses_deletedAt`
FROM
	`course_sections`
	INNER JOIN `district_courses` ON `course_sections`.`course_id` = `district_courses`.`course_id`
WHERE
	`district_courses`.`district_id` = ?
	AND(`course_sections`.`deletedAt` IS NULL
		OR `course_sections`.`deletedAt` > ?)
	AND(`district_courses`.`deletedAt` IS NULL
		OR `district_courses`.`deletedAt` > ?)

This results in a potentially large extraneous amount of data being sent over the wire.

To Reproduce

Run a query with a join + filter

Expected behavior

The query should not return the column set for the joined table unless it's asked for via .field or .with.

Environment

  • Vapor Framework version: 4.54.0
  • Vapor Toolbox version: N/A
  • OS version: macOS 12.0.1

Additional context

N/A

@jdmcd jdmcd added the bug Something isn't working label Dec 13, 2021
@gwynne gwynne self-assigned this Dec 13, 2021
@gwynne gwynne added the wontfix This will not be worked on label Dec 13, 2021
@gwynne
Copy link
Member

gwynne commented Dec 13, 2021

Some notes:

  • This can be mitigated with .fields(for: PrimaryModel.self) or individual .field() invocations
  • The default behavior of returning all joined columns can not be changed without breaking compatibility due to the availability of the .all(Model.self, \.$keypath) overload (which would suddenly stop working at runtime without warning).
  • This is something we should definitely consider handling differently for Fluent 5, as it's a significant performance consideration.

Since we can't safely change the behavior for existing code, this is a wontfix for Fluent 4.

@jdmcd
Copy link
Member Author

jdmcd commented Dec 13, 2021

@gwynne makes sense - thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants