Skip to content

Conversation

@zainforbjs
Copy link
Contributor

Added an exception in case the column specified in the select or group argument passed in the findAll() function does not exist in the database.

Added an exception in case the column specified in the select or group argument passed in the findAll() function does not exist in the database.
Changed the test case as suggested by @chapmandu
Copy link
Collaborator

@bpamiri bpamiri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@bpamiri
Copy link
Collaborator

bpamiri commented Mar 15, 2023

Closes #591

Copy link
Collaborator

@bpamiri bpamiri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zainforbjs this is breaking on columns added via a class property as in calculated or aliased columns. Do a search for firstID and you'll see it is defined as a property on the Post model but is getting caught by this exception because the column doesn't exist in the DB.

The test cases that were breaking was due to the group by statement that does not add the calculated property if there is an aggregate function in the query to the group by clause. This was breaking the test case so added the condition to only throw error in case of "select". Also added another condition due to the breaking of self join test case. If the table is self joined, the same columns are already in the select clause so the "local.toAppend" does not get the value in case of duplicate columns from the same table.
It gets the columns if the dupes are in different tables as they are then aliased with table dot notation.
@zainforbjs
Copy link
Contributor Author

zainforbjs commented Mar 16, 2023

@bpamiri Added some conditions to fix the 5 test cases that were failing. Added details in the commit.

@zainforbjs zainforbjs requested a review from bpamiri March 16, 2023 10:54
@bpamiri bpamiri merged commit 05f54c3 into wheels-dev:develop Sep 8, 2023
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