-
Notifications
You must be signed in to change notification settings - Fork 7
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
[DAT-18844] resolved some sonar issues #207
base: main
Are you sure you want to change the base?
Conversation
Quality Gate passedIssues Measures |
@@ -16,11 +15,6 @@ public class AlterTablePropertiesChangeDatabricks extends AbstractAlterPropertie | |||
private static final String CHANGE_TYPE_SUBJECT = "Table"; | |||
private String tableName; | |||
|
|||
@Override |
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.
AbstractAlterPropertiesChangeDatabricks already has supports() method that check DatabricksDatabase
@@ -106,20 +103,17 @@ public Sql[] generateSql(InsertOrUpdateStatement insertOrUpdateStatement, Databa | |||
completeSql.append(updateStatement); | |||
|
|||
// Add insert statement to MERGE if not ONLY an update statement | |||
if (!insertOrUpdateStatement.getOnlyUpdate()) { | |||
if (Boolean.FALSE.equals(insertOrUpdateStatement.getOnlyUpdate())) { |
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.
Was it a complain because of autoboxing?
I have a bunch of similar checks in other places, but with primitive type.
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.
kinda, it complains about auto-Unboxing, saying in case of getOnlyUpdate() is null
we'll get NPE
LGTM! |
No description provided.