Skip to content
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

implements conditionExpression instead of expression #31

Closed
wants to merge 1 commit into from

Conversation

morloderex
Copy link
Contributor

@morloderex morloderex commented Mar 18, 2025

In order to use the CaseGroup expression as "top-level" expression we must implement \Illuminate\Contracts\Database\Query\ConditionExpression instead of expression.

Before:

where(new CaseGroup)

Generates an is null constraint at the end.

after this pr

where(new CaseGroup)

Does NOT generate an is null constraint at the end.

@tpetry
Copy link
Owner

tpetry commented Mar 18, 2025

This is not how CASE should be used: WHERE case ... end doesn't make any sense. This is like WHERE 1, wheres your condition?

@tpetry tpetry closed this Mar 18, 2025
@tpetry
Copy link
Owner

tpetry commented Mar 18, 2025

Please explain why you wanted to use it that way. Maybe there's a better approach. But CaseGroup will definitely not changed to be a ConditionExpression - as it is not a condition. It only generates a values.

@morloderex
Copy link
Contributor Author

morloderex commented Mar 18, 2025

@tpetry

Here is my CaseGroup:

$query->where(
        new CaseGroup([
            new CaseRule(
                new Number(0),
                new Equal('role', new Value('tool')),
            ),
            new CaseRule(
                new Expression('JSON_LENGTH(`tool_calls`) = 0 OR `tool_calls` IS NULL'),
                new Equal('role', new Value('assistant')),
            ),
            new CaseRule(
                new Number(1),
                new Equal('role', new Value('user')),
            ),
        ])
    );

This is generating a 1 or a 0 depending the content of a column.

@morloderex
Copy link
Contributor Author

It's about making it possible to do complex conditions.

@tpetry
Copy link
Owner

tpetry commented Mar 18, 2025

The code wouldn't even work on PG because an expression in the WHERE has to be a boolean and not a number. So the PR would (for your usecase) broken the promise this package does: make code portable.

@kohenkatz That won't work either on all databases.

@morloderex What you're trying to achieve looks like something completely doeable with normal where() conditions. I mean these are just different WHERE conditions that should be AND-ed or OR-ed?

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 this pull request may close these issues.

2 participants