Allow Table and Column name expressions to return SqlIdentifier #2077
Conversation
Signed-off-by: Sergey Korotaev <[email protected]>
spring-projects-issues
added
the
status: waiting-for-triage
| */ | ||
| class BasicRelationalPersistentEntity<T> extends BasicPersistentEntity<T, RelationalPersistentProperty> | ||
| implements RelationalPersistentEntity<T> { | ||
|
|
||
| private static final SpelExpressionParser PARSER = new SpelExpressionParser(); | ||
|
|
||
| private final Lazy<SqlIdentifier> tableName; | ||
| private final @Nullable Expression tableNameExpression; | ||
| private final Lazy<SqlIdentifier> tableNameExpression; |
There was a problem hiding this comment.
That's not what was meant here. We should remain with Expression (or switch to ValueExpression).
It was rather meant that a Expression (respective ValueExpression) should be able to return an SqlIdentifier and not be forced to return a String (as it is done now).
We also want to evaluate the expression each time we obtain names and we don't want to introduce caching.
There was a problem hiding this comment.
Sorry but perhaps I don't understand correctly. I see that ExpressionEvaluator returns String from Expression. Do you mean about it that no it is forced to return a String and should be able to return an SqlIdentifier? Like this
String result = expression.getValue(provider.getEvaluationContext(null), String.class);
Roughly, here we need to return SqlIdentifier, right?
There was a problem hiding this comment.
ExpressionEvaluator should return a SqlIdentifier. This is a package-protected class and we're free to change its signature.
I suggest renaming it to SqlIdentifierExpressionEvaluator and not force a String type during expression evaluation (expression.getValue(…)), but introspect the returned object.
If it is already a SqlIdentifier, then just return it as-is, otherwise, transform the value to String and apply SqlIdentifierSanitizer. Then, apply force-quoting as per isForceQuote() of the calling code.
Does that help or did that increase confusion?
There was a problem hiding this comment.
No, thanks, it's clear! In first time I thought about something like this (not so detailed) but I was scary and not sure to change ExpressionEvaluator, sorry....
I'll do it a little bit later
By #1524