Support for Function Expressions in JDQL and Fluent API (#643)#704
Support for Function Expressions in JDQL and Fluent API (#643)#704omatheusmesmo wants to merge 7 commits into
Conversation
This commit introduces the data structures required to represent functions within JNoSQL queries. It also updates the changelog. Issue eclipse-jnosql#643 Signed-off-by: Matheus Oliveira <hi@omatheusmesmo.dev>
Updates the ANTLR4 grammar to support ABS, LENGTH, LOWER, UPPER, LEFT, and RIGHT functions. It also ensures that these function names can still be used as field identifiers to maintain backward compatibility. Issue eclipse-jnosql#643 Signed-off-by: Matheus Oliveira <hi@omatheusmesmo.dev>
Enhances PrimaryFunction to process function expression nodes from the AST, converting them into structured FunctionQueryValue objects. This enables recursive parsing of function arguments. Issue eclipse-jnosql#643 Signed-off-by: Matheus Oliveira <hi@omatheusmesmo.dev>
Updates AbstractWhere to support function expressions within query conditions, allowing them to be processed by the JDQL provider. Issue eclipse-jnosql#643 Signed-off-by: Matheus Oliveira <hi@omatheusmesmo.dev>
Extends the mapping layer to support function expressions in Select, Update, and Delete operations using the Fluent API. Introduces a mechanism for database drivers to declare function support. Issue eclipse-jnosql#643 Signed-off-by: Matheus Oliveira <hi@omatheusmesmo.dev>
Provides comprehensive tests for function support in both string queries and the Fluent API, covering single-parameter, multi-parameter and nested scenarios. Issue eclipse-jnosql#643 Signed-off-by: Matheus Oliveira <hi@omatheusmesmo.dev>
Adds usage examples and descriptions for the newly supported built-in functions (ABS, LENGTH, LOWER, UPPER, LEFT, RIGHT) in the reference guide. Issue eclipse-jnosql#643 Signed-off-by: Matheus Oliveira <hi@omatheusmesmo.dev>
|
Dear @omatheusmesmo , the description of this PR appears AI-generated. How much of the code is generated by AI or vibe-coded? |
Hi @OndroMih , Yes, the description is AI-generated. I can't define how much of the code is AI-generated because I used it in my workflow, but the documentation part is 100% AI, for example. I reviewed a lot before opening this PR and tried to follow the project's standards, besides testing locally. I opened a PR on Jakarta NoSQL about these functions, but Otavio asked me to start the implementation here first. You can see more details at: jakartaee/nosql#223 Please let me know if I need to correct anything. Thanks! |
|
Thanks. It’s fine if you reviewed the code yourself before raising the PR. I recently saw a trend that people raise PRs with code that is completely AI generated, without reviewing it themselves. Then it’s just a burden on those who review the code. Each line of code should be reviewed by a human before the PR is even submitted, otherwise it’s not fair to reviewers. |
That's a relief. Yes, I've already noticed some people doing this—it's quite unfortunate. My apologies for the PR description; I can edit it to improve readability if you want. What do you think about creating an issue to establish PR templates? We already have issue templates, so it would make sense to extend this consistency. @OndroMih |
| * @param name the function name | ||
| * @param params the parameters | ||
| */ | ||
| public record DefaultFunction(String name, Object... params) implements Function { |
There was a problem hiding this comment.
Do we need this record as public?
Maybe default package is just enought
| return ctx.getText(); | ||
| } | ||
|
|
||
| private String getFunctionName(JDQLParser.Function_expressionContext ctx) { |
There was a problem hiding this comment.
The String is ok, but given that this is the implementation, we can define another strategy, such as an enum or even a contract class that extends Function.
It is up to you.
| @ParameterizedTest | ||
| @DisplayName("Should parse function expressions in WHERE clause") | ||
| @MethodSource("functionsProvider") | ||
| void shouldHandleFunctions(String query, String fieldName, String functionName) { |
| /** | ||
| * A query value that represents a function. | ||
| */ | ||
| public record FunctionQueryValue(Function function) implements QueryValue<Function> { |
There was a problem hiding this comment.
The same question about visibility here.
|
|
||
| protected String name; | ||
|
|
||
| protected String nameForCondition; |
There was a problem hiding this comment.
Do you mean the function's name?
| * | ||
| * @param function the function expression to validate | ||
| * @throws UnsupportedFunctionException if the underlying database does not support the function | ||
| * @since 1.1.0 |
There was a problem hiding this comment.
This is version 1.2.0:
https://github.com/eclipse-jnosql/jnosql/blob/main/pom.xml#L26
I don't think that we will have the backport on this one.
| QueryMapper.MapperDeleteQueryBuild { | ||
|
|
||
| @Override | ||
| SemiStructuredMapperDelete where(String name); |
There was a problem hiding this comment.
Are we exposing it somehow, or does it belong only to the implementation?
My point here is:
We will break fluent-api this, because I could do the: where twice.
Such as:
delete(Car.class).where("type").where("color")And that is not a valid query.
| * @since 1.1.0 | ||
| * @see Function | ||
| */ | ||
| public interface SemiStructuredMapperSelect extends |
There was a problem hiding this comment.
The same problem here on the fluent level.
|
thanks you for your time @omatheusmesmo ! That is good progress. In general, I would suggest breaking this one into small steps. For example, create the API and make it support only text queries. Once approved, we can proceed to make it work with the vendors. |
Thank you for your feedback, @otaviojava. I’ll mark this PR as a draft and open a new one for the API first incorporating minor adjustments to address all your concerns. |
| QueryMapper.MapperUpdateQueryBuild { | ||
|
|
||
| @Override | ||
| SemiStructuredMapperUpdate set(String name); |
There was a problem hiding this comment.
The same problem here on the fluent level.
There was a problem hiding this comment.
Let me know if you need help with it, okay!?
There was a problem hiding this comment.
@dearrudam, this is huge, as the first contribution. I recommend @omatheusmesmo to do baby-steps.
Thus, start with the API only, then the implementation at query and so on.
There was a problem hiding this comment.
That's a really good advice! Thanks @otaviojava!
Description
This PR enables scalar functions (
ABS,LENGTH,LOWER,UPPER,LEFT,RIGHT) across JDQL string queries and the Fluent API.Fix #643
Tasks
ABS,LENGTH,LOWER,UPPER,LEFT,RIGHT.MAPPING.adoc.Standards Compliance
Technical Summary
JDQL.g4with dedicated tokens. Implemented a contextualidentifierrule to preserve backward compatibility for field names (e.g., a field namedlengthstill works).SemiStructuredMapper*interfaces using covariant return types. This extends the read-onlyjakarta.nosql-apiinterfaces withwhere(Function)overloads while keeping the fluent chain type-safe.checkFunctionSupportfail-fast hook in the template layer.Validation
SelectJakartaDataQueryFunctionTest(nesting, parameters, case-insensitivity).MapperSelectTest,MapperDeleteTest, andMapperUpdateTest.mvn clean verifypassed on all affected modules.