Skip to content

Commit 65a57e3

Browse files
kotkar-pallavikotkar-pallavi
authored
Updating exists() check and filter query to filter by deleted_ts column (#539)
* Updating exists() check and filter query to filter by deleted_ts column * Moving deleted_ts check towards end and fixing unit tests * Resolving merge conflict and fixing new unit tests
1 parent 76b54f2 commit 65a57e3

9 files changed

+76
-51
lines changed

‎dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -481,16 +481,10 @@ private SqlQuery createFilterSqlQuery(@Nullable IndexFilter indexFilter,
481481
filterSql.append(SQLStatementUtils.createFilterSql(_entityType, indexFilter, false, _nonDollarVirtualColumnsEnabled, validator));
482482

483483
if (lastUrn != null) {
484-
// because createFilterSql will only include a WHERE clause if there are non-urn filters, we need to make sure
485-
// that we add a WHERE if it wasn't added already.
484+
// because createFilterSql will always include a WHERE clause to filter by deleted_ts is NULL
485+
// We will append the lastUrn condition to the WHERE clause with AND operator.
486486
String operator = "AND";
487487

488-
if (indexFilter == null
489-
|| !indexFilter.hasCriteria()
490-
|| indexFilter.getCriteria().stream().allMatch(criteria -> isUrn(criteria.getAspect()))) {
491-
operator = "WHERE";
492-
}
493-
494488
filterSql.append(String.format(" %s URN > '%s'", operator, lastUrn));
495489
}
496490

‎dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtils.java

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
import org.apache.commons.lang.StringEscapeUtils;
2020

2121
import static com.linkedin.metadata.dao.utils.SQLSchemaUtils.*;
22-
import static com.linkedin.metadata.dao.utils.SQLStatementUtils.SOFT_DELETED_CHECK;
22+
import static com.linkedin.metadata.dao.utils.SQLStatementUtils.*;
2323

2424

2525
/**
@@ -100,39 +100,38 @@ public static String parseIndexFilter(@Nonnull String entityType, @Nullable Inde
100100
boolean nonDollarVirtualColumnsEnabled, @Nonnull SchemaValidatorUtil schemaValidator) {
101101
List<String> sqlFilters = new ArrayList<>();
102102

103-
if (indexFilter == null || !indexFilter.hasCriteria()) {
104-
return "";
105-
}
106-
107-
for (IndexCriterion indexCriterion : indexFilter.getCriteria()) {
108-
final String aspect = indexCriterion.getAspect();
109-
if (!isUrn(aspect)) {
110-
// if aspect is not urn, then check aspect is not soft deleted and is not null
111-
final String aspectColumn = getAspectColumnName(entityType, indexCriterion.getAspect());
112-
sqlFilters.add(aspectColumn + " IS NOT NULL");
113-
sqlFilters.add(String.format(SOFT_DELETED_CHECK, aspectColumn));
114-
}
103+
// Process index filter criteria if present
104+
if (indexFilter != null && indexFilter.hasCriteria()) {
105+
for (IndexCriterion indexCriterion : indexFilter.getCriteria()) {
106+
final String aspect = indexCriterion.getAspect();
107+
if (!isUrn(aspect)) {
108+
// if aspect is not urn, then check aspect is not soft deleted and is not null
109+
final String aspectColumn = getAspectColumnName(entityType, indexCriterion.getAspect());
110+
sqlFilters.add(aspectColumn + " IS NOT NULL");
111+
sqlFilters.add(String.format(SOFT_DELETED_CHECK, aspectColumn));
112+
}
115113

116-
final IndexPathParams pathParams = indexCriterion.getPathParams(GetMode.NULL);
117-
if (pathParams != null) {
118-
validateConditionAndValue(indexCriterion);
119-
final Condition condition = pathParams.getCondition();
120-
final String indexColumn = getGeneratedColumnName(entityType, aspect, pathParams.getPath(), nonDollarVirtualColumnsEnabled);
121-
final String tableName = SQLSchemaUtils.getTableName(entityType);
122-
// New: Skip filter if column doesn't exist
123-
if (!schemaValidator.columnExists(tableName, indexColumn)) {
124-
log.warn("Skipping filter: virtual column '{}' not found in table '{}'", indexColumn, tableName);
125-
continue;
114+
final IndexPathParams pathParams = indexCriterion.getPathParams(GetMode.NULL);
115+
if (pathParams != null) {
116+
validateConditionAndValue(indexCriterion);
117+
final Condition condition = pathParams.getCondition();
118+
final String indexColumn = getGeneratedColumnName(entityType, aspect, pathParams.getPath(), nonDollarVirtualColumnsEnabled);
119+
final String tableName = SQLSchemaUtils.getTableName(entityType);
120+
// New: Skip filter if column doesn't exist
121+
if (!schemaValidator.columnExists(tableName, indexColumn)) {
122+
log.warn("Skipping filter: virtual column '{}' not found in table '{}'", indexColumn, tableName);
123+
continue;
124+
}
125+
sqlFilters.add(parseSqlFilter(indexColumn, condition, pathParams.getValue()));
126126
}
127-
sqlFilters.add(parseSqlFilter(indexColumn, condition, pathParams.getValue()));
128127
}
129128
}
130129

131-
if (sqlFilters.isEmpty()) {
132-
return "";
133-
} else {
134-
return "WHERE " + String.join("\nAND ", sqlFilters);
135-
}
130+
// Add soft deleted check.
131+
sqlFilters.add(DELETED_TS_IS_NULL_CHECK);
132+
133+
return "WHERE " + String.join("\nAND ", sqlFilters);
134+
136135
}
137136

138137
/**

‎dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ public class SQLStatementUtils {
4040

4141
public static final String SOFT_DELETED_CHECK = "JSON_EXTRACT(%s, '$.gma_deleted') IS NULL"; // true when not soft deleted
4242

43+
public static final String DELETED_TS_IS_NULL_CHECK = "deleted_ts IS NULL"; // true when the deleted_ts is NULL, meaning the record is not soft deleted
44+
4345
public static final String NONNULL_CHECK = "%s IS NOT NULL"; // true when the value of aspect_column is not NULL
4446

4547
// Build manual SQL update query to enable optimistic locking on a given column
@@ -114,7 +116,10 @@ public class SQLStatementUtils {
114116

115117
private static final String INDEX_GROUP_BY_CRITERION = "SELECT count(*) as COUNT, %s FROM %s";
116118

117-
private static final String SQL_URN_EXIST_TEMPLATE = "SELECT urn FROM %s WHERE urn = '%s'";
119+
private static final String SQL_GET_ALL_COLUMNS =
120+
"SELECT COLUMN_NAME FROM information_schema.COLUMNS WHERE TABLE_SCHEMA = database() AND TABLE_NAME = '%s'";
121+
122+
private static final String SQL_URN_EXIST_TEMPLATE = "SELECT urn FROM %s WHERE urn = '%s' AND deleted_ts IS NULL";
118123

119124
private static final String INSERT_LOCAL_RELATIONSHIP = "INSERT INTO %s (metadata, source, destination, source_type, "
120125
+ "destination_type, lastmodifiedon, lastmodifiedby) VALUE (:metadata, :source, :destination, :source_type,"

‎dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@ public void testParseIndexFilter() {
6161

6262
String sql = SQLIndexFilterUtils.parseIndexFilter(FooUrn.ENTITY_TYPE, indexFilter, false, mockValidator);
6363
assertEquals(sql,
64-
"WHERE a_aspectfoo IS NOT NULL\nAND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\nAND i_aspectfoo$id < 12");
64+
"WHERE a_aspectfoo IS NOT NULL\nAND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\nAND i_aspectfoo$id < 12\nAND deleted_ts IS NULL");
6565

6666
sql = SQLIndexFilterUtils.parseIndexFilter(FooUrn.ENTITY_TYPE, indexFilter, true, mockValidator);
6767
assertEquals(sql,
68-
"WHERE a_aspectfoo IS NOT NULL\nAND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\nAND i_aspectfoo0id < 12");
68+
"WHERE a_aspectfoo IS NOT NULL\nAND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\nAND i_aspectfoo0id < 12\nAND deleted_ts IS NULL");
6969
}
7070
}

‎dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -133,21 +133,23 @@ public void testCreateFilterSql() {
133133
String expectedSql1 = "SELECT *, (SELECT COUNT(urn) FROM metadata_entity_foo WHERE a_aspectfoo IS NOT NULL\n"
134134
+ "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n" + "AND i_aspectfoo$value >= 25\n"
135135
+ "AND a_aspectfoo IS NOT NULL\n" + "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n"
136-
+ "AND i_aspectfoo$value < 50) as _total_count FROM metadata_entity_foo\n" + "WHERE a_aspectfoo IS NOT NULL\n"
137-
+ "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n" + "AND i_aspectfoo$value >= 25\n"
138-
+ "AND a_aspectfoo IS NOT NULL\n" + "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n"
139-
+ "AND i_aspectfoo$value < 50";
136+
+ "AND i_aspectfoo$value < 50\n" + "AND deleted_ts IS NULL)" + " as _total_count FROM metadata_entity_foo\n"
137+
+ "WHERE a_aspectfoo IS NOT NULL\n" + "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n"
138+
+ "AND i_aspectfoo$value >= 25\n" + "AND a_aspectfoo IS NOT NULL\n"
139+
+ "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n"
140+
+ "AND i_aspectfoo$value < 50\n" + "AND deleted_ts IS NULL";
140141

141142
assertEquals(sql1, expectedSql1);
142143

143144
String sql2 = SQLStatementUtils.createFilterSql("foo", indexFilter, true, true, mockValidator);
144145
String expectedSql2 = "SELECT *, (SELECT COUNT(urn) FROM metadata_entity_foo WHERE a_aspectfoo IS NOT NULL\n"
145146
+ "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n" + "AND i_aspectfoo0value >= 25\n"
146147
+ "AND a_aspectfoo IS NOT NULL\n" + "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n"
147-
+ "AND i_aspectfoo0value < 50) as _total_count FROM metadata_entity_foo\n" + "WHERE a_aspectfoo IS NOT NULL\n"
148-
+ "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n" + "AND i_aspectfoo0value >= 25\n"
149-
+ "AND a_aspectfoo IS NOT NULL\n" + "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n"
150-
+ "AND i_aspectfoo0value < 50";
148+
+ "AND i_aspectfoo0value < 50\n" + "AND deleted_ts IS NULL)" + " as _total_count FROM metadata_entity_foo\n"
149+
+ "WHERE a_aspectfoo IS NOT NULL\n" + "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n"
150+
+ "AND i_aspectfoo0value >= 25\n" + "AND a_aspectfoo IS NOT NULL\n"
151+
+ "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n"
152+
+ "AND i_aspectfoo0value < 50\n" + "AND deleted_ts IS NULL";
151153

152154
assertEquals(sql2, expectedSql2);
153155
}
@@ -176,14 +178,14 @@ public void testCreateGroupBySql() {
176178
+ "WHERE a_aspectfoo IS NOT NULL\n" + "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n"
177179
+ "AND i_aspectfoo$value >= 25\n" + "AND a_aspectfoo IS NOT NULL\n"
178180
+ "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n" + "AND i_aspectfoo$value < 50\n"
179-
+ "GROUP BY i_aspectfoo$value");
181+
+ "AND deleted_ts IS NULL\n" + "GROUP BY i_aspectfoo$value");
180182

181183
String sql2 = SQLStatementUtils.createGroupBySql("foo", indexFilter, indexGroupByCriterion, true, mockValidator);
182184
assertEquals(sql2, "SELECT count(*) as COUNT, i_aspectfoo0value FROM metadata_entity_foo\n"
183185
+ "WHERE a_aspectfoo IS NOT NULL\n" + "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n"
184186
+ "AND i_aspectfoo0value >= 25\n" + "AND a_aspectfoo IS NOT NULL\n"
185187
+ "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n" + "AND i_aspectfoo0value < 50\n"
186-
+ "GROUP BY i_aspectfoo0value");
188+
+ "AND deleted_ts IS NULL\n" + "GROUP BY i_aspectfoo0value");
187189
}
188190

189191
@Test
@@ -483,6 +485,16 @@ public void testAspectField() {
483485
assertEquals(SQLStatementUtils.getAssetType(aspectField), BarUrn.ENTITY_TYPE);
484486
}
485487

488+
@Test
489+
public void testExistsSql() {
490+
FooUrn fooUrn = makeFooUrn(1);
491+
String expectedSql = "SELECT urn "
492+
+ "FROM metadata_entity_foo "
493+
+ "WHERE urn = 'urn:li:foo:1' "
494+
+ "AND deleted_ts IS NULL";
495+
assertConditionsEqual(SQLStatementUtils.createExistSql(fooUrn), expectedSql);
496+
}
497+
486498
@Test
487499
public void testParseIndexFilterSkipsMissingVirtualColumn() {
488500
SchemaValidatorUtil mockValidator1 = mock(SchemaValidatorUtil.class);
@@ -546,7 +558,7 @@ public void testCreateGroupBySqlFilterColumnPresentGroupByColumnPresent() {
546558
+ "WHERE a_aspectfoo IS NOT NULL\n" + "AND JSON_EXTRACT(a_aspectfoo, '$.gma_deleted') IS NULL\n"
547559
+ "AND i_aspectfoo$age >= 25\n" + "AND a_aspectbar IS NOT NULL\n"
548560
+ "AND JSON_EXTRACT(a_aspectbar, '$.gma_deleted') IS NULL\n" + "AND i_aspectbar$name = 'PizzaMan'\n"
549-
+ "GROUP BY i_aspectbar$name");
561+
+ "AND deleted_ts IS NULL\n" + "GROUP BY i_aspectbar$name");
550562
}
551563

552564
@Test
@@ -583,6 +595,7 @@ public void testCreateGroupBySqlFilterColumnMissingGroupByPresent() {
583595
+ "AND a_aspectbar IS NOT NULL\n"
584596
+ "AND JSON_EXTRACT(a_aspectbar, '$.gma_deleted') IS NULL\n"
585597
+ "AND i_aspectbar$name = 'PizzaMan'\n"
598+
+ "AND deleted_ts IS NULL\n"
586599
+ "GROUP BY i_aspectbar$name");
587600
}
588601

‎dao-impl/ebean-dao/src/test/resources/ebean-local-access-create-all-with-non-dollar-virtual-column-names.sql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_foo (
1212
lastmodifiedon TIMESTAMP NOT NULL,
1313
lastmodifiedby VARCHAR(255) NOT NULL,
1414
createdfor VARCHAR(255),
15+
deleted_ts datetime(6) DEFAULT NULL,
1516
CONSTRAINT pk_metadata_entity_foo PRIMARY KEY (urn)
1617
);
1718

@@ -21,6 +22,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_bar (
2122
lastmodifiedon TIMESTAMP NOT NULL,
2223
lastmodifiedby VARCHAR(255) NOT NULL,
2324
createdfor VARCHAR(255),
25+
deleted_ts datetime(6) DEFAULT NULL,
2426
CONSTRAINT pk_metadata_entity_bar PRIMARY KEY (urn)
2527
);
2628

@@ -30,6 +32,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_burger (
3032
lastmodifiedon TIMESTAMP NOT NULL,
3133
lastmodifiedby VARCHAR(255) NOT NULL,
3234
createdfor VARCHAR(255),
35+
deleted_ts datetime(6) DEFAULT NULL,
3336
CONSTRAINT pk_metadata_entity_burger PRIMARY KEY (urn)
3437
);
3538

‎dao-impl/ebean-dao/src/test/resources/ebean-local-access-create-all.sql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_foo (
1212
lastmodifiedon TIMESTAMP NOT NULL,
1313
lastmodifiedby VARCHAR(255) NOT NULL,
1414
createdfor VARCHAR(255),
15+
deleted_ts datetime(6) DEFAULT NULL,
1516
CONSTRAINT pk_metadata_entity_foo PRIMARY KEY (urn)
1617
);
1718

@@ -21,6 +22,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_bar (
2122
lastmodifiedon TIMESTAMP NOT NULL,
2223
lastmodifiedby VARCHAR(255) NOT NULL,
2324
createdfor VARCHAR(255),
25+
deleted_ts datetime(6) DEFAULT NULL,
2426
CONSTRAINT pk_metadata_entity_bar PRIMARY KEY (urn)
2527
);
2628

@@ -30,6 +32,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_burger (
3032
lastmodifiedon TIMESTAMP NOT NULL,
3133
lastmodifiedby VARCHAR(255) NOT NULL,
3234
createdfor VARCHAR(255),
35+
deleted_ts datetime(6) DEFAULT NULL,
3336
CONSTRAINT pk_metadata_entity_burger PRIMARY KEY (urn)
3437
);
3538

‎dao-impl/ebean-dao/src/test/resources/ebean-local-dao-create-all-with-non-dollar-virtual-column-names.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_foo (
1414
lastmodifiedon TIMESTAMP NOT NULL,
1515
lastmodifiedby VARCHAR(255) NOT NULL,
1616
createdfor VARCHAR(255),
17+
deleted_ts datetime(6) DEFAULT NULL,
1718
CONSTRAINT pk_metadata_entity_foo PRIMARY KEY (urn)
1819
);
1920

@@ -23,6 +24,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_foo_test (
2324
lastmodifiedon TIMESTAMP NOT NULL,
2425
lastmodifiedby VARCHAR(255) NOT NULL,
2526
createdfor VARCHAR(255),
27+
deleted_ts datetime(6) DEFAULT NULL,
2628
CONSTRAINT pk_metadata_entity_foo_test PRIMARY KEY (urn)
2729
);
2830

@@ -32,6 +34,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_bar (
3234
lastmodifiedon TIMESTAMP NOT NULL,
3335
lastmodifiedby VARCHAR(255) NOT NULL,
3436
createdfor VARCHAR(255),
37+
deleted_ts datetime(6) DEFAULT NULL,
3538
CONSTRAINT pk_metadata_entity_bar PRIMARY KEY (urn)
3639
);
3740

@@ -41,6 +44,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_burger (
4144
lastmodifiedon TIMESTAMP NOT NULL,
4245
lastmodifiedby VARCHAR(255) NOT NULL,
4346
createdfor VARCHAR(255),
47+
deleted_ts datetime(6) DEFAULT NULL,
4448
CONSTRAINT pk_metadata_entity_burger PRIMARY KEY (urn)
4549
);
4650

‎dao-impl/ebean-dao/src/test/resources/ebean-local-dao-create-all.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_foo (
1414
lastmodifiedon TIMESTAMP NOT NULL,
1515
lastmodifiedby VARCHAR(255) NOT NULL,
1616
createdfor VARCHAR(255),
17+
deleted_ts datetime(6) DEFAULT NULL,
1718
CONSTRAINT pk_metadata_entity_foo PRIMARY KEY (urn)
1819
);
1920

@@ -23,6 +24,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_foo_test (
2324
lastmodifiedon TIMESTAMP NOT NULL,
2425
lastmodifiedby VARCHAR(255) NOT NULL,
2526
createdfor VARCHAR(255),
27+
deleted_ts datetime(6) DEFAULT NULL,
2628
CONSTRAINT pk_metadata_entity_foo_test PRIMARY KEY (urn)
2729
);
2830

@@ -32,6 +34,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_bar (
3234
lastmodifiedon TIMESTAMP NOT NULL,
3335
lastmodifiedby VARCHAR(255) NOT NULL,
3436
createdfor VARCHAR(255),
37+
deleted_ts datetime(6) DEFAULT NULL,
3538
CONSTRAINT pk_metadata_entity_bar PRIMARY KEY (urn)
3639
);
3740

@@ -41,6 +44,7 @@ CREATE TABLE IF NOT EXISTS metadata_entity_burger (
4144
lastmodifiedon TIMESTAMP NOT NULL,
4245
lastmodifiedby VARCHAR(255) NOT NULL,
4346
createdfor VARCHAR(255),
47+
deleted_ts datetime(6) DEFAULT NULL,
4448
CONSTRAINT pk_metadata_entity_burger PRIMARY KEY (urn)
4549
);
4650

0 commit comments

Comments
 (0)