Skip to content

Support for sum() with complex CASE expressions returning BasicColumn #828

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hanbonghun opened this issue Aug 6, 2024 · 4 comments · Fixed by #831
Closed

Support for sum() with complex CASE expressions returning BasicColumn #828

hanbonghun opened this issue Aug 6, 2024 · 4 comments · Fixed by #831

Comments

@hanbonghun
Copy link

hanbonghun commented Aug 6, 2024

Hello I'm trying to use a complex CASE expression within a sum() function, but I'm encountering difficulties due to type incompatibility. Here's an example of what I'm trying to achieve:

sum(case_()
    .when(
        statusCode, isEqualTo(ACCEPT_PROGRESS)
    )
    .then(1)
    .else_(0)
    .end()).as("receivedVocCount")

The issue arises because the end() method returns a BasicColumn, but the sum() function expects a BindableColumn. This makes it impossible to use complex CASE expressions directly within aggregate functions like sum().
I have two questions:

Is there currently any supported way to use complex CASE expressions within aggregate functions like sum()?
If not, what would be the recommended approach to extend the functionality to support this use case? Are there specific interfaces or classes that I should implement or extend to make this work?

Any guidance or suggestions would be greatly appreciated. This functionality would be very useful for creating more complex and flexible queries using MyBatis Dynamic SQL.
Thank you for your time and consideration.

@hanbonghun
Copy link
Author

I've implemented a potential solution to wrap a BasicColumn as a BindableColumn. Here's my implementation:

public class BindableColumnWrapper<T> implements BindableColumn<T> {
    private final BasicColumn column;

    public BindableColumnWrapper(BasicColumn column) {
        this.column = column;
    }

    @Override
    public Optional<String> alias() {
        return column.alias();
    }

    @Override
    public BindableColumn<T> as(String alias) {
        return new BindableColumnWrapper<>(column.as(alias));
    }

    @Override
    public FragmentAndParameters render(RenderingContext renderingContext) {
        return column.render(renderingContext);
    }

    @Override
    public String renderWithTableAlias(TableAliasCalculator tableAliasCalculator) {
        return column.renderWithTableAlias(tableAliasCalculator);
    }
}

This wrapper allows us to use a BasicColumn (such as the result of a CASE expression) in aggregate functions that expect a BindableColumn:

BasicColumn caseExpression = case_()
    .when(statusCode.isEqualTo(ACCEPT_PROGRESS))
    .then(1)
    .else_(0)
    .end();

BindableColumn<Integer> wrappedColumn = new BindableColumnWrapper<>(caseExpression);

sum(wrappedColumn).as("receivedVocCount")

I'd like to know if this approach is valid and aligns with the design principles of MyBatis Dynamic SQL. Are there any potential issues or improvements you can suggest? Also, are there any other methods from the BindableColumn interface that I should implement to make this wrapper more complete?
Thank you for your time and guidance.

@jeffgbutler
Copy link
Member

@hanbonghun this is a great effort to solve a tricky problem! But I can make it easier for you and not require the wrapper class.

I've got a prototype working in my fork now and I'll push it in the next day or two. Basically, we need a way to build the sum function without requiring a typed column. This required some refactoring, but I think it is a good improvement.

@hanbonghun
Copy link
Author

@jeffgbutler Thank you for your quick response, I really appreciate it.
I was wondering, once the work is done and incorporated, would I be able to get the updated version by simply applying the latest library version in a future release? Or is there anything else I should keep in mind to ensure I'm using the version with these changes?

@jeffgbutler
Copy link
Member

@hanbonghun the new support will be available soon in the Sonatype snapshots repository. You will need to add that repository to your maven configuration, then specify version 2.0.0-SNAPSHOT. Once released, it will be version 2.0.0 in Maven Central.

See this page for details about the Sonatype snapshot repository: https://github.com/mybatis/mybatis-3/wiki/Maven

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants