From 805d0e9dfdcfe5e5379e675835812db955d76235 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Thu, 20 Nov 2025 00:05:26 -0800 Subject: [PATCH 01/18] initial changes, need propagation + injection --- .../dao/utils/SQLIndexFilterUtils.java | 42 +++++++++++++------ .../metadata/dao/utils/SQLSchemaUtils.java | 29 +++++++++++-- .../metadata/dao/utils/SQLStatementUtils.java | 29 ++++++++++++- 3 files changed, 83 insertions(+), 17 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtils.java index e75798d52..018957d1f 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtils.java @@ -65,29 +65,47 @@ private static String parseIndexValue(@Nullable IndexValue indexValue) { } } + @Nullable + private static String getIndexedExpressionOrColumnGeneric(@Nonnull String expectedLegacyColumnName, @Nonnull String expectedExpressionIndexName, + @Nonnull String tableName, @Nonnull SchemaValidatorUtil schemaValidator) { + // Check if an expression-based index exists... if it does, use that + final String indexExpression = schemaValidator.getIndexExpression(tableName, expectedExpressionIndexName); + if (indexExpression != null) { + log.info("Using expression index '{}' in table '{}' with expression '{}'", expectedExpressionIndexName, tableName, indexExpression); + return indexExpression; + } else if (schemaValidator.columnExists(tableName, expectedLegacyColumnName)) { + // (Pre-functional-index logic) Check for existence of (virtual) column + return expectedLegacyColumnName; + } else { + return null; + } + } + /** * Get the expression index "identifier", if it exists, otherwise retrieve the generated column name. * The idea behind this is that whatever is returned from this method can be used verbatim to query the database; * it's either the expression index itself (new approach) or the virtual column (old approach). + * Intended to be used with entity tables. */ @Nullable public static String getIndexedExpressionOrColumn(@Nonnull String assetType, @Nonnull String aspect, @Nonnull String path, boolean nonDollarVirtualColumnsEnabled, @Nonnull SchemaValidatorUtil schemaValidator) { final String indexColumn = getGeneratedColumnName(assetType, aspect, path, nonDollarVirtualColumnsEnabled); final String tableName = getTableName(assetType); + return getIndexedExpressionOrColumnGeneric(indexColumn, getExpressionIndexName(assetType, aspect, path), tableName, schemaValidator); + } - // Check if an expression-based index exists... if it does, use that - final String expressionIndexName = getExpressionIndexName(assetType, aspect, path); - final String indexExpression = schemaValidator.getIndexExpression(tableName, expressionIndexName); - if (indexExpression != null) { - log.info("Using expression index '{}' in table '{}' with expression '{}'", expressionIndexName, tableName, indexExpression); - return indexExpression; - } else if (schemaValidator.columnExists(tableName, indexColumn)) { - // (Pre-functional-index logic) Check for existence of (virtual) column - return indexColumn; - } else { - return null; - } + /** + * Get the expression index "identifier", if it exists, otherwise retrieve the generated column name. + * The idea behind this is that whatever is returned from this method can be used verbatim to query the database; + * it's either the expression index itself (new approach) or the virtual column (old approach). + * Intended to be used with relationship tables. + */ + @Nullable + public static String getIndexedExpressionOrColumnRelationship(@Nonnull String expectedLegacyColumnName, @Nonnull String path, + @Nonnull String tableName, @Nonnull SchemaValidatorUtil schemaValidator) { + final String expectedExpressionIndexName = SQLSchemaUtils.getExpressionIndexNameRelationship(path); + return getIndexedExpressionOrColumnGeneric(expectedLegacyColumnName, expectedExpressionIndexName, tableName, schemaValidator); } /** diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLSchemaUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLSchemaUtils.java index 53f39538d..b5732acf8 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLSchemaUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLSchemaUtils.java @@ -25,6 +25,7 @@ public class SQLSchemaUtils { public static final String ASPECT_PREFIX = "a_"; public static final String INDEX_PREFIX = "i_"; public static final String EXPRESSION_INDEX_PREFIX = "e_"; + public static final String RELATIONSHIP_TABLE_EXPRESSION_INDEX_INFIX = "metadata"; private static final int MYSQL_MAX_COLUMN_NAME_LENGTH = 64 - ASPECT_PREFIX.length(); @@ -152,6 +153,16 @@ public static String getAspectColumnName(@Nonnul return getAspectColumnName(entityType, aspectClass.getCanonicalName()); } + @Nonnull + private static String getExpectedNameFormatter( + @Nonnull String prefix, + @Nonnull String infix, + @Nonnull String path, + boolean nonDollarVirtualColumnsEnabled) { + char delimiter = nonDollarVirtualColumnsEnabled ? '0' : '$'; + return prefix + infix + processPath(path, delimiter); + } + @Nonnull private static String getExpectedNameHelper( @Nonnull String prefix, @@ -159,14 +170,14 @@ private static String getExpectedNameHelper( @Nonnull String aspect, @Nonnull String path, boolean nonDollarVirtualColumnsEnabled) { - char delimiter = nonDollarVirtualColumnsEnabled ? '0' : '$'; if (isUrn(aspect)) { - return prefix + "urn" + processPath(path, delimiter); + return getExpectedNameFormatter(prefix, "urn", path, nonDollarVirtualColumnsEnabled); } if (UNKNOWN_ASSET.equals(assetType)) { - log.warn("query with unknown asset type. aspect = {}, path ={}, delimiter = {}", aspect, path, delimiter); + log.warn("query with unknown asset type. aspect = {}, path ={}, nonDollarVirtualColumnsEnabled={}", + aspect, path, nonDollarVirtualColumnsEnabled); } - return prefix + getColumnName(assetType, aspect) + processPath(path, delimiter); + return getExpectedNameFormatter(prefix, getColumnName(assetType, aspect), path, nonDollarVirtualColumnsEnabled); } /** @@ -191,6 +202,16 @@ public static String getExpressionIndexName(@Nonnull String assetType, @Nonnull return getExpectedNameHelper(EXPRESSION_INDEX_PREFIX, assetType, aspect, path, true); } + /** + * Get the expected expression index name for a relationship table given the path. + * With the expression index changes, we establish an expected naming as follows... + * ex. e_metadata0foo0bar + */ + @Nonnull + public static String getExpressionIndexNameRelationship(@Nonnull String path) { + return getExpectedNameFormatter(EXPRESSION_INDEX_PREFIX, RELATIONSHIP_TABLE_EXPRESSION_INDEX_INFIX, path, true); + } + /** * DEPRECATED, use getGeneratedColumnName(assetType, aspect, path, nonDollarVirtualColumnsEnabled) instead. */ diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index b2fa09a81..34359c586 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -1,5 +1,6 @@ package com.linkedin.metadata.dao.utils; +import com.google.common.annotations.VisibleForTesting; import com.google.common.escape.Escaper; import com.google.common.escape.Escapers; import com.linkedin.common.urn.Urn; @@ -587,6 +588,23 @@ public static String whereClauseOldSchema(@Nonnull Map suppor return sb.toString(); } + @VisibleForTesting + @Nonnull + protected static String addTablePrefixToExpression(@Nonnull String expression, @Nonnull String tablePrefix) { + if (tablePrefix == null || tablePrefix.isEmpty()) { + return expression; + } + + // Replace column references: `columnName` -> `tablePrefix`.`columnName` + // This is a simplified example - would need more robust parsing + return expression.replaceAll("`(a_\\w+)`", "`" + tablePrefix + "`.`$1`"); + } + + @VisibleForTesting + protected static String handleRelationshipField() { + + } + private static String parseLocalRelationshipField( @Nonnull final LocalRelationshipCriterion localRelationshipCriterion, @Nullable String tablePrefix, boolean nonDollarVirtualColumnsEnabled) { @@ -594,14 +612,23 @@ private static String parseLocalRelationshipField( LocalRelationshipCriterion.Field field = localRelationshipCriterion.getField(); char delimiter = nonDollarVirtualColumnsEnabled ? '0' : '$'; + // UrnField.pdl defines UrnField.name as 'urn' --> real column if (field.isUrnField()) { return tablePrefix + field.getUrnField().getName(); } + // RelationshipField.pdl defines RelationshipField.name as 'metadata' + // --> virtual column use case that needs to be functionalized if (field.isRelationshipField()) { - return tablePrefix + field.getRelationshipField().getName() + processPath(field.getRelationshipField().getPath(), delimiter); + final String expectedVirtualColumnName = + tablePrefix + field.getRelationshipField().getName() + processPath(field.getRelationshipField().getPath(), delimiter); + return SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( + expectedVirtualColumnName, field.getRelationshipField().getPath(), + tableName, schemaValidator); } + // This appears to be when a join has already occurred and this is some indexed field from an aspect column from + // the entity table(s) --> virtual column use case that needs to be functionalized if (field.isAspectField()) { // entity type from Urn definition. String assetType = getAssetType(field.getAspectField()); From a350be6e35fc07c59863cdf06f793ec46f68c343 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Fri, 21 Nov 2025 00:17:46 -0800 Subject: [PATCH 02/18] Large AI refactor, no new testing introduced though --- .../dao/EbeanLocalRelationshipQueryDAO.java | 9 +- .../utils/MultiHopsTraversalSqlGenerator.java | 9 +- .../metadata/dao/utils/SQLStatementUtils.java | 100 +++++++++++++----- .../dao/utils/SQLStatementUtilsTest.java | 46 ++++---- 4 files changed, 107 insertions(+), 57 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalRelationshipQueryDAO.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalRelationshipQueryDAO.java index 370a48792..6e86c88ba 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalRelationshipQueryDAO.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalRelationshipQueryDAO.java @@ -69,15 +69,15 @@ public class EbeanLocalRelationshipQueryDAO { public EbeanLocalRelationshipQueryDAO(EbeanServer server, EBeanDAOConfig eBeanDAOConfig) { _server = server; _eBeanDAOConfig = eBeanDAOConfig; - _sqlGenerator = new MultiHopsTraversalSqlGenerator(SUPPORTED_CONDITIONS); _schemaValidatorUtil = new SchemaValidatorUtil(server); + _sqlGenerator = new MultiHopsTraversalSqlGenerator(SUPPORTED_CONDITIONS, _schemaValidatorUtil); } public EbeanLocalRelationshipQueryDAO(EbeanServer server) { _server = server; _eBeanDAOConfig = new EBeanDAOConfig(); - _sqlGenerator = new MultiHopsTraversalSqlGenerator(SUPPORTED_CONDITIONS); _schemaValidatorUtil = new SchemaValidatorUtil(server); + _sqlGenerator = new MultiHopsTraversalSqlGenerator(SUPPORTED_CONDITIONS, _schemaValidatorUtil); } static final Map SUPPORTED_CONDITIONS = @@ -125,8 +125,8 @@ private List findEntitiesCore(@Nonnu final StringBuilder sqlBuilder = new StringBuilder(); sqlBuilder.append("SELECT * FROM ").append(tableName); if (filterHasNonEmptyCriteria(filter)) { - sqlBuilder.append(" WHERE ").append(SQLStatementUtils.whereClause(filter, SUPPORTED_CONDITIONS, null, - _eBeanDAOConfig.isNonDollarVirtualColumnsEnabled())); + sqlBuilder.append(" WHERE ").append(SQLStatementUtils.whereClause(filter, SUPPORTED_CONDITIONS, null, tableName, + _schemaValidatorUtil, _eBeanDAOConfig.isNonDollarVirtualColumnsEnabled())); } sqlBuilder.append(" ORDER BY urn LIMIT ").append(Math.max(1, count)).append(" OFFSET ").append(Math.max(0, offset)); @@ -795,6 +795,7 @@ public String buildFindRelationshipSQL(@Nonnull final String relationshipTableNa String whereClause = SQLStatementUtils.whereClause(SUPPORTED_CONDITIONS, _eBeanDAOConfig.isNonDollarVirtualColumnsEnabled(), + relationshipTableName, _schemaValidatorUtil, filters.toArray(new Pair[filters.size()])); if (whereClause != null) { diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/MultiHopsTraversalSqlGenerator.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/MultiHopsTraversalSqlGenerator.java index 7b26e9c85..d8516bcbc 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/MultiHopsTraversalSqlGenerator.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/MultiHopsTraversalSqlGenerator.java @@ -15,9 +15,11 @@ */ public class MultiHopsTraversalSqlGenerator { private static Map _supportedConditions; + private final SchemaValidatorUtil _schemaValidator; - public MultiHopsTraversalSqlGenerator(Map supportedConditions) { + public MultiHopsTraversalSqlGenerator(Map supportedConditions, SchemaValidatorUtil schemaValidator) { _supportedConditions = Collections.unmodifiableMap(supportedConditions); + _schemaValidator = schemaValidator; } /** @@ -77,6 +79,7 @@ private String firstHopUrnsDirected(String relationshipTable, String srcEntityTa urnColumn, relationshipTable, destEntityTable, srcEntityTable)); String whereClause = SQLStatementUtils.whereClause(_supportedConditions, nonDollarVirtualColumnsEnabled, + relationshipTable, _schemaValidator, new Pair<>(relationshipFilter, "rt"), new Pair<>(destFilter, "dt"), new Pair<>(srcFilter, "st")); @@ -105,6 +108,7 @@ private String firstHopUrnsUndirected(String relationshipTable, String entityTab relationshipTable, entityTable)); String whereClause = SQLStatementUtils.whereClause(_supportedConditions, nonDollarVirtualColumnsEnabled, + relationshipTable, _schemaValidator, new Pair<>(relationshipFilter, "rt"), new Pair<>(srcFilter, "et")); @@ -123,7 +127,8 @@ private String firstHopUrnsUndirected(String relationshipTable, String entityTab @ParametersAreNonnullByDefault private String findEntitiesUndirected(String entityTable, String relationshipTable, String firstHopUrnSql, LocalRelationshipFilter destFilter, boolean nonDollarVirtualColumnsEnabled) { - String whereClause = SQLStatementUtils.whereClause(_supportedConditions, nonDollarVirtualColumnsEnabled, new Pair<>(destFilter, "et")); + String whereClause = SQLStatementUtils.whereClause(_supportedConditions, nonDollarVirtualColumnsEnabled, + relationshipTable, _schemaValidator, new Pair<>(destFilter, "et")); StringBuilder sourceEntitySql = new StringBuilder( String.format("SELECT et.* FROM %s et INNER JOIN %s rt ON et.urn=rt.source WHERE rt.destination IN (%s)", diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index 34359c586..58b876e72 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -445,17 +445,20 @@ public static String deleteLocalRelationshipSQL(final String tableName, boolean * Construct where clause SQL from multiple filters. Return null if all filters are empty. * @param supportedConditions contains supported conditions such as EQUAL. * @param nonDollarVirtualColumnsEnabled true if virtual column does not contain $, false otherwise + * @param relationshipTableName the relationship table name (used for RelationshipField schema validation) + * @param schemaValidator schema validator for checking column/index existence * @param filters An array of pairs which are filter and table prefix. * @return sql that can be appended after where clause. */ @SafeVarargs @Nullable public static String whereClause(@Nonnull Map supportedConditions, boolean nonDollarVirtualColumnsEnabled, + @Nullable String relationshipTableName, @Nonnull SchemaValidatorUtil schemaValidator, @Nonnull Pair... filters) { List andClauses = new ArrayList<>(); for (Pair filter : filters) { if (LogicalExpressionLocalRelationshipCriterionUtils.filterHasNonEmptyCriteria(filter.getValue0())) { - andClauses.add("(" + whereClause(filter.getValue0(), supportedConditions, filter.getValue1(), nonDollarVirtualColumnsEnabled) + ")"); + andClauses.add("(" + whereClause(filter.getValue0(), supportedConditions, filter.getValue1(), relationshipTableName, schemaValidator, nonDollarVirtualColumnsEnabled) + ")"); } } if (andClauses.isEmpty()) { @@ -472,13 +475,15 @@ public static String whereClause(@Nonnull Map supportedCondit * @param filter contains field, condition and value * @param supportedConditions contains supported conditions such as EQUAL. * @param tablePrefix Table prefix append to the field name. Useful during SQL joining across multiple tables. + * @param tableName Full table name for the table referenced by tablePrefix + * @param schemaValidator schema validator for checking column/index existence * @param nonDollarVirtualColumnsEnabled whether to use dollar sign in virtual column names. * @return sql that can be appended after where clause. */ @Nonnull public static String whereClause(@Nonnull LocalRelationshipFilter filter, - @Nonnull Map supportedConditions, @Nullable String tablePrefix, - boolean nonDollarVirtualColumnsEnabled) { + @Nonnull Map supportedConditions, @Nullable String tablePrefix, @Nullable String tableName, + @Nonnull SchemaValidatorUtil schemaValidator, boolean nonDollarVirtualColumnsEnabled) { if (!LogicalExpressionLocalRelationshipCriterionUtils.filterHasNonEmptyCriteria(filter)) { throw new IllegalArgumentException("Empty filter cannot construct where clause."); } @@ -486,12 +491,12 @@ public static String whereClause(@Nonnull LocalRelationshipFilter filter, final LocalRelationshipFilter normalizedFilter = normalizeLocalRelationshipFilter(filter); return buildSQLQueryFromLogicalExpression(normalizedFilter.getLogicalExpressionCriteria(), supportedConditions, tablePrefix, - nonDollarVirtualColumnsEnabled); + tableName, schemaValidator, nonDollarVirtualColumnsEnabled); } private static String buildSQLQueryFromLogicalExpression(@Nonnull LogicalExpressionLocalRelationshipCriterion criterion, - @Nonnull Map supportedConditions, @Nullable String tablePrefix, - boolean nonDollarVirtualColumnsEnabled) { + @Nonnull Map supportedConditions, @Nullable String tablePrefix, @Nullable String tableName, + @Nonnull SchemaValidatorUtil schemaValidator, boolean nonDollarVirtualColumnsEnabled) { if (!criterion.hasExpr() || criterion.getExpr() == null) { throw new IllegalArgumentException("No logical expression found in criterion: " + criterion); } @@ -499,7 +504,7 @@ private static String buildSQLQueryFromLogicalExpression(@Nonnull LogicalExpress final LogicalExpressionLocalRelationshipCriterion.Expr expr = criterion.getExpr(); if (expr.isCriterion()) { - return buildSQLQueryFromLocalRelationshipCriterion(expr.getCriterion(), supportedConditions, tablePrefix, nonDollarVirtualColumnsEnabled); + return buildSQLQueryFromLocalRelationshipCriterion(expr.getCriterion(), supportedConditions, tablePrefix, tableName, schemaValidator, nonDollarVirtualColumnsEnabled); } // expr is logical @@ -510,7 +515,7 @@ private static String buildSQLQueryFromLogicalExpression(@Nonnull LogicalExpress if (op == Operator.NOT) { // NOT clause must only have 1 expreesion that is a criterion return "(NOT " + buildSQLQueryFromLocalRelationshipCriterion(expr.getLogical().getExpressions().get(0).getExpr().getCriterion(), - supportedConditions, tablePrefix, nonDollarVirtualColumnsEnabled) + ")"; + supportedConditions, tablePrefix, tableName, schemaValidator, nonDollarVirtualColumnsEnabled) + ")"; } final String opString = op == Operator.AND ? " AND " : " OR "; @@ -518,17 +523,17 @@ private static String buildSQLQueryFromLogicalExpression(@Nonnull LogicalExpress final LogicalExpressionLocalRelationshipCriterionArray array = logicalOperation.getExpressions(); final List subClauses = array.stream().map(c -> { - return buildSQLQueryFromLogicalExpression(c, supportedConditions, tablePrefix, nonDollarVirtualColumnsEnabled); + return buildSQLQueryFromLogicalExpression(c, supportedConditions, tablePrefix, tableName, schemaValidator, nonDollarVirtualColumnsEnabled); }).collect(Collectors.toList()); return "(" + String.join(opString, subClauses) + ")"; } private static String buildSQLQueryFromLocalRelationshipCriterion(@Nonnull LocalRelationshipCriterion criterion, - @Nonnull Map supportedConditions, @Nullable String tablePrefix, - boolean nonDollarVirtualColumnsEnabled) { + @Nonnull Map supportedConditions, @Nullable String tablePrefix, @Nullable String tableName, + @Nonnull SchemaValidatorUtil schemaValidator, boolean nonDollarVirtualColumnsEnabled) { - final String field = parseLocalRelationshipField(criterion, tablePrefix, nonDollarVirtualColumnsEnabled); + final String field = parseLocalRelationshipField(criterion, tablePrefix, tableName, schemaValidator, nonDollarVirtualColumnsEnabled); final Condition condition = criterion.getCondition(); final LocalRelationshipValue value = criterion.getValue(); @@ -588,26 +593,53 @@ public static String whereClauseOldSchema(@Nonnull Map suppor return sb.toString(); } + /** + * This is a util method that prepends a table prefix to an expression, which can either be a (Virtual) Column + * or the value of an Expression Index. In the latter case, we have to replace the table prefix in the expression + * as opposed to a simple prepend operation. + * + * @param tablePrefix table prefix, is expected to have a delimiter already appended + * @param expression expression + * @param expectedVirtualColumnName expected virtual column name + * @param originColumnName the column name in which the indexed field is derived / extracted + * @return expression with table prefix + */ @VisibleForTesting @Nonnull - protected static String addTablePrefixToExpression(@Nonnull String expression, @Nonnull String tablePrefix) { - if (tablePrefix == null || tablePrefix.isEmpty()) { + protected static String addTablePrefixToExpression(@Nonnull String tablePrefix, + @Nonnull String expression, + @Nonnull String expectedVirtualColumnName, + @Nonnull String originColumnName) { + if (tablePrefix.isEmpty()) { return expression; } - // Replace column references: `columnName` -> `tablePrefix`.`columnName` - // This is a simplified example - would need more robust parsing - return expression.replaceAll("`(a_\\w+)`", "`" + tablePrefix + "`.`$1`"); - } - - @VisibleForTesting - protected static String handleRelationshipField() { + // If the expression is the same as the expected virtual column name, we can just prepend the table prefix + // This is the case where, in evaluating "expression or column", the function returned a column (the expected column) + if (expression.equals(expectedVirtualColumnName)) { + return tablePrefix + expression; + } + // This means that an expression index is being used. In this case, we need to prepend the prefix by injecting it + // into the string at the right location. + // An example of this would be: + // (cast(json_extract(`a_aspectbar`, '$.aspect.value') as char(1024))) + // --> (cast(json_extract(`PREFIX`.`a_aspectbar`, '$.aspect.value') as char(1024))) + // Note that in this example, there are backtick marks (`) surrounding the column name. This is expected because + // of how index value extraction works. However, we should also prepare for the use case where there are NO backticks + // around the column name just to be extra safe. + // In this way, we could also have a case like: + // (cast(json_extract(a_aspectbar, '$.aspect.value') as char(1024))) + // --> (cast(json_extract(`PREFIX`.a_aspectbar, '$.aspect.value') as char(1024))) + // Note that for good syntactic practice, we will surround the table prefix with backticks no matter what. + + // So what we want to do is look for the originColumnName then inject the table prefix before it. + return expression.replaceAll("(`?" + originColumnName + "`?)", "`" + tablePrefix + "`.`$1`"); } private static String parseLocalRelationshipField( @Nonnull final LocalRelationshipCriterion localRelationshipCriterion, @Nullable String tablePrefix, - boolean nonDollarVirtualColumnsEnabled) { + @Nonnull String tableName, @Nonnull SchemaValidatorUtil schemaValidator, boolean nonDollarVirtualColumnsEnabled) { tablePrefix = tablePrefix == null ? "" : tablePrefix + "."; LocalRelationshipCriterion.Field field = localRelationshipCriterion.getField(); char delimiter = nonDollarVirtualColumnsEnabled ? '0' : '$'; @@ -620,20 +652,32 @@ private static String parseLocalRelationshipField( // RelationshipField.pdl defines RelationshipField.name as 'metadata' // --> virtual column use case that needs to be functionalized if (field.isRelationshipField()) { - final String expectedVirtualColumnName = - tablePrefix + field.getRelationshipField().getName() + processPath(field.getRelationshipField().getPath(), delimiter); - return SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( + final String expectedVirtualColumnName = field.getRelationshipField().getName() + processPath(field.getRelationshipField().getPath(), delimiter); + final String indexedExpressionOrColumn = SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( expectedVirtualColumnName, field.getRelationshipField().getPath(), tableName, schemaValidator); + if (indexedExpressionOrColumn == null) { + throw new IllegalArgumentException("Neither expression nor column index not found for relationship field: " + expectedVirtualColumnName); + } + return addTablePrefixToExpression(tablePrefix, indexedExpressionOrColumn, expectedVirtualColumnName, RELATIONSHIP_TABLE_EXPRESSION_INDEX_INFIX); } // This appears to be when a join has already occurred and this is some indexed field from an aspect column from // the entity table(s) --> virtual column use case that needs to be functionalized if (field.isAspectField()) { - // entity type from Urn definition. String assetType = getAssetType(field.getAspectField()); - return tablePrefix + getGeneratedColumnName(assetType, field.getAspectField().getAspect(), - field.getAspectField().getPath(), nonDollarVirtualColumnsEnabled); + final String indexedExpressionOrColumn = + SQLIndexFilterUtils.getIndexedExpressionOrColumn( + assetType, field.getAspectField().getAspect(), field.getAspectField().getPath(), + nonDollarVirtualColumnsEnabled, schemaValidator); + if (indexedExpressionOrColumn == null) { + throw new IllegalArgumentException("Neither expression nor column index not found for aspect field: " + assetType + + "." + field.getAspectField().getAspect() + "." + field.getAspectField().getPath()); + } + final String expectedVirtualColumnName = SQLSchemaUtils.getGeneratedColumnName( + assetType, field.getAspectField().getAspect(), field.getAspectField().getPath(), nonDollarVirtualColumnsEnabled); + return addTablePrefixToExpression(tablePrefix, indexedExpressionOrColumn, expectedVirtualColumnName, + SQLSchemaUtils.getAspectColumnName(assetType, field.getAspectField().getAspect())); } throw new IllegalArgumentException("Unrecognized field type"); diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java index 6f9d8a9ab..41c91208c 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java @@ -249,8 +249,8 @@ public void testWhereClauseSingleCondition() { .setValue(LocalRelationshipValue.create("value1")); LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, false), "urn='value1'"); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, true), "urn='value1'"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), "urn='value1'"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), "urn='value1'"); } @Test @@ -264,8 +264,8 @@ public void testWhereClauseSingleINCondition() { .setValue(LocalRelationshipValue.create(values)); LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.IN, "IN"), null, false), "urn IN ('value1')"); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.IN, "IN"), null, true), "urn IN ('value1')"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.IN, "IN"), null, null, mockValidator, false), "urn IN ('value1')"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.IN, "IN"), null, null, mockValidator, true), "urn IN ('value1')"); } @Test @@ -278,7 +278,7 @@ public void testWhereClauseSingleStartWithCondition() { .setValue(LocalRelationshipValue.create("value1")); LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.START_WITH, "LIKE"), null, false), "urn LIKE 'value1%'"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.START_WITH, "LIKE"), null, null, mockValidator, false), "urn LIKE 'value1%'"); } @Test @@ -299,8 +299,8 @@ public void testWhereClauseMultiConditionSameName() { LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion1, criterion2); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, false), "(urn='value1' OR urn='value2')"); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, true), "(urn='value1' OR urn='value2')"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), "(urn='value1' OR urn='value2')"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), "(urn='value1' OR urn='value2')"); } @Test @@ -321,9 +321,9 @@ public void testWhereClauseMultiConditionDifferentName() { LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion1, criterion2); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, false), + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), "(i_aspectfoo$value='value2' AND urn='value1')"); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, true), + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), "(i_aspectfoo0value='value2' AND urn='value1')"); } @@ -362,10 +362,10 @@ public void testWhereClauseMultiConditionMixedName() { LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion1, criterion2, criterion3, criterion4); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, false), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), "(urn='value1' OR urn='value3') AND metadata$value='value4' AND i_aspectfoo$value='value2'"); - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, true), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), "(urn='value1' OR urn='value3') AND metadata0value='value4' AND i_aspectfoo0value='value2'"); } @@ -421,13 +421,13 @@ public void testWhereClauseMultiFilters() { LocalRelationshipFilter filter2 = new LocalRelationshipFilter().setCriteria(criteria2); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, new Pair<>(filter1, "foo"), + assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, null, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), "(foo.i_aspectfoo$value='value2' AND (foo.urn='value1' OR foo.urn='value3') " + "AND foo.metadata$value='value4') AND (bar.urn='value1' OR bar.urn='value2')" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, new Pair<>(filter1, "foo"), + assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, null, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), "(foo.i_aspectfoo0value='value2' AND (foo.urn='value1' OR foo.urn='value3') " + "AND foo.metadata0value='value4') AND (bar.urn='value1' OR bar.urn='value2')" ); @@ -484,13 +484,13 @@ public void testWhereClauseMultiFilters2() { LocalRelationshipFilter filter2 = new LocalRelationshipFilter().setCriteria(criteria2); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, new Pair<>(filter1, "foo"), + assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, null, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), "(foo.i_aspectfoo$value LIKE 'value2%' AND (foo.urn LIKE 'value1%' OR foo.urn LIKE 'value3%') " + "AND foo.metadata$value LIKE 'value4%') AND (bar.urn LIKE 'value1%' OR bar.urn LIKE 'value2%')" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, new Pair<>(filter1, "foo"), + assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, null, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), "(foo.i_aspectfoo0value LIKE 'value2%' AND (foo.urn LIKE 'value1%' OR foo.urn LIKE 'value3%') " + "AND foo.metadata0value LIKE 'value4%') AND (bar.urn LIKE 'value1%' OR bar.urn LIKE 'value2%')" ); @@ -557,12 +557,12 @@ public void testWhereClauseWithLogicalExpression() { LocalRelationshipFilter filter = new LocalRelationshipFilter().setLogicalExpressionCriteria(root); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, false), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), "((urn='foo1' OR urn='foo2') AND i_aspectfoo$value='bar')" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, true), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), "((urn='foo1' OR urn='foo2') AND i_aspectfoo0value='bar')" ); } @@ -576,12 +576,12 @@ public void testWhereClauseWithLogicalExpressionWithNot() { LocalRelationshipFilter filter = new LocalRelationshipFilter().setLogicalExpressionCriteria(notNode); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, false), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), "(NOT i_aspectfoo$value='bar')" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, true), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), "(NOT i_aspectfoo0value='bar')" ); } @@ -608,12 +608,12 @@ public void testWhereClauseWithLogicalExpressionWithNotNested() { LocalRelationshipFilter filter = new LocalRelationshipFilter().setLogicalExpressionCriteria(root); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, false), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), "((urn='foo1' OR urn='foo2') AND (NOT i_aspectfoo$value='bar'))" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, true), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), "((urn='foo1' OR urn='foo2') AND (NOT i_aspectfoo0value='bar'))" ); } @@ -1141,7 +1141,7 @@ public void testWhereClauseCompleteInjectionScenario() { Map supportedConditions = new HashMap<>(); supportedConditions.put(Condition.EQUAL, "="); - String whereClause = SQLStatementUtils.whereClause(filter, supportedConditions, "rt", false); + String whereClause = SQLStatementUtils.whereClause(filter, supportedConditions, "rt", null, mockValidator, false); // Expect all quotes escaped assertEquals(whereClause, "rt.destination='urn:li:dataset:(urn:li:dataPlatform:hdfs,/data'') OR 1=1 OR destination LIKE ''%'"); @@ -1184,7 +1184,7 @@ public void testBuildSQLQueryFromLocalRelationshipCriterionWithInjection() { Map supportedConditions = new HashMap<>(); supportedConditions.put(Condition.START_WITH, "LIKE"); - String whereClause = SQLStatementUtils.whereClause(filter, supportedConditions, null, false); + String whereClause = SQLStatementUtils.whereClause(filter, supportedConditions, null, null, mockValidator, false); // Should properly escape and add the wildcard assertEquals(whereClause, "destination LIKE 'urn:li:dataset:prefix''%%'"); From f9649d1ac90b9610b7b019375bd2583f1973207e Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Fri, 21 Nov 2025 00:25:27 -0800 Subject: [PATCH 03/18] nits --- .../linkedin/metadata/dao/utils/SQLStatementUtils.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index 58b876e72..9b79f9752 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -458,7 +458,9 @@ public static String whereClause(@Nonnull Map supportedCondit List andClauses = new ArrayList<>(); for (Pair filter : filters) { if (LogicalExpressionLocalRelationshipCriterionUtils.filterHasNonEmptyCriteria(filter.getValue0())) { - andClauses.add("(" + whereClause(filter.getValue0(), supportedConditions, filter.getValue1(), relationshipTableName, schemaValidator, nonDollarVirtualColumnsEnabled) + ")"); + andClauses.add("(" + whereClause( + filter.getValue0(), supportedConditions, filter.getValue1(), relationshipTableName, + schemaValidator, nonDollarVirtualColumnsEnabled) + ")"); } } if (andClauses.isEmpty()) { @@ -504,7 +506,8 @@ private static String buildSQLQueryFromLogicalExpression(@Nonnull LogicalExpress final LogicalExpressionLocalRelationshipCriterion.Expr expr = criterion.getExpr(); if (expr.isCriterion()) { - return buildSQLQueryFromLocalRelationshipCriterion(expr.getCriterion(), supportedConditions, tablePrefix, tableName, schemaValidator, nonDollarVirtualColumnsEnabled); + return buildSQLQueryFromLocalRelationshipCriterion( + expr.getCriterion(), supportedConditions, tablePrefix, tableName, schemaValidator, nonDollarVirtualColumnsEnabled); } // expr is logical @@ -637,7 +640,8 @@ protected static String addTablePrefixToExpression(@Nonnull String tablePrefix, return expression.replaceAll("(`?" + originColumnName + "`?)", "`" + tablePrefix + "`.`$1`"); } - private static String parseLocalRelationshipField( + @VisibleForTesting + protected static String parseLocalRelationshipField( @Nonnull final LocalRelationshipCriterion localRelationshipCriterion, @Nullable String tablePrefix, @Nonnull String tableName, @Nonnull SchemaValidatorUtil schemaValidator, boolean nonDollarVirtualColumnsEnabled) { tablePrefix = tablePrefix == null ? "" : tablePrefix + "."; From 175992bc2903cbc1f244a94ef4e25b80cf5e49a5 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Fri, 21 Nov 2025 00:37:17 -0800 Subject: [PATCH 04/18] nits --- .../metadata/dao/utils/SQLIndexFilterUtilsTest.java | 13 +++++++++++++ .../metadata/dao/utils/SQLSchemaUtilsTest.java | 7 +++++++ .../metadata/dao/utils/SQLStatementUtilsTest.java | 12 +++++++++--- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java index fcf847dd4..47a7e1cce 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java @@ -45,6 +45,14 @@ public void setupValidator() { // This is an existing legacy way of array extraction, casting to a string (DataPolicyInfo.annotation.ontologyIris) when(mockValidator.getIndexExpression(anyString(), matches("e_aspectbar0annotation0ontologyIris"))) .thenReturn("(cast(replace(json_unquote(json_extract(`a_aspectbar`,'$.aspect.annotation.ontologyIris[*]')),'\"','') as char(255)))"); + + // New mocks for relationship field validation + when(mockValidator.getIndexExpression(anyString(), matches("e_aspectbar0destination"))) + .thenReturn("destination"); + when(mockValidator.getIndexExpression(anyString(), matches("e_aspectbar0destination_array"))) + .thenReturn("destination_array"); + when(mockValidator.getIndexExpression(anyString(), matches("e_aspectbar0destination_ontologyIris"))) + .thenReturn("destination_ontologyIris"); } @Test @@ -240,4 +248,9 @@ public void testGetIndexedExpressionOrColumn() { false, mockValidator), "(cast(json_extract(`a_aspectbar`, '$.aspect.value') as char(1024)))"); } + + @Test + public void testGetIndexedExpressionOrColumnRelationship() { + + } } \ No newline at end of file diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLSchemaUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLSchemaUtilsTest.java index 5826bdbc3..fba0ff561 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLSchemaUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLSchemaUtilsTest.java @@ -41,4 +41,11 @@ public void testGetExpressionIndexName() { "e_aspectfoo0value0fooBar"); } + @Test + public void testGetExpressionIndexNameRelationship() { + assertEquals( + SQLSchemaUtils.getExpressionIndexNameRelationship("/value/blue"), + "e_metadata0bvalue0blue"); + } + } \ No newline at end of file diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java index 41c91208c..b7feb9c37 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java @@ -278,7 +278,9 @@ public void testWhereClauseSingleStartWithCondition() { .setValue(LocalRelationshipValue.create("value1")); LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.START_WITH, "LIKE"), null, null, mockValidator, false), "urn LIKE 'value1%'"); + assertEquals(SQLStatementUtils.whereClause( + filter, Collections.singletonMap(Condition.START_WITH, "LIKE"), null, null, + mockValidator, false), "urn LIKE 'value1%'"); } @Test @@ -299,8 +301,12 @@ public void testWhereClauseMultiConditionSameName() { LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion1, criterion2); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), "(urn='value1' OR urn='value2')"); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), "(urn='value1' OR urn='value2')"); + assertEquals(SQLStatementUtils.whereClause( + filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, + mockValidator, false), "(urn='value1' OR urn='value2')"); + assertEquals(SQLStatementUtils.whereClause( + filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, + mockValidator, true), "(urn='value1' OR urn='value2')"); } @Test From c04b9b16cc3f933584a00df6836b9e97b1413619 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Fri, 21 Nov 2025 01:00:07 -0800 Subject: [PATCH 05/18] add testing --- .../metadata/dao/utils/SQLSchemaUtils.java | 2 +- .../metadata/dao/utils/SQLStatementUtils.java | 6 ++++-- .../dao/utils/SQLIndexFilterUtilsTest.java | 18 ++++++++++++------ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLSchemaUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLSchemaUtils.java index b5732acf8..343d68013 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLSchemaUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLSchemaUtils.java @@ -204,7 +204,7 @@ public static String getExpressionIndexName(@Nonnull String assetType, @Nonnull /** * Get the expected expression index name for a relationship table given the path. - * With the expression index changes, we establish an expected naming as follows... + * With the expression index changes, we **establish** an **expected** index naming as follows... * ex. e_metadata0foo0bar */ @Nonnull diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index 9b79f9752..ea513a579 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -648,14 +648,16 @@ protected static String parseLocalRelationshipField( LocalRelationshipCriterion.Field field = localRelationshipCriterion.getField(); char delimiter = nonDollarVirtualColumnsEnabled ? '0' : '$'; - // UrnField.pdl defines UrnField.name as 'urn' --> real column + // UrnField.pdl defines UrnField.name as 'urn' + // --> real column (not a virtual one), no need to "functionalize" if (field.isUrnField()) { return tablePrefix + field.getUrnField().getName(); } - // RelationshipField.pdl defines RelationshipField.name as 'metadata' + // RelationshipField.pdl defines RelationshipField.name as 'metadata' -- ie. this is how "metadata$foo$bar" column is formed // --> virtual column use case that needs to be functionalized if (field.isRelationshipField()) { + // This next line is basically the original "expected naming logic" verbatim, keeping here for consistency final String expectedVirtualColumnName = field.getRelationshipField().getName() + processPath(field.getRelationshipField().getPath(), delimiter); final String indexedExpressionOrColumn = SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( expectedVirtualColumnName, field.getRelationshipField().getPath(), diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java index 47a7e1cce..7e98cbb89 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java @@ -47,12 +47,8 @@ public void setupValidator() { .thenReturn("(cast(replace(json_unquote(json_extract(`a_aspectbar`,'$.aspect.annotation.ontologyIris[*]')),'\"','') as char(255)))"); // New mocks for relationship field validation - when(mockValidator.getIndexExpression(anyString(), matches("e_aspectbar0destination"))) - .thenReturn("destination"); - when(mockValidator.getIndexExpression(anyString(), matches("e_aspectbar0destination_array"))) - .thenReturn("destination_array"); - when(mockValidator.getIndexExpression(anyString(), matches("e_aspectbar0destination_ontologyIris"))) - .thenReturn("destination_ontologyIris"); + when(mockValidator.getIndexExpression(anyString(), matches("e_metadata0field"))) + .thenReturn("(cast(json_extract(`metadata`, '$.field') as char(64)))"); } @Test @@ -251,6 +247,16 @@ public void testGetIndexedExpressionOrColumn() { @Test public void testGetIndexedExpressionOrColumnRelationship() { + // Get something that is NOT an expression (not mocked) -- delimiters are not relevant here because this logic is deferred + // to the caller + assertEquals(SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( + "metadata$foofield", "/foofield", "tablenamedoesntmatterherebecauseofmock", mockValidator), + "metadata$foofield"); + // Get something that is an expression (mocked) + assertEquals(SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( + "metadata$field", "/field", "tablenamedoesntmatterherebecauseofmock", mockValidator), + "(cast(json_extract(`metadata`, '$.field') as char(64)))"); } + } \ No newline at end of file From 325eb6940934f08780cce1020b360ae1c538ab46 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Fri, 21 Nov 2025 01:02:50 -0800 Subject: [PATCH 06/18] typo --- .../com/linkedin/metadata/dao/utils/SQLSchemaUtilsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLSchemaUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLSchemaUtilsTest.java index fba0ff561..43b1596de 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLSchemaUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLSchemaUtilsTest.java @@ -45,7 +45,7 @@ public void testGetExpressionIndexName() { public void testGetExpressionIndexNameRelationship() { assertEquals( SQLSchemaUtils.getExpressionIndexNameRelationship("/value/blue"), - "e_metadata0bvalue0blue"); + "e_metadata0value0blue"); } } \ No newline at end of file From 98fb1c47c9e39395643664aef1f61f01f724492f Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Fri, 21 Nov 2025 01:12:39 -0800 Subject: [PATCH 07/18] fixes some tests --- .../metadata/dao/utils/SQLStatementUtils.java | 4 +- .../dao/utils/SQLStatementUtilsTest.java | 45 ++++++++++--------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index ea513a579..2d5a185f5 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -643,7 +643,7 @@ protected static String addTablePrefixToExpression(@Nonnull String tablePrefix, @VisibleForTesting protected static String parseLocalRelationshipField( @Nonnull final LocalRelationshipCriterion localRelationshipCriterion, @Nullable String tablePrefix, - @Nonnull String tableName, @Nonnull SchemaValidatorUtil schemaValidator, boolean nonDollarVirtualColumnsEnabled) { + @Nonnull String relationshipTableName, @Nonnull SchemaValidatorUtil schemaValidator, boolean nonDollarVirtualColumnsEnabled) { tablePrefix = tablePrefix == null ? "" : tablePrefix + "."; LocalRelationshipCriterion.Field field = localRelationshipCriterion.getField(); char delimiter = nonDollarVirtualColumnsEnabled ? '0' : '$'; @@ -661,7 +661,7 @@ protected static String parseLocalRelationshipField( final String expectedVirtualColumnName = field.getRelationshipField().getName() + processPath(field.getRelationshipField().getPath(), delimiter); final String indexedExpressionOrColumn = SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( expectedVirtualColumnName, field.getRelationshipField().getPath(), - tableName, schemaValidator); + relationshipTableName, schemaValidator); if (indexedExpressionOrColumn == null) { throw new IllegalArgumentException("Neither expression nor column index not found for relationship field: " + expectedVirtualColumnName); } diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java index b7feb9c37..61bfca017 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java @@ -49,6 +49,11 @@ public class SQLStatementUtilsTest { private static SchemaValidatorUtil mockValidator; + // This is (dummy) table name placeholder for testing, introduced because we need to propagate the relationship table + // name for relationship table functional index processing. However, for existing tests that simply don't test for + // relationship table functional index processing, we can just use a dummy table name. + private static final String PLACEHOLDER_TABLE_NAME = "placeholder"; + @BeforeClass public void setupValidator() { mockValidator = mock(SchemaValidatorUtil.class); @@ -249,8 +254,8 @@ public void testWhereClauseSingleCondition() { .setValue(LocalRelationshipValue.create("value1")); LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), "urn='value1'"); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), "urn='value1'"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, false), "urn='value1'"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, true), "urn='value1'"); } @Test @@ -264,8 +269,8 @@ public void testWhereClauseSingleINCondition() { .setValue(LocalRelationshipValue.create(values)); LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.IN, "IN"), null, null, mockValidator, false), "urn IN ('value1')"); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.IN, "IN"), null, null, mockValidator, true), "urn IN ('value1')"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.IN, "IN"), null, PLACEHOLDER_TABLE_NAME, mockValidator, false), "urn IN ('value1')"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.IN, "IN"), null, PLACEHOLDER_TABLE_NAME, mockValidator, true), "urn IN ('value1')"); } @Test @@ -327,9 +332,9 @@ public void testWhereClauseMultiConditionDifferentName() { LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion1, criterion2); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, false), "(i_aspectfoo$value='value2' AND urn='value1')"); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, true), "(i_aspectfoo0value='value2' AND urn='value1')"); } @@ -368,10 +373,10 @@ public void testWhereClauseMultiConditionMixedName() { LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion1, criterion2, criterion3, criterion4); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, false), "(urn='value1' OR urn='value3') AND metadata$value='value4' AND i_aspectfoo$value='value2'"); - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, true), "(urn='value1' OR urn='value3') AND metadata0value='value4' AND i_aspectfoo0value='value2'"); } @@ -427,13 +432,13 @@ public void testWhereClauseMultiFilters() { LocalRelationshipFilter filter2 = new LocalRelationshipFilter().setCriteria(criteria2); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, null, mockValidator, new Pair<>(filter1, "foo"), + assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), "(foo.i_aspectfoo$value='value2' AND (foo.urn='value1' OR foo.urn='value3') " + "AND foo.metadata$value='value4') AND (bar.urn='value1' OR bar.urn='value2')" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, null, mockValidator, new Pair<>(filter1, "foo"), + assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), "(foo.i_aspectfoo0value='value2' AND (foo.urn='value1' OR foo.urn='value3') " + "AND foo.metadata0value='value4') AND (bar.urn='value1' OR bar.urn='value2')" ); @@ -490,13 +495,13 @@ public void testWhereClauseMultiFilters2() { LocalRelationshipFilter filter2 = new LocalRelationshipFilter().setCriteria(criteria2); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, null, mockValidator, new Pair<>(filter1, "foo"), + assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), "(foo.i_aspectfoo$value LIKE 'value2%' AND (foo.urn LIKE 'value1%' OR foo.urn LIKE 'value3%') " + "AND foo.metadata$value LIKE 'value4%') AND (bar.urn LIKE 'value1%' OR bar.urn LIKE 'value2%')" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, null, mockValidator, new Pair<>(filter1, "foo"), + assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), "(foo.i_aspectfoo0value LIKE 'value2%' AND (foo.urn LIKE 'value1%' OR foo.urn LIKE 'value3%') " + "AND foo.metadata0value LIKE 'value4%') AND (bar.urn LIKE 'value1%' OR bar.urn LIKE 'value2%')" ); @@ -563,12 +568,12 @@ public void testWhereClauseWithLogicalExpression() { LocalRelationshipFilter filter = new LocalRelationshipFilter().setLogicalExpressionCriteria(root); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, false), "((urn='foo1' OR urn='foo2') AND i_aspectfoo$value='bar')" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, true), "((urn='foo1' OR urn='foo2') AND i_aspectfoo0value='bar')" ); } @@ -582,12 +587,12 @@ public void testWhereClauseWithLogicalExpressionWithNot() { LocalRelationshipFilter filter = new LocalRelationshipFilter().setLogicalExpressionCriteria(notNode); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, false), "(NOT i_aspectfoo$value='bar')" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, true), "(NOT i_aspectfoo0value='bar')" ); } @@ -614,12 +619,12 @@ public void testWhereClauseWithLogicalExpressionWithNotNested() { LocalRelationshipFilter filter = new LocalRelationshipFilter().setLogicalExpressionCriteria(root); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, false), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, false), "((urn='foo1' OR urn='foo2') AND (NOT i_aspectfoo$value='bar'))" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, null, mockValidator, true), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, true), "((urn='foo1' OR urn='foo2') AND (NOT i_aspectfoo0value='bar'))" ); } @@ -1147,7 +1152,7 @@ public void testWhereClauseCompleteInjectionScenario() { Map supportedConditions = new HashMap<>(); supportedConditions.put(Condition.EQUAL, "="); - String whereClause = SQLStatementUtils.whereClause(filter, supportedConditions, "rt", null, mockValidator, false); + String whereClause = SQLStatementUtils.whereClause(filter, supportedConditions, "rt", PLACEHOLDER_TABLE_NAME, mockValidator, false); // Expect all quotes escaped assertEquals(whereClause, "rt.destination='urn:li:dataset:(urn:li:dataPlatform:hdfs,/data'') OR 1=1 OR destination LIKE ''%'"); @@ -1190,7 +1195,7 @@ public void testBuildSQLQueryFromLocalRelationshipCriterionWithInjection() { Map supportedConditions = new HashMap<>(); supportedConditions.put(Condition.START_WITH, "LIKE"); - String whereClause = SQLStatementUtils.whereClause(filter, supportedConditions, null, null, mockValidator, false); + String whereClause = SQLStatementUtils.whereClause(filter, supportedConditions, null, PLACEHOLDER_TABLE_NAME, mockValidator, false); // Should properly escape and add the wildcard assertEquals(whereClause, "destination LIKE 'urn:li:dataset:prefix''%%'"); From 7e0e347f9afd908b02127742bf0fcf6b97bfc8b0 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Fri, 21 Nov 2025 01:20:01 -0800 Subject: [PATCH 08/18] line lengths --- .../dao/utils/SQLStatementUtilsTest.java | 56 ++++++++++++------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java index 61bfca017..3c994f739 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java @@ -254,8 +254,10 @@ public void testWhereClauseSingleCondition() { .setValue(LocalRelationshipValue.create("value1")); LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, false), "urn='value1'"); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, true), "urn='value1'"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, + PLACEHOLDER_TABLE_NAME, mockValidator, false), "urn='value1'"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, + PLACEHOLDER_TABLE_NAME, mockValidator, true), "urn='value1'"); } @Test @@ -269,8 +271,10 @@ public void testWhereClauseSingleINCondition() { .setValue(LocalRelationshipValue.create(values)); LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.IN, "IN"), null, PLACEHOLDER_TABLE_NAME, mockValidator, false), "urn IN ('value1')"); - assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.IN, "IN"), null, PLACEHOLDER_TABLE_NAME, mockValidator, true), "urn IN ('value1')"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.IN, "IN"), null, + PLACEHOLDER_TABLE_NAME, mockValidator, false), "urn IN ('value1')"); + assertEquals(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.IN, "IN"), null, + PLACEHOLDER_TABLE_NAME, mockValidator, true), "urn IN ('value1')"); } @Test @@ -373,10 +377,12 @@ public void testWhereClauseMultiConditionMixedName() { LocalRelationshipCriterionArray criteria = new LocalRelationshipCriterionArray(criterion1, criterion2, criterion3, criterion4); LocalRelationshipFilter filter = new LocalRelationshipFilter().setCriteria(criteria); - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, false), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), + null, PLACEHOLDER_TABLE_NAME, mockValidator, false), "(urn='value1' OR urn='value3') AND metadata$value='value4' AND i_aspectfoo$value='value2'"); - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, true), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), + null, PLACEHOLDER_TABLE_NAME, mockValidator, true), "(urn='value1' OR urn='value3') AND metadata0value='value4' AND i_aspectfoo0value='value2'"); } @@ -432,14 +438,16 @@ public void testWhereClauseMultiFilters() { LocalRelationshipFilter filter2 = new LocalRelationshipFilter().setCriteria(criteria2); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), - new Pair<>(filter2, "bar")), "(foo.i_aspectfoo$value='value2' AND (foo.urn='value1' OR foo.urn='value3') " + assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, + PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), + "(foo.i_aspectfoo$value='value2' AND (foo.urn='value1' OR foo.urn='value3') " + "AND foo.metadata$value='value4') AND (bar.urn='value1' OR bar.urn='value2')" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), - new Pair<>(filter2, "bar")), "(foo.i_aspectfoo0value='value2' AND (foo.urn='value1' OR foo.urn='value3') " + assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, + PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), + "(foo.i_aspectfoo0value='value2' AND (foo.urn='value1' OR foo.urn='value3') " + "AND foo.metadata0value='value4') AND (bar.urn='value1' OR bar.urn='value2')" ); } @@ -495,14 +503,16 @@ public void testWhereClauseMultiFilters2() { LocalRelationshipFilter filter2 = new LocalRelationshipFilter().setCriteria(criteria2); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), - new Pair<>(filter2, "bar")), "(foo.i_aspectfoo$value LIKE 'value2%' AND (foo.urn LIKE 'value1%' OR foo.urn LIKE 'value3%') " + assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, + PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), + "(foo.i_aspectfoo$value LIKE 'value2%' AND (foo.urn LIKE 'value1%' OR foo.urn LIKE 'value3%') " + "AND foo.metadata$value LIKE 'value4%') AND (bar.urn LIKE 'value1%' OR bar.urn LIKE 'value2%')" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), - new Pair<>(filter2, "bar")), "(foo.i_aspectfoo0value LIKE 'value2%' AND (foo.urn LIKE 'value1%' OR foo.urn LIKE 'value3%') " + assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, + PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), + "(foo.i_aspectfoo0value LIKE 'value2%' AND (foo.urn LIKE 'value1%' OR foo.urn LIKE 'value3%') " + "AND foo.metadata0value LIKE 'value4%') AND (bar.urn LIKE 'value1%' OR bar.urn LIKE 'value2%')" ); } @@ -568,12 +578,14 @@ public void testWhereClauseWithLogicalExpression() { LocalRelationshipFilter filter = new LocalRelationshipFilter().setLogicalExpressionCriteria(root); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, false), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), + null, PLACEHOLDER_TABLE_NAME, mockValidator, false), "((urn='foo1' OR urn='foo2') AND i_aspectfoo$value='bar')" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, true), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), + null, PLACEHOLDER_TABLE_NAME, mockValidator, true), "((urn='foo1' OR urn='foo2') AND i_aspectfoo0value='bar')" ); } @@ -587,12 +599,14 @@ public void testWhereClauseWithLogicalExpressionWithNot() { LocalRelationshipFilter filter = new LocalRelationshipFilter().setLogicalExpressionCriteria(notNode); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, false), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), + null, PLACEHOLDER_TABLE_NAME, mockValidator, false), "(NOT i_aspectfoo$value='bar')" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, true), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), + null, PLACEHOLDER_TABLE_NAME, mockValidator, true), "(NOT i_aspectfoo0value='bar')" ); } @@ -619,12 +633,14 @@ public void testWhereClauseWithLogicalExpressionWithNotNested() { LocalRelationshipFilter filter = new LocalRelationshipFilter().setLogicalExpressionCriteria(root); //test for multi filters with dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, false), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), + null, PLACEHOLDER_TABLE_NAME, mockValidator, false), "((urn='foo1' OR urn='foo2') AND (NOT i_aspectfoo$value='bar'))" ); //test for multi filters with non dollar virtual columns names - assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), null, PLACEHOLDER_TABLE_NAME, mockValidator, true), + assertConditionsEqual(SQLStatementUtils.whereClause(filter, Collections.singletonMap(Condition.EQUAL, "="), + null, PLACEHOLDER_TABLE_NAME, mockValidator, true), "((urn='foo1' OR urn='foo2') AND (NOT i_aspectfoo0value='bar'))" ); } From 7d4f1fe80156df2123d034f33b38ac23819d8109 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Fri, 21 Nov 2025 01:39:30 -0800 Subject: [PATCH 09/18] short name --- .../metadata/dao/utils/SQLStatementUtils.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index 2d5a185f5..a6ce2def7 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -671,17 +671,20 @@ protected static String parseLocalRelationshipField( // This appears to be when a join has already occurred and this is some indexed field from an aspect column from // the entity table(s) --> virtual column use case that needs to be functionalized if (field.isAspectField()) { + final String aspectFqcn = field.getAspectField().getAspect(); + final String aspectSimpleName = aspectFqcn.substring(aspectFqcn.lastIndexOf('.') + 1); String assetType = getAssetType(field.getAspectField()); final String indexedExpressionOrColumn = SQLIndexFilterUtils.getIndexedExpressionOrColumn( - assetType, field.getAspectField().getAspect(), field.getAspectField().getPath(), + assetType, aspectSimpleName, field.getAspectField().getPath(), nonDollarVirtualColumnsEnabled, schemaValidator); if (indexedExpressionOrColumn == null) { - throw new IllegalArgumentException("Neither expression nor column index not found for aspect field: " + assetType - + "." + field.getAspectField().getAspect() + "." + field.getAspectField().getPath()); + throw new IllegalArgumentException( + String.format("Neither expression nor column index not found for aspect field: Asset: %s, Aspect: %s, Path: %s", + assetType, aspectFqcn, field.getAspectField().getPath())); } final String expectedVirtualColumnName = SQLSchemaUtils.getGeneratedColumnName( - assetType, field.getAspectField().getAspect(), field.getAspectField().getPath(), nonDollarVirtualColumnsEnabled); + assetType, aspectFqcn, field.getAspectField().getPath(), nonDollarVirtualColumnsEnabled); return addTablePrefixToExpression(tablePrefix, indexedExpressionOrColumn, expectedVirtualColumnName, SQLSchemaUtils.getAspectColumnName(assetType, field.getAspectField().getAspect())); } From 5f8d0c1e8f7c40050e6b504b01c025860fec19e0 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Fri, 21 Nov 2025 01:45:05 -0800 Subject: [PATCH 10/18] formatting --- .../metadata/dao/utils/SQLStatementUtils.java | 17 +++++++++-------- .../dao/utils/SQLStatementUtilsTest.java | 4 ++++ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index a6ce2def7..2a3629ca4 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -673,20 +673,21 @@ protected static String parseLocalRelationshipField( if (field.isAspectField()) { final String aspectFqcn = field.getAspectField().getAspect(); final String aspectSimpleName = aspectFqcn.substring(aspectFqcn.lastIndexOf('.') + 1); - String assetType = getAssetType(field.getAspectField()); + final String path = field.getAspectField().getPath(); + + final String assetType = getAssetType(field.getAspectField()); + final String indexedExpressionOrColumn = - SQLIndexFilterUtils.getIndexedExpressionOrColumn( - assetType, aspectSimpleName, field.getAspectField().getPath(), - nonDollarVirtualColumnsEnabled, schemaValidator); + SQLIndexFilterUtils.getIndexedExpressionOrColumn(assetType, aspectSimpleName, path, nonDollarVirtualColumnsEnabled, schemaValidator); if (indexedExpressionOrColumn == null) { throw new IllegalArgumentException( String.format("Neither expression nor column index not found for aspect field: Asset: %s, Aspect: %s, Path: %s", - assetType, aspectFqcn, field.getAspectField().getPath())); + assetType, aspectFqcn, path)); } - final String expectedVirtualColumnName = SQLSchemaUtils.getGeneratedColumnName( - assetType, aspectFqcn, field.getAspectField().getPath(), nonDollarVirtualColumnsEnabled); + final String expectedVirtualColumnName = + SQLSchemaUtils.getGeneratedColumnName(assetType, aspectFqcn, path, nonDollarVirtualColumnsEnabled); return addTablePrefixToExpression(tablePrefix, indexedExpressionOrColumn, expectedVirtualColumnName, - SQLSchemaUtils.getAspectColumnName(assetType, field.getAspectField().getAspect())); + SQLSchemaUtils.getAspectColumnName(assetType, aspectFqcn)); } throw new IllegalArgumentException("Unrecognized field type"); diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java index 3c994f739..277bd4a68 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java @@ -73,6 +73,10 @@ public void setupValidator() { // This is an existing legacy way of array extraction, casting to a string (DataPolicyInfo.annotation.ontologyIris) when(mockValidator.getIndexExpression(anyString(), matches("e_aspectbar0annotation0ontologyIris"))) .thenReturn("(cast(replace(json_unquote(json_extract(`a_aspectbar`,'$.aspect.annotation.ontologyIris[*]')),'\"','') as char(255)))"); + + // New mocks for relationship field validation + when(mockValidator.getIndexExpression(anyString(), matches("e_metadata0field"))) + .thenReturn("(cast(json_extract(`metadata`, '$.field') as char(64)))"); } @Test From ae811008f333b01b2691e0f62cc020c327a6995a Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Fri, 21 Nov 2025 01:52:50 -0800 Subject: [PATCH 11/18] no more short name --- .../com/linkedin/metadata/dao/utils/SQLStatementUtils.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index 2a3629ca4..8fcafa517 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -672,13 +672,12 @@ protected static String parseLocalRelationshipField( // the entity table(s) --> virtual column use case that needs to be functionalized if (field.isAspectField()) { final String aspectFqcn = field.getAspectField().getAspect(); - final String aspectSimpleName = aspectFqcn.substring(aspectFqcn.lastIndexOf('.') + 1); final String path = field.getAspectField().getPath(); final String assetType = getAssetType(field.getAspectField()); final String indexedExpressionOrColumn = - SQLIndexFilterUtils.getIndexedExpressionOrColumn(assetType, aspectSimpleName, path, nonDollarVirtualColumnsEnabled, schemaValidator); + SQLIndexFilterUtils.getIndexedExpressionOrColumn(assetType, aspectFqcn, path, nonDollarVirtualColumnsEnabled, schemaValidator); if (indexedExpressionOrColumn == null) { throw new IllegalArgumentException( String.format("Neither expression nor column index not found for aspect field: Asset: %s, Aspect: %s, Path: %s", From 1a82ae2a73eba5d1ebf092a3d3549814092293df Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Mon, 24 Nov 2025 13:36:28 -0800 Subject: [PATCH 12/18] Testing is busted because DAO requires real tables to form proper query --- .../dao/utils/SQLIndexFilterUtils.java | 24 +++++++--- .../metadata/dao/utils/SQLSchemaUtils.java | 12 +++++ .../metadata/dao/utils/SQLStatementUtils.java | 46 +++++++++---------- .../EbeanLocalRelationshipQueryDAOTest.java | 1 + .../dao/utils/SQLIndexFilterUtilsTest.java | 15 ++++-- 5 files changed, 65 insertions(+), 33 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtils.java index 018957d1f..31e9299dd 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtils.java @@ -85,25 +85,37 @@ private static String getIndexedExpressionOrColumnGeneric(@Nonnull String expect * Get the expression index "identifier", if it exists, otherwise retrieve the generated column name. * The idea behind this is that whatever is returned from this method can be used verbatim to query the database; * it's either the expression index itself (new approach) or the virtual column (old approach). - * Intended to be used with entity tables. + * Intended to be used with entity table metadata. + */ + @Nullable + public static String getIndexedExpressionOrColumn(@Nonnull String assetType, @Nonnull String aspect, @Nonnull String path, + boolean nonDollarVirtualColumnsEnabled, @Nonnull String tableName, @Nonnull SchemaValidatorUtil schemaValidator) { + final String expectedLegacyColumnName = getGeneratedColumnName(assetType, aspect, path, nonDollarVirtualColumnsEnabled); + return getIndexedExpressionOrColumnGeneric(expectedLegacyColumnName, getExpressionIndexName(assetType, aspect, path), tableName, schemaValidator); + } + + /** + * Same as {@link #getIndexedExpressionOrColumn(String, String, String, boolean, String, SchemaValidatorUtil)} but + * uses the table name derived from the asset type. Requires strict assurance that assetType is well-formed. */ @Nullable public static String getIndexedExpressionOrColumn(@Nonnull String assetType, @Nonnull String aspect, @Nonnull String path, boolean nonDollarVirtualColumnsEnabled, @Nonnull SchemaValidatorUtil schemaValidator) { - final String indexColumn = getGeneratedColumnName(assetType, aspect, path, nonDollarVirtualColumnsEnabled); final String tableName = getTableName(assetType); - return getIndexedExpressionOrColumnGeneric(indexColumn, getExpressionIndexName(assetType, aspect, path), tableName, schemaValidator); + return getIndexedExpressionOrColumn(assetType, aspect, path, nonDollarVirtualColumnsEnabled, tableName, schemaValidator); } /** * Get the expression index "identifier", if it exists, otherwise retrieve the generated column name. * The idea behind this is that whatever is returned from this method can be used verbatim to query the database; * it's either the expression index itself (new approach) or the virtual column (old approach). - * Intended to be used with relationship tables. + * Intended to be used with relationship table metadata. */ @Nullable - public static String getIndexedExpressionOrColumnRelationship(@Nonnull String expectedLegacyColumnName, @Nonnull String path, - @Nonnull String tableName, @Nonnull SchemaValidatorUtil schemaValidator) { + public static String getIndexedExpressionOrColumnRelationship(@Nonnull String relationshipFieldName, @Nonnull String path, + boolean nonDollarVirtualColumnsEnabled, @Nonnull String tableName, @Nonnull SchemaValidatorUtil schemaValidator) { + final String expectedLegacyColumnName = + SQLSchemaUtils.getGeneratedColumnNameRelationship(relationshipFieldName, path, nonDollarVirtualColumnsEnabled); final String expectedExpressionIndexName = SQLSchemaUtils.getExpressionIndexNameRelationship(path); return getIndexedExpressionOrColumnGeneric(expectedLegacyColumnName, expectedExpressionIndexName, tableName, schemaValidator); } diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLSchemaUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLSchemaUtils.java index 343d68013..839461993 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLSchemaUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLSchemaUtils.java @@ -192,6 +192,18 @@ public static String getGeneratedColumnName(@Nonnull String assetType, @Nonnull return getExpectedNameHelper(INDEX_PREFIX, assetType, aspect, path, nonDollarVirtualColumnsEnabled); } + /** + * Like the above but follows the convention for relationship tables. + * DEPRECATED, when attempting to obtain an indexed value, please use + * {@link SQLIndexFilterUtils#getIndexedExpressionOrColumnRelationship(String, String, boolean, String, SchemaValidatorUtil)} instead. + */ + @Deprecated + @Nonnull + protected static String getGeneratedColumnNameRelationship(@Nonnull String relationshipFieldName, @Nonnull String path, + boolean nonDollarVirtualColumnsEnabled) { + return getExpectedNameFormatter("", relationshipFieldName, path, nonDollarVirtualColumnsEnabled); + } + /** * Get the expected expression index name from aspect and path. */ diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index 8fcafa517..d862ba5ed 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -445,7 +445,7 @@ public static String deleteLocalRelationshipSQL(final String tableName, boolean * Construct where clause SQL from multiple filters. Return null if all filters are empty. * @param supportedConditions contains supported conditions such as EQUAL. * @param nonDollarVirtualColumnsEnabled true if virtual column does not contain $, false otherwise - * @param relationshipTableName the relationship table name (used for RelationshipField schema validation) + * @param tableName the relationship table name (used for RelationshipField schema validation) * @param schemaValidator schema validator for checking column/index existence * @param filters An array of pairs which are filter and table prefix. * @return sql that can be appended after where clause. @@ -453,13 +453,13 @@ public static String deleteLocalRelationshipSQL(final String tableName, boolean @SafeVarargs @Nullable public static String whereClause(@Nonnull Map supportedConditions, boolean nonDollarVirtualColumnsEnabled, - @Nullable String relationshipTableName, @Nonnull SchemaValidatorUtil schemaValidator, + @Nullable String tableName, @Nonnull SchemaValidatorUtil schemaValidator, @Nonnull Pair... filters) { List andClauses = new ArrayList<>(); for (Pair filter : filters) { if (LogicalExpressionLocalRelationshipCriterionUtils.filterHasNonEmptyCriteria(filter.getValue0())) { andClauses.add("(" + whereClause( - filter.getValue0(), supportedConditions, filter.getValue1(), relationshipTableName, + filter.getValue0(), supportedConditions, filter.getValue1(), tableName, schemaValidator, nonDollarVirtualColumnsEnabled) + ")"); } } @@ -603,7 +603,6 @@ public static String whereClauseOldSchema(@Nonnull Map suppor * * @param tablePrefix table prefix, is expected to have a delimiter already appended * @param expression expression - * @param expectedVirtualColumnName expected virtual column name * @param originColumnName the column name in which the indexed field is derived / extracted * @return expression with table prefix */ @@ -611,15 +610,14 @@ public static String whereClauseOldSchema(@Nonnull Map suppor @Nonnull protected static String addTablePrefixToExpression(@Nonnull String tablePrefix, @Nonnull String expression, - @Nonnull String expectedVirtualColumnName, @Nonnull String originColumnName) { if (tablePrefix.isEmpty()) { return expression; } - // If the expression is the same as the expected virtual column name, we can just prepend the table prefix - // This is the case where, in evaluating "expression or column", the function returned a column (the expected column) - if (expression.equals(expectedVirtualColumnName)) { + // We make a reasonable assumption that if the expression has the characters '(' and ')', it's an expression + // Otherwise, it's a column + if (!expression.contains("(") && !expression.contains(")")) { return tablePrefix + expression; } @@ -640,13 +638,17 @@ protected static String addTablePrefixToExpression(@Nonnull String tablePrefix, return expression.replaceAll("(`?" + originColumnName + "`?)", "`" + tablePrefix + "`.`$1`"); } + // Note that tableName is currently only consumed if the relationshipCriterion is a RelationshipField. Otherwise, + // something like an Aspect-based field can have its table name derived through metadata extraction -- see AspectField.pdl, + // the same kind of metadata is NOT stored in RelationshipField.pdl and thus cannot be extracted in the same way. + // TODO (@jhui): It's possible to shrink the signature of the method by doing something similar in RelationshipField.pdl, + // but this is a query-facing model, not sure if it makes sense to surface it there... @VisibleForTesting protected static String parseLocalRelationshipField( @Nonnull final LocalRelationshipCriterion localRelationshipCriterion, @Nullable String tablePrefix, - @Nonnull String relationshipTableName, @Nonnull SchemaValidatorUtil schemaValidator, boolean nonDollarVirtualColumnsEnabled) { + @Nonnull String tableName, @Nonnull SchemaValidatorUtil schemaValidator, boolean nonDollarVirtualColumnsEnabled) { tablePrefix = tablePrefix == null ? "" : tablePrefix + "."; LocalRelationshipCriterion.Field field = localRelationshipCriterion.getField(); - char delimiter = nonDollarVirtualColumnsEnabled ? '0' : '$'; // UrnField.pdl defines UrnField.name as 'urn' // --> real column (not a virtual one), no need to "functionalize" @@ -657,15 +659,17 @@ protected static String parseLocalRelationshipField( // RelationshipField.pdl defines RelationshipField.name as 'metadata' -- ie. this is how "metadata$foo$bar" column is formed // --> virtual column use case that needs to be functionalized if (field.isRelationshipField()) { - // This next line is basically the original "expected naming logic" verbatim, keeping here for consistency - final String expectedVirtualColumnName = field.getRelationshipField().getName() + processPath(field.getRelationshipField().getPath(), delimiter); + final String relationshipFieldName = field.getRelationshipField().getName(); + final String path = field.getRelationshipField().getPath(); + final String indexedExpressionOrColumn = SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( - expectedVirtualColumnName, field.getRelationshipField().getPath(), - relationshipTableName, schemaValidator); + relationshipFieldName, path, nonDollarVirtualColumnsEnabled, tableName, schemaValidator); if (indexedExpressionOrColumn == null) { - throw new IllegalArgumentException("Neither expression nor column index not found for relationship field: " + expectedVirtualColumnName); + throw new IllegalArgumentException( + String.format("Neither expression nor column index not found for relationship field: RelationshipFieldName: %s, Path: %s", + relationshipFieldName, path)); } - return addTablePrefixToExpression(tablePrefix, indexedExpressionOrColumn, expectedVirtualColumnName, RELATIONSHIP_TABLE_EXPRESSION_INDEX_INFIX); + return addTablePrefixToExpression(tablePrefix, indexedExpressionOrColumn, RELATIONSHIP_TABLE_EXPRESSION_INDEX_INFIX); } // This appears to be when a join has already occurred and this is some indexed field from an aspect column from @@ -673,20 +677,16 @@ protected static String parseLocalRelationshipField( if (field.isAspectField()) { final String aspectFqcn = field.getAspectField().getAspect(); final String path = field.getAspectField().getPath(); - - final String assetType = getAssetType(field.getAspectField()); + final String assetType = getAssetType(field.getAspectField()); // NOT guaranteed to be set, must provide tableName in next call final String indexedExpressionOrColumn = - SQLIndexFilterUtils.getIndexedExpressionOrColumn(assetType, aspectFqcn, path, nonDollarVirtualColumnsEnabled, schemaValidator); + SQLIndexFilterUtils.getIndexedExpressionOrColumn(assetType, aspectFqcn, path, nonDollarVirtualColumnsEnabled, tableName, schemaValidator); if (indexedExpressionOrColumn == null) { throw new IllegalArgumentException( String.format("Neither expression nor column index not found for aspect field: Asset: %s, Aspect: %s, Path: %s", assetType, aspectFqcn, path)); } - final String expectedVirtualColumnName = - SQLSchemaUtils.getGeneratedColumnName(assetType, aspectFqcn, path, nonDollarVirtualColumnsEnabled); - return addTablePrefixToExpression(tablePrefix, indexedExpressionOrColumn, expectedVirtualColumnName, - SQLSchemaUtils.getAspectColumnName(assetType, aspectFqcn)); + return addTablePrefixToExpression(tablePrefix, indexedExpressionOrColumn, SQLSchemaUtils.getAspectColumnName(assetType, aspectFqcn)); } throw new IllegalArgumentException("Unrecognized field type"); diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/localrelationship/EbeanLocalRelationshipQueryDAOTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/localrelationship/EbeanLocalRelationshipQueryDAOTest.java index 5f7a3fcdb..e367ac4ce 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/localrelationship/EbeanLocalRelationshipQueryDAOTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/localrelationship/EbeanLocalRelationshipQueryDAOTest.java @@ -74,6 +74,7 @@ import static com.linkedin.metadata.dao.EbeanLocalRelationshipQueryDAO.*; import static com.linkedin.metadata.dao.utils.LogicalExpressionLocalRelationshipCriterionUtils.*; import static com.linkedin.testing.TestUtils.*; +import static org.mockito.Mockito.*; import static org.testng.Assert.*; diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java index 7e98cbb89..7f31ca6a4 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java @@ -247,15 +247,22 @@ public void testGetIndexedExpressionOrColumn() { @Test public void testGetIndexedExpressionOrColumnRelationship() { - // Get something that is NOT an expression (not mocked) -- delimiters are not relevant here because this logic is deferred - // to the caller + // Get something that is NOT an expression (not mocked) -- '$' variant assertEquals(SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( - "metadata$foofield", "/foofield", "tablenamedoesntmatterherebecauseofmock", mockValidator), + "metadata$foofield", "/foofield", false, + "tablenamedoesntmatterherebecauseofmock", mockValidator), "metadata$foofield"); + // Get something that is NOT an expression (not mocked) -- '0' variant + assertEquals(SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( + "metadata0foofield", "/foofield", true, + "tablenamedoesntmatterherebecauseofmock", mockValidator), + "metadata0foofield"); + // Get something that is an expression (mocked) assertEquals(SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( - "metadata$field", "/field", "tablenamedoesntmatterherebecauseofmock", mockValidator), + "metadata$field", "/field", false, + "tablenamedoesntmatterherebecauseofmock", mockValidator), "(cast(json_extract(`metadata`, '$.field') as char(64)))"); } From ba7362f25c61b41f8c56a817ae48bcc5f08d138f Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Wed, 26 Nov 2025 00:34:33 -0800 Subject: [PATCH 13/18] triplet --- .../dao/EbeanLocalRelationshipQueryDAO.java | 17 +++++----- .../utils/MultiHopsTraversalSqlGenerator.java | 18 +++++----- .../dao/utils/SQLIndexFilterUtils.java | 6 ++-- .../metadata/dao/utils/SQLStatementUtils.java | 34 ++++++++----------- .../EbeanLocalRelationshipQueryDAOTest.java | 28 +++++++-------- .../dao/utils/SQLIndexFilterUtilsTest.java | 6 ++-- .../dao/utils/SQLStatementUtilsTest.java | 10 +++--- 7 files changed, 58 insertions(+), 61 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalRelationshipQueryDAO.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalRelationshipQueryDAO.java index 6e86c88ba..52ca9b8ab 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalRelationshipQueryDAO.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalRelationshipQueryDAO.java @@ -35,7 +35,7 @@ import javax.persistence.PersistenceException; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang.StringUtils; -import org.javatuples.Pair; +import org.javatuples.Triplet; import pegasus.com.linkedin.metadata.query.LogicalExpressionLocalRelationshipCriterion; import pegasus.com.linkedin.metadata.query.LogicalOperation; import pegasus.com.linkedin.metadata.query.innerLogicalOperation.Operator; @@ -750,19 +750,19 @@ public String buildFindRelationshipSQL(@Nonnull final String relationshipTableNa sqlBuilder.append(" FROM ").append(relationshipTableName).append(" rt "); - List> filters = new ArrayList<>(); + List> filters = new ArrayList<>(); if (_schemaConfig == EbeanLocalDAO.SchemaConfig.NEW_SCHEMA_ONLY || _schemaConfig == EbeanLocalDAO.SchemaConfig.DUAL_SCHEMA) { if (destTableName != null) { sqlBuilder.append("INNER JOIN ").append(destTableName).append(" dt ON dt.urn=rt.destination "); if (destinationEntityFilter != null) { - filters.add(new Pair<>(destinationEntityFilter, "dt")); + filters.add(new Triplet<>(destinationEntityFilter, "dt", destTableName)); } } else if (destinationEntityFilter != null) { validateEntityFilterOnlyOneUrn(destinationEntityFilter); // non-mg entity case, applying dest filter on relationship table - filters.add(new Pair<>(destinationEntityFilter, "rt")); + filters.add(new Triplet<>(destinationEntityFilter, "rt", relationshipTableName)); } else if (filterHasNonEmptyCriteria(relationshipFilter)) { // Apply FORCE INDEX if destination field is being filtered, and the index exists final LocalRelationshipCriterionArray relationshipCriteria = @@ -783,7 +783,7 @@ public String buildFindRelationshipSQL(@Nonnull final String relationshipTableNa sqlBuilder.append("INNER JOIN ").append(sourceTableName).append(" st ON st.urn=rt.source "); if (sourceEntityFilter != null) { - filters.add(new Pair<>(sourceEntityFilter, "st")); + filters.add(new Triplet<>(sourceEntityFilter, "st", sourceTableName)); } } @@ -791,12 +791,11 @@ public String buildFindRelationshipSQL(@Nonnull final String relationshipTableNa sqlBuilder.append("WHERE rt.deleted_ts is NULL"); } - filters.add(new Pair<>(relationshipFilter, "rt")); + filters.add(new Triplet<>(relationshipFilter, "rt", relationshipTableName)); String whereClause = SQLStatementUtils.whereClause(SUPPORTED_CONDITIONS, - _eBeanDAOConfig.isNonDollarVirtualColumnsEnabled(), - relationshipTableName, _schemaValidatorUtil, - filters.toArray(new Pair[filters.size()])); + _eBeanDAOConfig.isNonDollarVirtualColumnsEnabled(), _schemaValidatorUtil, + filters.toArray(new Triplet[filters.size()])); if (whereClause != null) { sqlBuilder.append(includeNonCurrentRelationships ? " WHERE " : " AND ").append(whereClause); diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/MultiHopsTraversalSqlGenerator.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/MultiHopsTraversalSqlGenerator.java index d8516bcbc..7db96f505 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/MultiHopsTraversalSqlGenerator.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/MultiHopsTraversalSqlGenerator.java @@ -7,7 +7,7 @@ import java.util.Map; import javax.annotation.Nonnull; import javax.annotation.ParametersAreNonnullByDefault; -import org.javatuples.Pair; +import org.javatuples.Triplet; /** @@ -79,10 +79,10 @@ private String firstHopUrnsDirected(String relationshipTable, String srcEntityTa urnColumn, relationshipTable, destEntityTable, srcEntityTable)); String whereClause = SQLStatementUtils.whereClause(_supportedConditions, nonDollarVirtualColumnsEnabled, - relationshipTable, _schemaValidator, - new Pair<>(relationshipFilter, "rt"), - new Pair<>(destFilter, "dt"), - new Pair<>(srcFilter, "st")); + _schemaValidator, + new Triplet<>(relationshipFilter, "rt", relationshipTable), + new Triplet<>(destFilter, "dt", destEntityTable), + new Triplet<>(srcFilter, "st", srcEntityTable)); if (whereClause != null) { sqlBuilder.append(" AND ").append(whereClause); @@ -108,9 +108,9 @@ private String firstHopUrnsUndirected(String relationshipTable, String entityTab relationshipTable, entityTable)); String whereClause = SQLStatementUtils.whereClause(_supportedConditions, nonDollarVirtualColumnsEnabled, - relationshipTable, _schemaValidator, - new Pair<>(relationshipFilter, "rt"), - new Pair<>(srcFilter, "et")); + _schemaValidator, + new Triplet<>(relationshipFilter, "rt", relationshipTable), + new Triplet<>(srcFilter, "et", entityTable)); if (whereClause != null) { sourceUrnsSql.append(" AND ").append(whereClause); @@ -128,7 +128,7 @@ private String firstHopUrnsUndirected(String relationshipTable, String entityTab private String findEntitiesUndirected(String entityTable, String relationshipTable, String firstHopUrnSql, LocalRelationshipFilter destFilter, boolean nonDollarVirtualColumnsEnabled) { String whereClause = SQLStatementUtils.whereClause(_supportedConditions, nonDollarVirtualColumnsEnabled, - relationshipTable, _schemaValidator, new Pair<>(destFilter, "et")); + _schemaValidator, new Triplet<>(destFilter, "et", entityTable)); StringBuilder sourceEntitySql = new StringBuilder( String.format("SELECT et.* FROM %s et INNER JOIN %s rt ON et.urn=rt.source WHERE rt.destination IN (%s)", diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtils.java index 31e9299dd..673d64776 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtils.java @@ -95,8 +95,7 @@ public static String getIndexedExpressionOrColumn(@Nonnull String assetType, @No } /** - * Same as {@link #getIndexedExpressionOrColumn(String, String, String, boolean, String, SchemaValidatorUtil)} but - * uses the table name derived from the asset type. Requires strict assurance that assetType is well-formed. + * Like the above but derives the 'tableName' from the 'assetType' parameter. Requires strict assurance that it is set. */ @Nullable public static String getIndexedExpressionOrColumn(@Nonnull String assetType, @Nonnull String aspect, @Nonnull String path, @@ -110,6 +109,9 @@ public static String getIndexedExpressionOrColumn(@Nonnull String assetType, @No * The idea behind this is that whatever is returned from this method can be used verbatim to query the database; * it's either the expression index itself (new approach) or the virtual column (old approach). * Intended to be used with relationship table metadata. + * + * @param relationshipFieldName the name of the relationship field "name" as stored in the RelationshipField object + * defined by the RelationshipField.pdl model. This defaults to "metadata". */ @Nullable public static String getIndexedExpressionOrColumnRelationship(@Nonnull String relationshipFieldName, @Nonnull String path, diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index d862ba5ed..e833dbcca 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -24,7 +24,7 @@ import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang.StringEscapeUtils; import org.apache.commons.lang.StringUtils; -import org.javatuples.Pair; +import org.javatuples.Triplet; import pegasus.com.linkedin.metadata.query.LogicalExpressionLocalRelationshipCriterion; import pegasus.com.linkedin.metadata.query.LogicalExpressionLocalRelationshipCriterionArray; import pegasus.com.linkedin.metadata.query.LogicalOperation; @@ -445,21 +445,19 @@ public static String deleteLocalRelationshipSQL(final String tableName, boolean * Construct where clause SQL from multiple filters. Return null if all filters are empty. * @param supportedConditions contains supported conditions such as EQUAL. * @param nonDollarVirtualColumnsEnabled true if virtual column does not contain $, false otherwise - * @param tableName the relationship table name (used for RelationshipField schema validation) * @param schemaValidator schema validator for checking column/index existence - * @param filters An array of pairs which are filter and table prefix. + * @param filters An array of pairs which are filter and table prefix and table name * @return sql that can be appended after where clause. */ @SafeVarargs @Nullable public static String whereClause(@Nonnull Map supportedConditions, boolean nonDollarVirtualColumnsEnabled, - @Nullable String tableName, @Nonnull SchemaValidatorUtil schemaValidator, - @Nonnull Pair... filters) { + @Nonnull SchemaValidatorUtil schemaValidator, @Nonnull Triplet... filters) { List andClauses = new ArrayList<>(); - for (Pair filter : filters) { + for (Triplet filter : filters) { if (LogicalExpressionLocalRelationshipCriterionUtils.filterHasNonEmptyCriteria(filter.getValue0())) { andClauses.add("(" + whereClause( - filter.getValue0(), supportedConditions, filter.getValue1(), tableName, + filter.getValue0(), supportedConditions, filter.getValue1(), filter.getValue2(), schemaValidator, nonDollarVirtualColumnsEnabled) + ")"); } } @@ -484,7 +482,7 @@ public static String whereClause(@Nonnull Map supportedCondit */ @Nonnull public static String whereClause(@Nonnull LocalRelationshipFilter filter, - @Nonnull Map supportedConditions, @Nullable String tablePrefix, @Nullable String tableName, + @Nonnull Map supportedConditions, @Nullable String tablePrefix, @Nonnull String tableName, @Nonnull SchemaValidatorUtil schemaValidator, boolean nonDollarVirtualColumnsEnabled) { if (!LogicalExpressionLocalRelationshipCriterionUtils.filterHasNonEmptyCriteria(filter)) { throw new IllegalArgumentException("Empty filter cannot construct where clause."); @@ -497,7 +495,7 @@ public static String whereClause(@Nonnull LocalRelationshipFilter filter, } private static String buildSQLQueryFromLogicalExpression(@Nonnull LogicalExpressionLocalRelationshipCriterion criterion, - @Nonnull Map supportedConditions, @Nullable String tablePrefix, @Nullable String tableName, + @Nonnull Map supportedConditions, @Nullable String tablePrefix, @Nonnull String tableName, @Nonnull SchemaValidatorUtil schemaValidator, boolean nonDollarVirtualColumnsEnabled) { if (!criterion.hasExpr() || criterion.getExpr() == null) { throw new IllegalArgumentException("No logical expression found in criterion: " + criterion); @@ -533,7 +531,7 @@ private static String buildSQLQueryFromLogicalExpression(@Nonnull LogicalExpress } private static String buildSQLQueryFromLocalRelationshipCriterion(@Nonnull LocalRelationshipCriterion criterion, - @Nonnull Map supportedConditions, @Nullable String tablePrefix, @Nullable String tableName, + @Nonnull Map supportedConditions, @Nullable String tablePrefix, @Nonnull String tableName, @Nonnull SchemaValidatorUtil schemaValidator, boolean nonDollarVirtualColumnsEnabled) { final String field = parseLocalRelationshipField(criterion, tablePrefix, tableName, schemaValidator, nonDollarVirtualColumnsEnabled); @@ -638,9 +636,6 @@ protected static String addTablePrefixToExpression(@Nonnull String tablePrefix, return expression.replaceAll("(`?" + originColumnName + "`?)", "`" + tablePrefix + "`.`$1`"); } - // Note that tableName is currently only consumed if the relationshipCriterion is a RelationshipField. Otherwise, - // something like an Aspect-based field can have its table name derived through metadata extraction -- see AspectField.pdl, - // the same kind of metadata is NOT stored in RelationshipField.pdl and thus cannot be extracted in the same way. // TODO (@jhui): It's possible to shrink the signature of the method by doing something similar in RelationshipField.pdl, // but this is a query-facing model, not sure if it makes sense to surface it there... @VisibleForTesting @@ -666,8 +661,8 @@ protected static String parseLocalRelationshipField( relationshipFieldName, path, nonDollarVirtualColumnsEnabled, tableName, schemaValidator); if (indexedExpressionOrColumn == null) { throw new IllegalArgumentException( - String.format("Neither expression nor column index not found for relationship field: RelationshipFieldName: %s, Path: %s", - relationshipFieldName, path)); + String.format("Neither expression nor column index not found for relationship field: RelationshipFieldName: %s, Path: %s, TableName: %s", + relationshipFieldName, path, tableName)); } return addTablePrefixToExpression(tablePrefix, indexedExpressionOrColumn, RELATIONSHIP_TABLE_EXPRESSION_INDEX_INFIX); } @@ -677,14 +672,15 @@ protected static String parseLocalRelationshipField( if (field.isAspectField()) { final String aspectFqcn = field.getAspectField().getAspect(); final String path = field.getAspectField().getPath(); - final String assetType = getAssetType(field.getAspectField()); // NOT guaranteed to be set, must provide tableName in next call + final String assetType = getAssetType(field.getAspectField()); // NOT guaranteed to be set final String indexedExpressionOrColumn = - SQLIndexFilterUtils.getIndexedExpressionOrColumn(assetType, aspectFqcn, path, nonDollarVirtualColumnsEnabled, tableName, schemaValidator); + SQLIndexFilterUtils.getIndexedExpressionOrColumn(assetType, aspectFqcn, path, nonDollarVirtualColumnsEnabled, + tableName, schemaValidator); if (indexedExpressionOrColumn == null) { throw new IllegalArgumentException( - String.format("Neither expression nor column index not found for aspect field: Asset: %s, Aspect: %s, Path: %s", - assetType, aspectFqcn, path)); + String.format("Neither expression nor column index not found for aspect field: Asset: %s, Aspect: %s, Path: %s, TableName: %s", + assetType, aspectFqcn, path, tableName)); } return addTablePrefixToExpression(tablePrefix, indexedExpressionOrColumn, SQLSchemaUtils.getAspectColumnName(assetType, aspectFqcn)); } diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/localrelationship/EbeanLocalRelationshipQueryDAOTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/localrelationship/EbeanLocalRelationshipQueryDAOTest.java index e367ac4ce..8501db315 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/localrelationship/EbeanLocalRelationshipQueryDAOTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/localrelationship/EbeanLocalRelationshipQueryDAOTest.java @@ -1922,12 +1922,12 @@ public void testBuildFindRelationshipSQLWithSource() { String sql = _localRelationshipQueryDAO.buildFindRelationshipSQL("relationship_table_name", new LocalRelationshipFilter().setCriteria(new LocalRelationshipCriterionArray()).setDirection(RelationshipDirection.UNDIRECTED), - "source_table_name", srcFilter, "destination_table_name", null, + "metadata_entity_foo", srcFilter, "destination_table_name", null, -1, -1, new RelationshipLookUpContext()); assertEquals(sql, "SELECT rt.* FROM relationship_table_name rt INNER JOIN destination_table_name dt ON dt.urn=rt.destination " - + "INNER JOIN source_table_name st ON st.urn=rt.source WHERE rt.deleted_ts is NULL AND st.i_aspectfoo" + + "INNER JOIN metadata_entity_foo st ON st.urn=rt.source WHERE rt.deleted_ts is NULL AND st.i_aspectfoo" + (_eBeanDAOConfig.isNonDollarVirtualColumnsEnabled() ? "0" : "$") + "value='Alice'"); } @@ -1940,11 +1940,11 @@ public void testBuildFindRelationshipSQLWithDestination() { String sql = _localRelationshipQueryDAO.buildFindRelationshipSQL("relationship_table_name", new LocalRelationshipFilter().setCriteria(new LocalRelationshipCriterionArray()).setDirection(RelationshipDirection.UNDIRECTED), - "source_table_name", null, "destination_table_name", destFilter, + "source_table_name", null, "metadata_entity_bar", destFilter, -1, -1, new RelationshipLookUpContext()); assertEquals(sql, - "SELECT rt.* FROM relationship_table_name rt INNER JOIN destination_table_name dt ON dt.urn=rt.destination " + "SELECT rt.* FROM relationship_table_name rt INNER JOIN metadata_entity_bar dt ON dt.urn=rt.destination " + "INNER JOIN source_table_name st ON st.urn=rt.source WHERE rt.deleted_ts is NULL AND dt.i_aspectfoo" + (_eBeanDAOConfig.isNonDollarVirtualColumnsEnabled() ? "0" : "$") + "value='Alice'"); } @@ -1963,13 +1963,13 @@ public void testBuildFindRelationshipSQLWithSourceAndDestination() { String sql = _localRelationshipQueryDAO.buildFindRelationshipSQL("relationship_table_name", new LocalRelationshipFilter().setCriteria(new LocalRelationshipCriterionArray()).setDirection(RelationshipDirection.UNDIRECTED), - "source_table_name", srcFilter, "destination_table_name", destFilter, + "metadata_entity_foo", srcFilter, "metadata_entity_bar", destFilter, -1, -1, new RelationshipLookUpContext()); char virtualColumnDelimiter = _eBeanDAOConfig.isNonDollarVirtualColumnsEnabled() ? '0' : '$'; assertEquals(sql, - "SELECT rt.* FROM relationship_table_name rt INNER JOIN destination_table_name dt ON dt.urn=rt.destination " - + "INNER JOIN source_table_name st ON st.urn=rt.source WHERE rt.deleted_ts is NULL AND (dt.i_aspectfoo" + "SELECT rt.* FROM relationship_table_name rt INNER JOIN metadata_entity_bar dt ON dt.urn=rt.destination " + + "INNER JOIN metadata_entity_foo st ON st.urn=rt.source WHERE rt.deleted_ts is NULL AND (dt.i_aspectfoo" + virtualColumnDelimiter + "value='Bob') AND (st.i_aspectfoo" + virtualColumnDelimiter + "value='Alice')"); } @@ -2012,14 +2012,14 @@ public void testBuildFindRelationshipSQLWithHistoryWithSource() { String sql = _localRelationshipQueryDAO.buildFindRelationshipSQL("relationship_table_name", new LocalRelationshipFilter().setCriteria(new LocalRelationshipCriterionArray()).setDirection(RelationshipDirection.UNDIRECTED), - "source_table_name", srcFilter, "destination_table_name", null, + "metadata_entity_foo", srcFilter, "destination_table_name", null, -1, -1, new RelationshipLookUpContext(true)); assertEquals(sql, "SELECT * FROM (" + "SELECT rt.*, ROW_NUMBER() OVER (PARTITION BY rt.source, rt.destination ORDER BY rt.lastmodifiedon DESC) AS row_num " + "FROM relationship_table_name rt INNER JOIN destination_table_name dt ON dt.urn=rt.destination " - + "INNER JOIN source_table_name st ON st.urn=rt.source WHERE st.i_aspectfoo" + + "INNER JOIN metadata_entity_foo st ON st.urn=rt.source WHERE st.i_aspectfoo" + (_eBeanDAOConfig.isNonDollarVirtualColumnsEnabled() ? "0" : "$") + "value='Alice') ranked_rows WHERE row_num = 1"); } @@ -2032,13 +2032,13 @@ public void testBuildFindRelationshipSQLWithHistoryWithDestination() { String sql = _localRelationshipQueryDAO.buildFindRelationshipSQL("relationship_table_name", new LocalRelationshipFilter().setCriteria(new LocalRelationshipCriterionArray()).setDirection(RelationshipDirection.UNDIRECTED), - "source_table_name", null, "destination_table_name", destFilter, + "source_table_name", null, "metadata_entity_bar", destFilter, -1, -1, new RelationshipLookUpContext(true)); assertEquals(sql, "SELECT * FROM (" + "SELECT rt.*, ROW_NUMBER() OVER (PARTITION BY rt.source, rt.destination ORDER BY rt.lastmodifiedon DESC) AS row_num " - + "FROM relationship_table_name rt INNER JOIN destination_table_name dt ON dt.urn=rt.destination " + + "FROM relationship_table_name rt INNER JOIN metadata_entity_bar dt ON dt.urn=rt.destination " + "INNER JOIN source_table_name st ON st.urn=rt.source WHERE dt.i_aspectfoo" + (_eBeanDAOConfig.isNonDollarVirtualColumnsEnabled() ? "0" : "$") + "value='Alice') ranked_rows WHERE row_num = 1"); } @@ -2058,7 +2058,7 @@ public void testBuildFindRelationshipSQLWithHistoryWithSourceAndDestination() { String sql = _localRelationshipQueryDAO.buildFindRelationshipSQL("relationship_table_name", new LocalRelationshipFilter().setCriteria(new LocalRelationshipCriterionArray()).setDirection(RelationshipDirection.UNDIRECTED), - "source_table_name", srcFilter, "destination_table_name", destFilter, + "metadata_entity_foo", srcFilter, "metadata_entity_bar", destFilter, -1, -1, new RelationshipLookUpContext(true)); char virtualColumnDelimiter = _eBeanDAOConfig.isNonDollarVirtualColumnsEnabled() ? '0' : '$'; @@ -2066,8 +2066,8 @@ public void testBuildFindRelationshipSQLWithHistoryWithSourceAndDestination() { assertEquals(sql, "SELECT * FROM (" + "SELECT rt.*, ROW_NUMBER() OVER (PARTITION BY rt.source, rt.destination ORDER BY rt.lastmodifiedon DESC) AS row_num " - + "FROM relationship_table_name rt INNER JOIN destination_table_name dt ON dt.urn=rt.destination " - + "INNER JOIN source_table_name st ON st.urn=rt.source WHERE (dt.i_aspectfoo" + virtualColumnDelimiter + + "FROM relationship_table_name rt INNER JOIN metadata_entity_bar dt ON dt.urn=rt.destination " + + "INNER JOIN metadata_entity_foo st ON st.urn=rt.source WHERE (dt.i_aspectfoo" + virtualColumnDelimiter + "value='Bob') AND (st.urn='urn:li:foo:4')) ranked_rows WHERE row_num = 1"); } diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java index 7f31ca6a4..3805f6c65 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLIndexFilterUtilsTest.java @@ -249,19 +249,19 @@ public void testGetIndexedExpressionOrColumn() { public void testGetIndexedExpressionOrColumnRelationship() { // Get something that is NOT an expression (not mocked) -- '$' variant assertEquals(SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( - "metadata$foofield", "/foofield", false, + "metadata", "/foofield", false, "tablenamedoesntmatterherebecauseofmock", mockValidator), "metadata$foofield"); // Get something that is NOT an expression (not mocked) -- '0' variant assertEquals(SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( - "metadata0foofield", "/foofield", true, + "metadata", "/foofield", true, "tablenamedoesntmatterherebecauseofmock", mockValidator), "metadata0foofield"); // Get something that is an expression (mocked) assertEquals(SQLIndexFilterUtils.getIndexedExpressionOrColumnRelationship( - "metadata$field", "/field", false, + "metadata", "/field", false, "tablenamedoesntmatterherebecauseofmock", mockValidator), "(cast(json_extract(`metadata`, '$.field') as char(64)))"); } diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java index 277bd4a68..fcd85442f 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java @@ -31,7 +31,7 @@ import java.util.List; import java.util.Map; import java.util.Set; -import org.javatuples.Pair; +import org.javatuples.Triplet; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; import pegasus.com.linkedin.metadata.query.LogicalExpressionLocalRelationshipCriterion; @@ -443,14 +443,14 @@ public void testWhereClauseMultiFilters() { //test for multi filters with dollar virtual columns names assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, - PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), + mockValidator, new Triplet<>(filter1, "foo", "faketable1"), new Triplet<>(filter2, "bar", "faketable2")), "(foo.i_aspectfoo$value='value2' AND (foo.urn='value1' OR foo.urn='value3') " + "AND foo.metadata$value='value4') AND (bar.urn='value1' OR bar.urn='value2')" ); //test for multi filters with non dollar virtual columns names assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, - PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), + mockValidator, new Triplet<>(filter1, "foo", "faketable1"), new Triplet<>(filter2, "bar", "faketable2")), "(foo.i_aspectfoo0value='value2' AND (foo.urn='value1' OR foo.urn='value3') " + "AND foo.metadata0value='value4') AND (bar.urn='value1' OR bar.urn='value2')" ); @@ -508,14 +508,14 @@ public void testWhereClauseMultiFilters2() { //test for multi filters with dollar virtual columns names assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), false, - PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), + mockValidator, new Triplet<>(filter1, "foo", "faketable1"), new Triplet<>(filter2, "bar", "faketable2")), "(foo.i_aspectfoo$value LIKE 'value2%' AND (foo.urn LIKE 'value1%' OR foo.urn LIKE 'value3%') " + "AND foo.metadata$value LIKE 'value4%') AND (bar.urn LIKE 'value1%' OR bar.urn LIKE 'value2%')" ); //test for multi filters with non dollar virtual columns names assertConditionsEqual(SQLStatementUtils.whereClause(Collections.singletonMap(Condition.EQUAL, "="), true, - PLACEHOLDER_TABLE_NAME, mockValidator, new Pair<>(filter1, "foo"), new Pair<>(filter2, "bar")), + mockValidator, new Triplet<>(filter1, "foo", "faketable1"), new Triplet<>(filter2, "bar", "faketable2")), "(foo.i_aspectfoo0value LIKE 'value2%' AND (foo.urn LIKE 'value1%' OR foo.urn LIKE 'value3%') " + "AND foo.metadata0value LIKE 'value4%') AND (bar.urn LIKE 'value1%' OR bar.urn LIKE 'value2%')" ); From c07c1ce7f6c7b72d39833dfe5fbdab25c958753c Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Wed, 26 Nov 2025 00:55:22 -0800 Subject: [PATCH 14/18] comments --- .../metadata/dao/utils/SQLStatementUtilsTest.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java index fcd85442f..6ac676e60 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java @@ -49,9 +49,10 @@ public class SQLStatementUtilsTest { private static SchemaValidatorUtil mockValidator; - // This is (dummy) table name placeholder for testing, introduced because we need to propagate the relationship table - // name for relationship table functional index processing. However, for existing tests that simply don't test for - // relationship table functional index processing, we can just use a dummy table name. + // This is (dummy) table name placeholder for testing, introduced because we need to propagate the table name + // used in a "where filter" through the "where()" call. + // HOWEVER, since the table name ends up being directed into mocked calls in SchemaValidator, it doesn't matter + // what the table name is, so we'll just create and use a placeholder here. private static final String PLACEHOLDER_TABLE_NAME = "placeholder"; @BeforeClass @@ -1221,4 +1222,6 @@ public void testBuildSQLQueryFromLocalRelationshipCriterionWithInjection() { assertEquals(whereClause, "destination LIKE 'urn:li:dataset:prefix''%%'"); } + + } \ No newline at end of file From 2ea9aa33194e7794cf462e261956a9ab0ff6b767 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Wed, 26 Nov 2025 01:18:35 -0800 Subject: [PATCH 15/18] testing added for addTablePrefixToExpression --- .../metadata/dao/utils/SQLStatementUtils.java | 13 ++-- .../dao/utils/SQLStatementUtilsTest.java | 67 +++++++++++++++++++ 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index e833dbcca..d22e9805b 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -599,7 +599,7 @@ public static String whereClauseOldSchema(@Nonnull Map suppor * or the value of an Expression Index. In the latter case, we have to replace the table prefix in the expression * as opposed to a simple prepend operation. * - * @param tablePrefix table prefix, is expected to have a delimiter already appended + * @param tablePrefix table prefix, is expected to NOT the delimiter '.' already appended * @param expression expression * @param originColumnName the column name in which the indexed field is derived / extracted * @return expression with table prefix @@ -616,7 +616,7 @@ protected static String addTablePrefixToExpression(@Nonnull String tablePrefix, // We make a reasonable assumption that if the expression has the characters '(' and ')', it's an expression // Otherwise, it's a column if (!expression.contains("(") && !expression.contains(")")) { - return tablePrefix + expression; + return tablePrefix + "." + expression; } // This means that an expression index is being used. In this case, we need to prepend the prefix by injecting it @@ -633,22 +633,21 @@ protected static String addTablePrefixToExpression(@Nonnull String tablePrefix, // Note that for good syntactic practice, we will surround the table prefix with backticks no matter what. // So what we want to do is look for the originColumnName then inject the table prefix before it. - return expression.replaceAll("(`?" + originColumnName + "`?)", "`" + tablePrefix + "`.`$1`"); + // We use a negative lookbehind (? real column (not a virtual one), no need to "functionalize" if (field.isUrnField()) { - return tablePrefix + field.getUrnField().getName(); + return tablePrefix + "." + field.getUrnField().getName(); } // RelationshipField.pdl defines RelationshipField.name as 'metadata' -- ie. this is how "metadata$foo$bar" column is formed diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java index 6ac676e60..3e25ba0f7 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java @@ -1222,6 +1222,73 @@ public void testBuildSQLQueryFromLocalRelationshipCriterionWithInjection() { assertEquals(whereClause, "destination LIKE 'urn:li:dataset:prefix''%%'"); } + @Test + public void testAddTablePrefixToExpression() { + // Test case 1: Empty table prefix should return expression unchanged + String expression1 = "(cast(json_extract(`a_aspectbar`, '$.aspect.value') as char(1024)))"; + assertEquals(SQLStatementUtils.addTablePrefixToExpression("", expression1, "a_aspectbar"), expression1); + + // Test case 2: Simple column (no parentheses) should just prepend prefix + assertEquals(SQLStatementUtils.addTablePrefixToExpression("rt", "i_aspectfoo$value", "a_aspectfoo"), + "rt.i_aspectfoo$value"); + + // Test case 3 (from comments): Expression with backticks around column name + // (cast(json_extract(`a_aspectbar`, '$.aspect.value') as char(1024))) + // --> (cast(json_extract(`PREFIX`.`a_aspectbar`, '$.aspect.value') as char(1024))) + assertEquals( + SQLStatementUtils.addTablePrefixToExpression("PREFIX", + "(cast(json_extract(`a_aspectbar`, '$.aspect.value') as char(1024)))", "a_aspectbar"), + "(cast(json_extract(`PREFIX`.`a_aspectbar`, '$.aspect.value') as char(1024)))"); + + // Test case 4 (from comments): Expression without backticks around column name + // (cast(json_extract(a_aspectbar, '$.aspect.value') as char(1024))) + // --> (cast(json_extract(`PREFIX`.`a_aspectbar`, '$.aspect.value') as char(1024))) + assertEquals( + SQLStatementUtils.addTablePrefixToExpression("PREFIX", + "(cast(json_extract(a_aspectbar, '$.aspect.value') as char(1024)))", "a_aspectbar"), + "(cast(json_extract(`PREFIX`.a_aspectbar, '$.aspect.value') as char(1024)))"); + + // Test case 5: Metadata column in relationship table (common use case) with backticks + assertEquals( + SQLStatementUtils.addTablePrefixToExpression("rt", + "(cast(json_extract(`metadata`, '$.field') as char(64)))", "metadata"), + "(cast(json_extract(`rt`.`metadata`, '$.field') as char(64)))"); + + // Test case 6: Metadata column without backticks + assertEquals( + SQLStatementUtils.addTablePrefixToExpression("rt", + "(cast(json_extract(metadata, '$.field') as char(64)))", "metadata"), + "(cast(json_extract(`rt`.metadata, '$.field') as char(64)))"); + + // Test case 7: Array expression index + assertEquals( + SQLStatementUtils.addTablePrefixToExpression("dt", + "(cast(json_extract(`a_aspectbar`, '$.aspect.value_array') as char(128) array))", "a_aspectbar"), + "(cast(json_extract(`dt`.`a_aspectbar`, '$.aspect.value_array') as char(128) array))"); + + // Test case 8: Complex nested JSON path (legacy array extraction) + assertEquals( + SQLStatementUtils.addTablePrefixToExpression("st", + "(cast(replace(json_unquote(json_extract(`a_aspectbar`,'$.aspect.annotation.ontologyIris[*]')),'\"','') as char(255)))", + "a_aspectbar"), + "(cast(replace(json_unquote(json_extract(`st`.`a_aspectbar`,'$.aspect.annotation.ontologyIris[*]')),'\"','') as char(255)))"); + + // Test case 9: Column name appears in JSON path - should only replace column reference + assertEquals( + SQLStatementUtils.addTablePrefixToExpression("foo", + "(cast(json_extract(`a_aspectfoo`, '$.a_aspectfoo.value') as char(1024)))", "a_aspectfoo"), + "(cast(json_extract(`foo`.`a_aspectfoo`, '$.a_aspectfoo.value') as char(1024)))"); + // Test case 13: Column name substring appears in JSON path - should only replace actual column reference + assertEquals( + SQLStatementUtils.addTablePrefixToExpression("rt", + "(cast(json_extract(`metadata`, '$.metadata.field') as char(64)))", "metadata"), + "(cast(json_extract(`rt`.`metadata`, '$.metadata.field') as char(64)))"); + + // Test case 14: Multiple occurrences of column name - should replace all + assertEquals( + SQLStatementUtils.addTablePrefixToExpression("t1", "CONCAT(`a_col`, `a_col`)", "a_col"), + "CONCAT(`t1`.`a_col`, `t1`.`a_col`)"); + } } \ No newline at end of file From 60d5360bed0daad2a081ee49cc26225987a4d379 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Wed, 26 Nov 2025 01:35:21 -0800 Subject: [PATCH 16/18] fix prefix --- .../metadata/dao/utils/SQLStatementUtils.java | 2 +- .../dao/utils/SQLStatementUtilsTest.java | 119 ++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index d22e9805b..d3b8775c2 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -647,7 +647,7 @@ protected static String parseLocalRelationshipField( // UrnField.pdl defines UrnField.name as 'urn' // --> real column (not a virtual one), no need to "functionalize" if (field.isUrnField()) { - return tablePrefix + "." + field.getUrnField().getName(); + return addTablePrefixToExpression(tablePrefix, field.getUrnField().getName(), field.getUrnField().getName()); } // RelationshipField.pdl defines RelationshipField.name as 'metadata' -- ie. this is how "metadata$foo$bar" column is formed diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java index 609d8c5e4..141bdc6e9 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/SQLStatementUtilsTest.java @@ -1291,4 +1291,123 @@ public void testAddTablePrefixToExpression() { "CONCAT(`t1`.`a_col`, `t1`.`a_col`)"); } +// @Test + public void testParseLocalRelationshipField() { + // Test case 1: UrnField with null tablePrefix + LocalRelationshipCriterion.Field urnField1 = new LocalRelationshipCriterion.Field(); + urnField1.setUrnField(new UrnField().setName("urn")); + LocalRelationshipCriterion urnCriterion1 = new LocalRelationshipCriterion() + .setField(urnField1) + .setCondition(Condition.EQUAL) + .setValue(LocalRelationshipValue.create("value1")); + + assertEquals(SQLStatementUtils.parseLocalRelationshipField(urnCriterion1, null, PLACEHOLDER_TABLE_NAME, + mockValidator, false), ".urn"); + + // Test case 2: UrnField with non-null tablePrefix + LocalRelationshipCriterion.Field urnField2 = new LocalRelationshipCriterion.Field(); + urnField2.setUrnField(new UrnField().setName("urn")); + LocalRelationshipCriterion urnCriterion2 = new LocalRelationshipCriterion() + .setField(urnField2) + .setCondition(Condition.EQUAL) + .setValue(LocalRelationshipValue.create("value2")); + + assertEquals(SQLStatementUtils.parseLocalRelationshipField(urnCriterion2, "rt", PLACEHOLDER_TABLE_NAME, + mockValidator, false), "rt.urn"); + + // Test case 3: RelationshipField with virtual column (dollar-separated) + LocalRelationshipCriterion.Field relationshipField1 = new LocalRelationshipCriterion.Field(); + relationshipField1.setRelationshipField(new RelationshipField().setName("metadata").setPath("/value")); + LocalRelationshipCriterion relationshipCriterion1 = new LocalRelationshipCriterion() + .setField(relationshipField1) + .setCondition(Condition.EQUAL) + .setValue(LocalRelationshipValue.create("value3")); + + assertEquals(SQLStatementUtils.parseLocalRelationshipField(relationshipCriterion1, null, PLACEHOLDER_TABLE_NAME, + mockValidator, false), "metadata$value"); + + // Test case 4: RelationshipField with virtual column (non-dollar) + assertEquals(SQLStatementUtils.parseLocalRelationshipField(relationshipCriterion1, null, PLACEHOLDER_TABLE_NAME, + mockValidator, true), "metadata0value"); + + // Test case 5: RelationshipField with table prefix and virtual column + assertEquals(SQLStatementUtils.parseLocalRelationshipField(relationshipCriterion1, "rt", PLACEHOLDER_TABLE_NAME, + mockValidator, false), "rt.metadata$value"); + + // Test case 6: RelationshipField with expression index and table prefix + LocalRelationshipCriterion.Field relationshipField2 = new LocalRelationshipCriterion.Field(); + relationshipField2.setRelationshipField(new RelationshipField().setName("metadata").setPath("/field")); + LocalRelationshipCriterion relationshipCriterion2 = new LocalRelationshipCriterion() + .setField(relationshipField2) + .setCondition(Condition.EQUAL) + .setValue(LocalRelationshipValue.create("value4")); + + assertEquals(SQLStatementUtils.parseLocalRelationshipField(relationshipCriterion2, "rt", PLACEHOLDER_TABLE_NAME, + mockValidator, false), "(cast(json_extract(`rt`.`metadata`, '$.field') as char(64)))"); + + // Test case 7: AspectField with virtual column (dollar-separated) + LocalRelationshipCriterion.Field aspectField1 = new LocalRelationshipCriterion.Field(); + aspectField1.setAspectField(new AspectField().setAspect(AspectFoo.class.getCanonicalName()).setPath("/value")); + LocalRelationshipCriterion aspectCriterion1 = new LocalRelationshipCriterion() + .setField(aspectField1) + .setCondition(Condition.EQUAL) + .setValue(LocalRelationshipValue.create("value5")); + + assertEquals(SQLStatementUtils.parseLocalRelationshipField(aspectCriterion1, null, PLACEHOLDER_TABLE_NAME, + mockValidator, false), "i_aspectfoo$value"); + + // Test case 8: AspectField with virtual column (non-dollar) + assertEquals(SQLStatementUtils.parseLocalRelationshipField(aspectCriterion1, null, PLACEHOLDER_TABLE_NAME, + mockValidator, true), "i_aspectfoo0value"); + + // Test case 9: AspectField with table prefix and virtual column + assertEquals(SQLStatementUtils.parseLocalRelationshipField(aspectCriterion1, "t1", PLACEHOLDER_TABLE_NAME, + mockValidator, false), "t1.i_aspectfoo$value"); + + // Test case 10: AspectField with expression index (AspectBar with "value" field) + LocalRelationshipCriterion.Field aspectField2 = new LocalRelationshipCriterion.Field(); + aspectField2.setAspectField(new AspectField().setAspect(AspectBar.class.getCanonicalName()).setPath("/aspect/value")); + LocalRelationshipCriterion aspectCriterion2 = new LocalRelationshipCriterion() + .setField(aspectField2) + .setCondition(Condition.EQUAL) + .setValue(LocalRelationshipValue.create("value6")); + + assertEquals(SQLStatementUtils.parseLocalRelationshipField(aspectCriterion2, "PREFIX", PLACEHOLDER_TABLE_NAME, + mockValidator, false), "(cast(json_extract(`PREFIX`.`a_aspectbar`, '$.aspect.value') as char(1024)))"); + + // Test case 11: AspectField with expression index and no table prefix + assertEquals(SQLStatementUtils.parseLocalRelationshipField(aspectCriterion2, null, PLACEHOLDER_TABLE_NAME, + mockValidator, false), "(cast(json_extract(`a_aspectbar`, '$.aspect.value') as char(1024)))"); + + // Test case 12: AspectField with asset type specified + LocalRelationshipCriterion.Field aspectField3 = new LocalRelationshipCriterion.Field(); + aspectField3.setAspectField(new AspectField() + .setAspect(AspectBar.class.getCanonicalName()) + .setAsset(BarAsset.class.getCanonicalName()) + .setPath("/aspect/value")); + LocalRelationshipCriterion aspectCriterion3 = new LocalRelationshipCriterion() + .setField(aspectField3) + .setCondition(Condition.EQUAL) + .setValue(LocalRelationshipValue.create("value7")); + + assertEquals(SQLStatementUtils.parseLocalRelationshipField(aspectCriterion3, "t2", PLACEHOLDER_TABLE_NAME, + mockValidator, false), "(cast(json_extract(`t2`.`a_aspectbar`, '$.aspect.value') as char(1024)))"); + + // Test case 13: Invalid field type - should throw exception + LocalRelationshipCriterion.Field invalidField = new LocalRelationshipCriterion.Field(); + // Don't set any field type (not urn, relationship, or aspect) + LocalRelationshipCriterion invalidCriterion = new LocalRelationshipCriterion() + .setField(invalidField) + .setCondition(Condition.EQUAL) + .setValue(LocalRelationshipValue.create("value8")); + + try { + SQLStatementUtils.parseLocalRelationshipField(invalidCriterion, null, PLACEHOLDER_TABLE_NAME, + mockValidator, false); + fail("Expected IllegalArgumentException for unrecognized field type"); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "Unrecognized field type"); + } + } + } \ No newline at end of file From c8ed02327d9c53bb5a3d89e29ca43bba56f91e38 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Thu, 27 Nov 2025 17:00:22 -0800 Subject: [PATCH 17/18] Rounds off testing! --- .../metadata/dao/utils/SQLStatementUtils.java | 3 +- .../dao/utils/SQLStatementUtilsTest.java | 43 ++++++++++++------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index d3b8775c2..977019caa 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -630,11 +630,10 @@ protected static String addTablePrefixToExpression(@Nonnull String tablePrefix, // In this way, we could also have a case like: // (cast(json_extract(a_aspectbar, '$.aspect.value') as char(1024))) // --> (cast(json_extract(`PREFIX`.a_aspectbar, '$.aspect.value') as char(1024))) - // Note that for good syntactic practice, we will surround the table prefix with backticks no matter what. // So what we want to do is look for the originColumnName then inject the table prefix before it. // We use a negative lookbehind (? Date: Thu, 27 Nov 2025 17:10:36 -0800 Subject: [PATCH 18/18] add tick --- .../com/linkedin/metadata/dao/utils/SQLStatementUtils.java | 3 ++- .../linkedin/metadata/dao/utils/SQLStatementUtilsTest.java | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java index 977019caa..a5c0f2567 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/SQLStatementUtils.java @@ -630,10 +630,11 @@ protected static String addTablePrefixToExpression(@Nonnull String tablePrefix, // In this way, we could also have a case like: // (cast(json_extract(a_aspectbar, '$.aspect.value') as char(1024))) // --> (cast(json_extract(`PREFIX`.a_aspectbar, '$.aspect.value') as char(1024))) + // For safety, we'll always attach the '`' characters in non-column settings for clarity // So what we want to do is look for the originColumnName then inject the table prefix before it. // We use a negative lookbehind (?