-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix NPE on adding new columns in the tables #12464
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
base: 4.20
Are you sure you want to change the base?
Fix NPE on adding new columns in the tables #12464
Conversation
|
@blueorangutan package |
|
@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12464 +/- ##
=========================================
Coverage 16.23% 16.23%
Complexity 13380 13380
=========================================
Files 5657 5657
Lines 499035 499043 +8
Branches 60567 60567
=========================================
+ Hits 81029 81041 +12
+ Misses 408969 408963 -6
- Partials 9037 9039 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✖️ debian ✔️ suse15. SL-JID 16417 |
DaanHoogland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@harikrishna-patnala , I don’t see any of the possible NPEs in the old code being mitigated in the new code. Am I missing something?
This is happening during any development or when an extra column was added in the table and no reference in the VO. Here few safe checks are added for that. |
68b203e to
0679607
Compare
framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java
Outdated
Show resolved
Hide resolved
@DaanHoogland unfortunately I could not reproduce this now, as the column count seems to be fine in most of the cases, but I think this has caused issues in retrieving the attr object when the sql statement usage contains "SELECT *". In remaining cases where specific columns in SELECT query are used, there won't be this issue. I feel this check can be kept as we are rightly logging when attr is not found. Please let me know your thoughts. |
|
Tnx @harikrishna-patnala , I think this can be white-box tested by adding a column to a table in “production”. Some operators may do that to try out and or filter things or aggregate data. If we know how this NPE occurred in the wild it will be easy to simulate in a test env. |
I've tried couple of operations with mshost, configuration, account, domain tables but the tested operations didnt hit the "select *" query. Currently we don't have the information of last failure happened. |
Description
This PR fixes an NPE in case during the development or any other case when there is an extra column in the tables that are not there in the corresponding VO.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?