fix(materialization): dont always run get column on update if no query is there #29271
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR optimizes the update process for saved queries in the data warehouse by avoiding unnecessary column recalculation when the query itself hasn't changed.
- Modified
DataWarehouseSavedQuerySerializer.update()inposthog/warehouse/api/saved_query.pyto only callget_columns()and update status when the query field is present in validated data - Added test
test_update_without_query_change_doesnt_call_get_columnsinposthog/warehouse/api/test/test_saved_query.pyto verify optimization when only updating the name - Added test
test_update_with_query_change_calls_get_columnsto ensure column recalculation still happens when the query changes - This optimization improves performance for view updates by eliminating redundant processing
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| except RecursionError: | ||
| raise serializers.ValidationError("Model contains a cycle") | ||
| except Exception as err: | ||
| raise serializers.ValidationError(str(err)) |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 1 day ago
To fix the problem, we should avoid exposing the detailed error message to the end user. Instead, we can log the detailed error message on the server and return a generic error message to the user. This approach ensures that developers can still access the detailed error information for debugging purposes, while the end user only sees a generic message.
- Modify the code on line 154 to log the detailed error message using the
loggerand raise aserializers.ValidationErrorwith a generic error message. - Ensure that the
loggeris used to capture the detailed error information.
| @@ -153,3 +153,4 @@ | ||
| except Exception as err: | ||
| raise serializers.ValidationError(str(err)) | ||
| logger.exception("An error occurred while updating the view: %s", err) | ||
| raise serializers.ValidationError("An internal error has occurred.") | ||
|
|
Problem
Changes
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Does this work well for both Cloud and self-hosted?
How did you test this code?