fluent-kit icon indicating copy to clipboard operation
fluent-kit copied to clipboard

Joined tables add all columns to column set

Open jdmcd opened this issue 2 years ago • 2 comments

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 avatar Dec 13 '21 03:12 jdmcd

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.

gwynne avatar Dec 13 '21 07:12 gwynne

@gwynne makes sense - thank you!

jdmcd avatar Dec 13 '21 14:12 jdmcd