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

Query: Determine omitted & target columns correctly #139

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

yhabteab
Copy link
Member

Excluding a column from a query/model that also contains expression columns fails as follows: I originally noticed this when working on Icinga/icingaweb2-module-nagvis#61 where I wanted to exclude some columns from the ServicestateSummary model that were causing a PostgreSQL grouping error, but you can also easily trigger this in Icinga DB Web by adding $servicegroupss->withoutColumns(‘id’); in ServicegroupsController for example.

Bildschirmfoto 2024-09-20 um 09 46 32

@nilmerg
Copy link
Member

nilmerg commented Sep 23, 2024

We now know that expressions cause problems here, why not support them properly?

In case with() or without() is called with an alias of an expression, I'd expect the expression should be included/excluded (respectively).

@yhabteab yhabteab force-pushed the bugfix/without-columns-and-expr-columns branch from d9bd9fb to d141e1f Compare September 23, 2024 09:56
@yhabteab yhabteab requested review from nilmerg and removed request for nilmerg September 23, 2024 09:57
@yhabteab yhabteab force-pushed the bugfix/without-columns-and-expr-columns branch from d141e1f to 2010c4c Compare September 23, 2024 10:30
src/Query.php Outdated Show resolved Hide resolved
@yhabteab yhabteab requested a review from nilmerg September 23, 2024 13:42
src/Query.php Outdated Show resolved Hide resolved
src/Query.php Outdated Show resolved Hide resolved
tests/QueryTest.php Outdated Show resolved Hide resolved
@yhabteab yhabteab force-pushed the bugfix/without-columns-and-expr-columns branch from 2010c4c to b513a7f Compare September 24, 2024 07:21
@yhabteab yhabteab requested a review from nilmerg September 24, 2024 07:22
@nilmerg nilmerg added the bug Something isn't working label Sep 24, 2024
@nilmerg nilmerg added this to the v0.6.2 milestone Sep 24, 2024
@nilmerg nilmerg merged commit 832d9ff into main Sep 24, 2024
22 checks passed
@nilmerg nilmerg deleted the bugfix/without-columns-and-expr-columns branch September 24, 2024 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cla/signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants