fix: do not cast enum to text within min/max #4453
Conversation
Weakky
requested review from
miguelff and
jkomyno
and removed request for
a team
CodSpeed Performance ReportMerging #4453 will not alter performanceComparing Summary
|
| let select = columns | ||
| .map(|c| c.set_is_selected(true)) | ||
| .fold(select, |acc, col| acc.column(col)); | ||
| let select = columns.fold(select, |acc, col| acc.column(col)); |
There was a problem hiding this comment.
get_records is no longer responsible for setting the hacky is_selected field, the caller is
There was a problem hiding this comment.
This makes it possible in aggregate and group_by to not mark columns of the sub-select as selected and thus not cast them.
Here's an example:
SELECT
MAX(col) -- mark as selected
FROM (
SELECT col FROM "Table" -- not mark as selected
) as "sub"
| select.value(min(Column::from(next_field.db_name().to_owned()) | ||
| .set_is_enum(next_field.type_identifier().is_enum()) | ||
| .set_is_selected(true))) | ||
| }), |
There was a problem hiding this comment.
These outer columns (see comment above) are now set as enum (if they are) and selected
There was a problem hiding this comment.
Thanks for these clarifying comments
| fn visit_min(&mut self, min: Minimum<'a>) -> visitor::Result { | ||
| // If the inner column is a selected enum, then we cast the result of MIN(enum)::text instead of casting the inner enum column, which changes the behavior of MIN. | ||
| let should_cast = min.column.is_enum && min.column.is_selected; | ||
|
|
||
| self.write("MIN")?; | ||
| self.surround_with("(", ")", |ref mut s| s.visit_column(min.column.set_is_selected(false)))?; | ||
|
|
||
| if should_cast { | ||
| self.write("::text")?; | ||
| } | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
We use, again, is_enum and is_selected to only cast enum columns to ::text when they're seleted. We need a custom implementation now to perform the cast on the function itself rather than the result.
|
Just wondering, wouldn't the initial issue also affect the operator visitors |
Hey @Oreilles, no, because we currently don't allow selecting columns with these functions. Only |
Overview
fixes prisma/prisma#21789
Changes text casting on enums from:
to
So that the behavior of min/max on enums remains unaltered. sum/avg/count remain unchanged because we don't allow these aggregations on enums.