fix: EXPOSED-472 Alias IdTable fails with isNull and eq ops #2189
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
approved these changes
Aug 5, 2024
e5l
approved these changes
Aug 5, 2024
| toCompare: Any?, | ||
| operator: (Column<*>, Expression<*>) -> Op<Boolean>, | ||
| ): Op<Boolean> { | ||
| return when (delegate) { |
There was a problem hiding this comment.
I would use = instead of return here and eliminate inner returns to simplify reading
| @Suppress("UNCHECKED_CAST") | ||
| override fun mapIdComparison( | ||
| toCompare: Any?, | ||
| operator: (Column<*>, Expression<*>) -> Op<Boolean>, |
There was a problem hiding this comment.
operator is a keyword (soft), please consider using other name
bog-walk
force-pushed
the
bog-walk/fix-alias-isnull
branch
from
02e020d
to
4ba7724
Compare
The introduction of `CompositeIdTable`, and functions like `mapIdComparison` an> `mapIdOperator`, led to a regression. When any `IdTable` is aliased and used wi> the affected comparison operators, `eq` and `isNull` (and negative variants), the following is thrown: org.jetbrains.exposed.sql.Alias cannot be cast to org.jetbrains.exposed.dao.id.> The aliased `idColumns` of the `Alias` delegate need to be used in these functi> so these internal functions have been brought a level down, into the `Table` cl> so they can be overridden in `Alias` as well. `Alias.get()` operator has also been fixed to allow `CompositeIdTable` to be al> and still access the delegate `id` column properly.
- Change Alias override internal return to function = operator - Rename both internal function parameters to booleanOperator instead of operator
- Rebase from main - Remove unecessary cast
bog-walk
force-pushed
the
bog-walk/fix-alias-isnull
branch
from
4ba7724
to
6e0553c
Compare
4 tasks
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: Using any aliased
IdTablewithisNull()oreq()no longer throws a class cast exception.Detailed description:
Why:
The introduction of
CompositeIdTable(PR feat: EXPOSED-77 Support entity class for table with composite primary key #1987 ), and internal functionsmapIdComparisonandmapIdOperator, led to a regression. When anyIdTableis aliased and used with the associated comparison operators,eqandisNull(and negative variants), the following is thrown:org.jetbrains.exposed.sql.Alias cannot be cast to org.jetbrains.exposed.dao.id.IdTableWhat:
The
Aliasclass now has its own override for these internal functions, so that the aliasedidColumnsof the delegate are used.How:
mapIdComparisonandmapIdOperatorhave been moved a level down toTable, so that they can be overridden byAliasas well, which is a subclass too.Alias.get()operator has also been fixed to allowCompositeIdTableto be aliased and still access the delegateidcolumn properly.Type of Change
Please mark the relevant options with an "X":
Affected databases:
Checklist
Related Issues
EXPOSED-472