Skip to content

Fix: Skipping Multiple column references by a single column in Foreign Key during Index Creation #22658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 30, 2025
Merged

Fix: Skipping Multiple column references by a single column in Foreign Key during Index Creation #22658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e478dd3
Fix: Skipping Multiple column references by a single column in Foreig…
yan-3005 Jul 30, 2025
d107e34
Merge branch 'main' into table-constraint-fix-main
yan-3005 Jul 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 134 additions & 36 deletions openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/SearchIndex.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
import org.openmetadata.service.search.SearchClient;
import org.openmetadata.service.search.SearchIndexUtils;
import org.openmetadata.service.util.FullyQualifiedName;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public interface SearchIndex {
Set<String> DEFAULT_EXCLUDED_FIELDS =
Expand All @@ -56,6 +58,7 @@ public interface SearchIndex {
"changeSummary");

public static final SearchClient searchClient = Entity.getSearchRepository().getSearchClient();
static final Logger LOG = LoggerFactory.getLogger(SearchIndex.class);

default Map<String, Object> buildSearchIndexDoc() {
// Build Index Doc
Expand Down Expand Up @@ -208,6 +211,15 @@ private static void processUpstreamConstraints(
continue;
}

// Validate constraint has required data
if (nullOrEmpty(tableConstraint.getColumns())
|| nullOrEmpty(tableConstraint.getReferredColumns())) {
LOG.warn(
"Skipping invalid constraint for entity '{}': missing columns or referredColumns",
entity.getFullyQualifiedName());
continue;
}

int columnIndex = 0;
for (String referredColumn : listOrEmpty(tableConstraint.getReferredColumns())) {
String relatedEntityFQN = getParentFQN(referredColumn);
Expand All @@ -226,9 +238,12 @@ private static void processUpstreamConstraints(
updateExistingUpstreamRelationship(
entity, tableConstraint, relationshipMap, referredColumn, columnIndex);
} else {
upstreamRelationships.add(
Map<String, Object> newRelationshipMap =
buildUpstreamRelationshipMap(
entity, relatedEntity, tableConstraint, referredColumn, columnIndex));
entity, relatedEntity, tableConstraint, referredColumn, columnIndex);
if (newRelationshipMap != null) {
upstreamRelationships.add(newRelationshipMap);
}
}

columnIndex++;
Expand All @@ -244,28 +259,70 @@ private static Map<String, Object> buildUpstreamRelationshipMap(
TableConstraint tableConstraint,
String referredColumn,
int columnIndex) {
Map<String, Object> relationshipMap = new HashMap<>();

// Store only entity field (upstream entity)
// relatedEntity is the upstream entity that the current entity depends on
relationshipMap.put(
"entity", buildEntityRefMap(relatedEntity.getEntityReference())); // upstream entity only
relationshipMap.put(
"docId", relatedEntity.getId().toString() + "-" + entity.getId().toString());

List<Map<String, Object>> columns = new ArrayList<>();
String columnFQN =
FullyQualifiedName.add(
entity.getFullyQualifiedName(), tableConstraint.getColumns().get(columnIndex));

Map<String, Object> columnMap = new HashMap<>();
columnMap.put("columnFQN", referredColumn); // Upstream column
columnMap.put("relatedColumnFQN", columnFQN); // Downstream column
columnMap.put("relationshipType", tableConstraint.getRelationshipType());
columns.add(columnMap);

relationshipMap.put("columns", columns);
return relationshipMap;

// Handle composite key scenarios gracefully
List<String> columns = tableConstraint.getColumns();
List<String> referredColumns = tableConstraint.getReferredColumns();

if (columns == null || columns.isEmpty()) {
LOG.warn(
"Table constraint has no local columns for entity: {}. Skipping constraint creation.",
entity.getFullyQualifiedName());
return null;
}

// Detect composite foreign key constraints
if (referredColumns != null && columns.size() != referredColumns.size()) {
LOG.info(
"Composite foreign key constraint detected for table '{}': {} Table columns mapped to {} referred columns.",
entity.getFullyQualifiedName(),
columns.size(),
referredColumns.size());
return null;
}

// Safe bounds checking for matching sizes
if (columnIndex >= columns.size()) {
LOG.warn(
"Column index {} is out of bounds for constraint columns of size {}. Skipping constraint creation.",
columnIndex,
columns.size());
return null;
}

try {
Map<String, Object> relationshipMap = new HashMap<>();

// Store only entity field (upstream entity)
// relatedEntity is the upstream entity that the current entity depends on
relationshipMap.put(
"entity", buildEntityRefMap(relatedEntity.getEntityReference())); // upstream entity only
relationshipMap.put(
"docId", relatedEntity.getId().toString() + "-" + entity.getId().toString());

List<Map<String, Object>> columnsList = new ArrayList<>();
String columnFQN =
FullyQualifiedName.add(entity.getFullyQualifiedName(), columns.get(columnIndex));

Map<String, Object> columnMap = new HashMap<>();
columnMap.put("columnFQN", referredColumn); // Upstream column
columnMap.put("relatedColumnFQN", columnFQN); // Downstream column
columnMap.put("relationshipType", tableConstraint.getRelationshipType());
columnsList.add(columnMap);

relationshipMap.put("columns", columnsList);
return relationshipMap;

} catch (Exception ex) {
LOG.error(
"Failed to create constraint relationship for entity '{}', column index {}, referred column '{}'. "
+ "Skipping this relationship to continue processing. Error: {}",
entity.getFullyQualifiedName(),
columnIndex,
referredColumn,
ex.getMessage());
return null;
}
}

static Map<String, Object> checkUpstreamRelationship(
Expand All @@ -286,18 +343,59 @@ private static void updateExistingUpstreamRelationship(
Map<String, Object> existingRelationship,
String referredColumn,
int columnIndex) {
String columnFQN =
FullyQualifiedName.add(
entity.getFullyQualifiedName(), tableConstraint.getColumns().get(columnIndex));

Map<String, Object> columnMap = new HashMap<>();
columnMap.put("columnFQN", referredColumn); // Upstream column
columnMap.put("relatedColumnFQN", columnFQN); // Downstream column
columnMap.put("relationshipType", tableConstraint.getRelationshipType());

List<Map<String, Object>> existingColumns =
(List<Map<String, Object>>) existingRelationship.get("columns");
existingColumns.add(columnMap);

// Handle composite key scenarios gracefully
List<String> columns = tableConstraint.getColumns();
List<String> referredColumns = tableConstraint.getReferredColumns();

if (columns == null || columns.isEmpty()) {
LOG.warn(
"Table constraint has no local columns for entity: {}. Skipping constraint update.",
entity.getFullyQualifiedName());
return;
}

// Detect composite foreign key constraints
if (referredColumns != null && columns.size() != referredColumns.size()) {
LOG.info(
"Composite foreign key constraint detected for table '{}': {} Table columns mapped to {} referred columns.",
entity.getFullyQualifiedName(),
columns.size(),
referredColumns.size());
return;
}

// Safe bounds checking for matching sizes
if (columnIndex >= columns.size()) {
LOG.warn(
"Column index {} is out of bounds for constraint columns of size {}. Skipping constraint update.",
columnIndex,
columns.size());
return;
}

try {
String columnFQN =
FullyQualifiedName.add(entity.getFullyQualifiedName(), columns.get(columnIndex));

Map<String, Object> columnMap = new HashMap<>();
columnMap.put("columnFQN", referredColumn); // Upstream column
columnMap.put("relatedColumnFQN", columnFQN); // Downstream column
columnMap.put("relationshipType", tableConstraint.getRelationshipType());

List<Map<String, Object>> existingColumns =
(List<Map<String, Object>>) existingRelationship.get("columns");
existingColumns.add(columnMap);

} catch (Exception ex) {
LOG.error(
"Failed to update constraint relationship for entity '{}', column index {}, referred column '{}'. "
+ "Skipping this relationship to continue processing. Error: {}",
entity.getFullyQualifiedName(),
columnIndex,
referredColumn,
ex.getMessage());
}
}

static Map<String, Object> buildEntityRefMap(EntityReference entityRef) {
Expand Down
78 changes: 78 additions & 0 deletions ...service/src/test/java/org/openmetadata/service/resources/databases/TableResourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static jakarta.ws.rs.core.Response.Status.UNAUTHORIZED;
import static java.lang.String.format;
import static java.util.Collections.singletonList;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
Expand Down Expand Up @@ -5743,4 +5744,81 @@ void test_tableEntityRelationshipDirectionEndpointWithDepth() throws IOException
e.getEntity().getId().equals(tableB.getId())
&& e.getRelatedEntity().getId().equals(tableC.getId())));
}

@Test
void test_compositeKeyConstraintIndexOutOfBounds_fixed(TestInfo test) throws IOException {
// Create a schema for this test to avoid conflicts
CreateDatabaseSchema createSchema = schemaTest.createRequest("composite_key_test_schema");
DatabaseSchema schema = schemaTest.createEntity(createSchema, ADMIN_AUTH_HEADERS);

// Create tables and columns for FK relationships
Column c1 = new Column().withName("user_ref").withDataType(ColumnDataType.STRING);
Column c2 = new Column().withName("tenant_id").withDataType(ColumnDataType.STRING);
Column c3 = new Column().withName("user_id").withDataType(ColumnDataType.STRING);

// Create target table (referenced table with 2 columns)
Table targetTable =
createEntity(
createRequest("target_table")
.withDatabaseSchema(schema.getFullyQualifiedName())
.withTableConstraints(null)
.withColumns(List.of(c2, c3)),
ADMIN_AUTH_HEADERS);

// Create source table (no constraints initially)
Table sourceTable =
createEntity(
createRequest("source_table")
.withDatabaseSchema(schema.getFullyQualifiedName())
.withColumns(List.of(c1))
.withTableConstraints(null),
ADMIN_AUTH_HEADERS);

// Resolve column FQNs needed for FK definitions
Table targetRef = getEntityByName(targetTable.getFullyQualifiedName(), ADMIN_AUTH_HEADERS);

// Step 2: Create the problematic constraint using deep copy method (1 local column -> 2
// referred columns)
String targetCol1FQN = targetRef.getColumns().get(0).getFullyQualifiedName();
String targetCol2FQN = targetRef.getColumns().get(1).getFullyQualifiedName();

String originalJson = JsonUtils.pojoToJson(sourceTable);
Table sourceTableV2 = JsonUtils.deepCopy(sourceTable, Table.class);

// Create the problematic constraint: 1 local column referencing 2 referred columns
TableConstraint problematicConstraint =
new TableConstraint()
.withConstraintType(TableConstraint.ConstraintType.FOREIGN_KEY)
.withColumns(Arrays.asList("user_ref")) // 1 local column
.withReferredColumns(Arrays.asList(targetCol1FQN, targetCol2FQN)); // 2 referred columns

sourceTableV2.setTableConstraints(Arrays.asList(problematicConstraint));

Table updatedTable =
patchEntity(sourceTable.getId(), originalJson, sourceTableV2, ADMIN_AUTH_HEADERS);

// Step 3: Verify constraint structure (the problematic case: 1 column -> 2 referred columns)
assertNotNull(updatedTable.getTableConstraints());
assertEquals(1, updatedTable.getTableConstraints().size());
TableConstraint constraint = updatedTable.getTableConstraints().get(0);
assertEquals(TableConstraint.ConstraintType.FOREIGN_KEY, constraint.getConstraintType());
assertEquals(1, constraint.getColumns().size()); // 1 local column
assertEquals(
2,
constraint
.getReferredColumns()
.size()); // 2 referred columns - this causes IndexOutOfBounds!

// Step 4: Build search index doc - this should NOT crash with our fix
assertDoesNotThrow(
() -> {
Entity.buildSearchIndex(Entity.TABLE, updatedTable);
},
"Search index building should not crash with composite key constraints");

// Step 5: Verify the constraint was properly stored
Table fetchedTable = getEntity(updatedTable.getId(), ADMIN_AUTH_HEADERS);
assertNotNull(fetchedTable.getTableConstraints());
assertEquals(1, fetchedTable.getTableConstraints().size());
}
}
Loading