fix: EXPOSED-565 Subquery alias with id fails to use correct alias with eq #2258
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
obabichevjb
reviewed
Sep 27, 2024
| @@ -339,7 +339,7 @@ interface ISqlExpressionBuilder { | |||
| val table = (columnType as EntityIDColumnType<*>).idColumn.table as IdTable<T> | |||
| val entityID = EntityID(t, table) | |||
| return if ((this as? Column<*>)?.isEntityIdentifier() == true) { | |||
| table.mapIdComparison(entityID, ::EqOp) | |||
| (this as Column<*>).table.mapIdComparison(entityID, ::EqOp) | |||
There was a problem hiding this comment.
Minor: cast to Column<*> should not be required, because it's already check in the condition. The same for NeqOp
There was a problem hiding this comment.
Changed. Used this directly to make sure local property table isn't used.
obabichevjb
approved these changes
Sep 27, 2024
…th eq A fix for using mapIdComparison() with aliased table had been implemented, but it did not fully cover the case when the right-hand side of the comparison involved an unwrapped id column value. The latter was still using the delegate table associated with the column type, instead of the invoking alias expression's alias table identifier.
…th eq - Add test that uses CompositeID on right-hand side
…th eq - Remove unnecessary (this as Column<*>) cast
bog-walk
force-pushed
the
bog-walk/fix-alias-id-eq
branch
from
213ac8f
to
d70d45d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Summary of the change:
The aliased
idcolumn from a subquery now generates the correct aliased table identifier when used in an equality comparison with an unwrapped right-hand side value.Detailed description:
Why:
mapIdComparison()andmapIdOperator(). The added tests only covered cases when the right-hand side of the operators was a wrappedEntityIDvalue. Unwrapped values use operators that first need to wrap them as anEntityIDinstance, for which reason the invoking expression's table is found as anIdTable. The latter was then incorrectly being used to callmapIdComparison(), meaning that the column identifier used in SQL generation was the delegate non-aliased identifier.How:
mapIdComparison()called directly on the invoking column's table, not the delegateIdTableused to create a wrappingEntityIDinstance.Type of Change
Please mark the relevant options with an "X":
Affected databases:
Checklist
Related Issues
EXPOSED-565, EXPOSED-568, EXPOSED-472