Skip to content

fix(sql-check): avoid npe when virtual column exists #2031

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.

Sign up for GitHub

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

Merged
yhilmare merged 1 commit into from
Mar 25, 2024
Merged

fix(sql-check): avoid npe when virtual column exists #2031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ...ice/src/main/java/com/oceanbase/odc/service/sqlcheck/rule/BaseRestrictIndexDataTypes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ private List<CheckViolation> builds(String sql, Stream<ColumnDefinition> stream)
ColumnAttributes a = d.getColumnAttributes();
if (a == null || CollectionUtils.isEmpty(a.getConstraints())) {
return false;
} else if (d.getDataType() == null) {
return false;
}
return a.getConstraints().stream().anyMatch(c -> c.isPrimaryKey() || c.isUniqueKey())
&& !isTypeAllowed(d.getDataType().getName());
Expand Down
5 changes: 5 additions & 0 deletions ...ervice/src/main/java/com/oceanbase/odc/service/sqlcheck/rule/BaseRestrictPKDataTypes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ protected CreateTable getTable(String schema, String tableName, SqlCheckContext
protected Map<String, String> getColumnName2TypeName(CreateTable createTable) {
Map<String, String> col2TypeName = new HashMap<>();
createTable.getColumnDefinitions().forEach(d -> {
if (d.getDataType() == null) {
return;
}
String column = unquoteIdentifier(d.getColumnReference().getColumn());
col2TypeName.put(column, d.getDataType().getName());
});
Expand Down Expand Up @@ -160,6 +163,8 @@ private List<CheckViolation> builds(String sql, Stream<ColumnDefinition> stream)
ColumnAttributes a = d.getColumnAttributes();
if (a == null || CollectionUtils.isEmpty(a.getConstraints())) {
return false;
} else if (d.getDataType() == null) {
return false;
}
return a.getConstraints().stream().anyMatch(InLineConstraint::isPrimaryKey)
&& !isTypeAllowed(d.getDataType().getName());
Expand Down
2 changes: 1 addition & 1 deletion ...ain/java/com/oceanbase/odc/service/sqlcheck/rule/MySQLRestrictAutoIncrementDataTypes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ private List<CheckViolation> builds(String sql, Stream<ColumnDefinition> stream)
return false;
}
return Boolean.TRUE.equals(ca.getAutoIncrement());
}).filter(d -> this.allowedTypeNames.stream()
}).filter(d -> d.getDataType() != null && this.allowedTypeNames.stream()
.noneMatch(s -> StringUtils.equalsIgnoreCase(s, d.getDataType().getName())))
.map(d -> {
DataType type = d.getDataType();
Expand Down
11 changes: 9 additions & 2 deletions ...c-service/src/main/java/com/oceanbase/odc/service/sqlcheck/rule/NoDefaultValueExists.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,16 +76,19 @@ public List<DialectType> getSupportsDialectTypes() {
}

private boolean containsIgnoreCase(String name) {
if (name == null) {
return true;
}
return this.typesAllowNoDefault.stream().anyMatch(s -> StringUtils.equalsIgnoreCase(s, name));
}

private List<CheckViolation> builds(String sql, Stream<ColumnDefinition> stream) {
return stream.filter(d -> {
ColumnAttributes attributes = d.getColumnAttributes();
if (attributes == null) {
return !containsIgnoreCase(d.getDataType().getName());
return !containsIgnoreCase(getDataTypeName(d));
}
return attributes.getDefaultValue() == null && !containsIgnoreCase(d.getDataType().getName());
return attributes.getDefaultValue() == null && !containsIgnoreCase(getDataTypeName(d));
}).map(d -> {
String dataTypes = "N/A";
if (CollectionUtils.isNotEmpty(typesAllowNoDefault)) {
Expand All @@ -95,4 +98,8 @@ private List<CheckViolation> builds(String sql, Stream<ColumnDefinition> stream)
}).collect(Collectors.toList());
}

private String getDataTypeName(ColumnDefinition definition) {
return definition.getDataType() == null ? null : definition.getDataType().getName();
}

}
5 changes: 4 additions & 1 deletion ...rvice/src/main/java/com/oceanbase/odc/service/sqlcheck/rule/ProhibitedDatatypeExists.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public List<DialectType> getSupportsDialectTypes() {
}

private List<CheckViolation> builds(String sql, Stream<ColumnDefinition> stream) {
return stream.filter(d -> containsIgnoreCase(d.getDataType().getName()))
return stream.filter(d -> containsIgnoreCase(d.getDataType() == null ? null : d.getDataType().getName()))
.map(d -> {
DataType type = d.getDataType();
String dataTypes = "N/A";
Expand All @@ -88,6 +88,9 @@ private List<CheckViolation> builds(String sql, Stream<ColumnDefinition> stream)
}

private boolean containsIgnoreCase(String name) {
if (name == null) {
return false;
}
return this.prohibitedTypeNames.stream().anyMatch(s -> StringUtils.equalsIgnoreCase(s, name));
}

Expand Down
13 changes: 10 additions & 3 deletions ...-service/src/main/java/com/oceanbase/odc/service/sqlcheck/rule/RestrictColumnNotNull.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,21 +77,24 @@ public List<DialectType> getSupportsDialectTypes() {
}

private boolean containsIgnoreCase(String name) {
if (name == null) {
return true;
}
return this.nullableDatatypes.stream().anyMatch(s -> StringUtils.equalsIgnoreCase(s, name));
}

private List<CheckViolation> builds(String sql, Stream<ColumnDefinition> stream) {
return stream.filter(d -> {
ColumnAttributes attributes = d.getColumnAttributes();
if (attributes == null) {
return !containsIgnoreCase(d.getDataType().getName());
return !containsIgnoreCase(getDataTypeName(d));
}
List<InLineConstraint> cs = attributes.getConstraints();
if (CollectionUtils.isEmpty(cs)) {
return !containsIgnoreCase(d.getDataType().getName());
return !containsIgnoreCase(getDataTypeName(d));
}
return cs.stream().noneMatch(c -> Boolean.FALSE.equals(c.getNullable()))
&& !containsIgnoreCase(d.getDataType().getName());
&& !containsIgnoreCase(getDataTypeName(d));
}).map(d -> {
String dataTypes = "N/A";
if (CollectionUtils.isNotEmpty(nullableDatatypes)) {
Expand All @@ -101,4 +104,8 @@ private List<CheckViolation> builds(String sql, Stream<ColumnDefinition> stream)
}).collect(Collectors.toList());
}

private String getDataTypeName(ColumnDefinition definition) {
return definition.getDataType() == null ? null : definition.getDataType().getName();
}

}
25 changes: 20 additions & 5 deletions ...er/odc-service/src/test/java/com/oceanbase/odc/service/sqlcheck/OracleSqlCheckerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,10 @@ public void check_restrictPrimaryKeyType_violationGenerated() {
+ "constraint abcd_pk primary key(name, \"age\"))",
"alter table abcd add primary key (NAME)",
"alter table abcd add primary key (age)",
"alter table \"abcd\" add primary key (NAME)"
"alter table \"abcd\" add primary key (NAME)",
"CREATE TABLE ORACLE_RANGE_VARCHAR_2(\n"
+ " char_column_number GENERATED ALWAYS AS (TO_NUMBER(char_column)) VIRTUAL\n"
+ ")",
};
JdbcTemplate jdbcTemplate = Mockito.mock(JdbcTemplate.class);
Mockito.when(jdbcTemplate.queryForObject(Mockito.anyString(), Mockito.any(RowMapper.class)))
Expand Down Expand Up @@ -563,7 +566,10 @@ public void check_restrictIndexType_violationGenerated() {
"alter table abcd add unique (\"age\")",
"create index abcd_idx1 on abcd(id)",
"create index abcd_idx2 on abcd(id, name)",
"create index abcd_idx3 on abcd(age_t, name)"
"create index abcd_idx3 on abcd(age_t, name)",
"CREATE TABLE ORACLE_RANGE_VARCHAR_2(\n"
+ " char_column_number GENERATED ALWAYS AS (TO_NUMBER(char_column)) VIRTUAL\n"
+ ")"
};
JdbcTemplate jdbcTemplate = Mockito.mock(JdbcTemplate.class);
Mockito.when(jdbcTemplate.queryForObject(Mockito.anyString(), Mockito.any(RowMapper.class)))
Expand Down Expand Up @@ -814,7 +820,10 @@ public void check_columnCollation_violationGenerated() {
public void check_columnIsNullable_violationGenerated() {
String[] sqls = new String[] {
"create table aaaa(id varchar(64), col1 blob, col2 number not null)",
"alter table abdcd add id number default 123, add id1 nchar(23) null, add col1 blob, add col2 number not null"
"alter table abdcd add id number default 123, add id1 nchar(23) null, add col1 blob, add col2 number not null",
"CREATE TABLE ORACLE_RANGE_VARCHAR_2(\n"
+ " char_column_number GENERATED ALWAYS AS (TO_NUMBER(char_column)) VIRTUAL\n"
+ ")"
};
DefaultSqlChecker sqlChecker = new DefaultSqlChecker(DialectType.OB_ORACLE, "$$",
Collections.singletonList(new RestrictColumnNotNull(new HashSet<>(Collections.singletonList("blob")))));
Expand All @@ -833,7 +842,10 @@ public void check_columnIsNullable_violationGenerated() {
public void check_noDefaultValue_violationGenerated() {
String[] sqls = new String[] {
"create table aaaa(id varchar(64), col1 blob, col2 number default 123)",
"alter table abdcd add id number unique, add id1 nchar(23) null, add col1 blob, add col2 number default 567"
"alter table abdcd add id number unique, add id1 nchar(23) null, add col1 blob, add col2 number default 567",
"CREATE TABLE ORACLE_RANGE_VARCHAR_2(\n"
+ " char_column_number GENERATED ALWAYS AS (TO_NUMBER(char_column)) VIRTUAL\n"
+ ")"
};
DefaultSqlChecker sqlChecker = new DefaultSqlChecker(DialectType.OB_ORACLE, "$$",
Collections.singletonList(new NoDefaultValueExists(new HashSet<>(Collections.singletonList("blob")))));
Expand Down Expand Up @@ -1062,7 +1074,10 @@ public void check_prohibitedDataTypes_violationGenerated() {
String[] sqls = {
"create table abcd(ida blob not null, col1 varchar(64) not null default 'abcd')",
"alter table abcd add idb varchar2(64) not null",
"alter table abcd add idv clob not null default 123"
"alter table abcd add idv clob not null default 123",
"CREATE TABLE ORACLE_RANGE_VARCHAR_2(\n"
+ " char_column_number GENERATED ALWAYS AS (TO_NUMBER(char_column)) VIRTUAL\n"
+ ")"
};
DefaultSqlChecker sqlChecker = new DefaultSqlChecker(DialectType.OB_ORACLE,
null,
Expand Down