Skip to content

Commit ef9dc08

Browse files
minleejaeminleejae
authored
Validate ALTER index names according to their roles (#2776)
Signed-off-by: minleejae <mmj9808@gmail.com>
1 parent 2c0359a commit ef9dc08

2 files changed

Lines changed: 202 additions & 8 deletions

File tree

‎src/main/java/net/sf/jsqlparser/util/validation/validator/AlterValidator.java‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import net.sf.jsqlparser.statement.create.table.ForeignKeyConstraint;
2727
import net.sf.jsqlparser.statement.create.table.ConstraintKind;
2828
import net.sf.jsqlparser.statement.create.table.KeyConstraint;
29+
import net.sf.jsqlparser.statement.create.table.Index;
2930
import net.sf.jsqlparser.util.TableDefinitionTraversal;
3031
import net.sf.jsqlparser.util.validation.ValidationCapability;
3132
import net.sf.jsqlparser.util.validation.ValidationUtil;
@@ -80,14 +81,7 @@ public void validate(Alter alter, AlterExpression e) {
8081
}
8182

8283
validateConstraint(c, e.getConstraint());
83-
if (e.getIndex() != null) {
84-
validateName(c, NamedObject.index, e.getIndex().getName());
85-
if (e.getIndex().getColumns() != null) {
86-
validateOptionalColumnNames(c, e.getIndex().getColumns().stream()
87-
.filter(key -> !key.isExpression()).map(KeyElement::getColumnName)
88-
.collect(toList()), NamedObject.index);
89-
}
90-
}
84+
validateIndex(c, e);
9185
}
9286
}
9387

@@ -135,5 +129,23 @@ private boolean hasActiveDropColumns(AlterExpression expression) {
135129
}
136130
}
137131

132+
private void validateIndex(ValidationCapability capability, AlterExpression expression) {
133+
Index index = expression.getIndex();
134+
if (index == null) {
135+
return;
136+
}
137+
boolean rename = expression.getOperation() == AlterOperation.RENAME_INDEX
138+
|| expression.getOperation() == AlterOperation.RENAME_KEY;
139+
if (rename && expression.getOldIndex() != null) {
140+
validateName(capability, NamedObject.index, expression.getOldIndex().getName());
141+
}
142+
boolean introducedName = expression.getOperation() == AlterOperation.ADD || rename;
143+
validateOptionalName(capability, NamedObject.index, index.getName(), null, !introducedName);
144+
if (index.getColumns() != null) {
145+
validateOptionalColumnNames(capability, index.getColumns().stream()
146+
.filter(key -> !key.isExpression()).map(KeyElement::getColumnName)
147+
.collect(toList()), NamedObject.index);
148+
}
149+
}
138150

139151
}

‎src/test/java/net/sf/jsqlparser/util/validation/validator/AlterIndexValidationTest.java‎

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
/*-
2+
* #%L
3+
* JSQLParser library
4+
* %%
5+
* Copyright (C) 2004 - 2026 JSQLParser
6+
* %%
7+
* Dual licensed under GNU LGPL 2.1 or Apache License 2.0
8+
* #L%
9+
*/
10+
package net.sf.jsqlparser.util.validation.validator;
11+
12+
import static org.junit.jupiter.api.Assertions.assertEquals;
13+
import static org.junit.jupiter.api.Assertions.assertNull;
14+
import static org.junit.jupiter.api.Assertions.assertTrue;
15+
16+
import java.util.ArrayList;
17+
import java.util.List;
18+
import java.util.Map;
19+
import java.util.Set;
20+
import net.sf.jsqlparser.JSQLParserException;
21+
import net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect;
22+
import net.sf.jsqlparser.parser.CCJSqlParserUtil;
23+
import net.sf.jsqlparser.parser.feature.FeatureConfiguration;
24+
import net.sf.jsqlparser.schema.Table;
25+
import net.sf.jsqlparser.statement.alter.Alter;
26+
import net.sf.jsqlparser.statement.alter.AlterExpression;
27+
import net.sf.jsqlparser.statement.alter.AlterOperation;
28+
import net.sf.jsqlparser.statement.create.table.Index;
29+
import net.sf.jsqlparser.util.validation.Validation;
30+
import net.sf.jsqlparser.util.validation.ValidationCapability;
31+
import net.sf.jsqlparser.util.validation.ValidationException;
32+
import net.sf.jsqlparser.util.validation.metadata.DatabaseMetaDataValidation;
33+
import net.sf.jsqlparser.util.validation.metadata.Named;
34+
import net.sf.jsqlparser.util.validation.metadata.NamedObject;
35+
import org.junit.jupiter.api.Test;
36+
import org.junit.jupiter.params.ParameterizedTest;
37+
import org.junit.jupiter.params.provider.ValueSource;
38+
39+
class AlterIndexValidationTest {
40+
private static final Named CHILD = new Named(NamedObject.table, "child");
41+
private static final Named ID = new Named(NamedObject.column, "id");
42+
private static final Named INDEX = new Named(NamedObject.index, "ix");
43+
44+
@ParameterizedTest
45+
@ValueSource(strings = {"INDEX", "KEY", "FULLTEXT INDEX", "SPATIAL INDEX"})
46+
void addIndexRequiresANewNameAndExistingColumns(String type) throws JSQLParserException {
47+
Alter statement = parse("ALTER TABLE child ADD " + type + " ix (id)", Dialect.MYSQL);
48+
List<Named> visited = new ArrayList<>();
49+
Map<ValidationCapability, Set<ValidationException>> errors =
50+
validate(statement, Set.of(CHILD, ID), visited);
51+
assertTrue(errors.isEmpty(), errors.toString());
52+
assertEquals(List.of(CHILD, INDEX, ID), visited);
53+
54+
errors = validate(statement, Set.of(CHILD, ID, INDEX), new ArrayList<>());
55+
assertEquals(1, errors.size());
56+
assertTrue(errors.toString().contains("ix does exist"), errors.toString());
57+
58+
errors = validate(statement, Set.of(CHILD), new ArrayList<>());
59+
assertEquals(1, errors.size());
60+
assertTrue(errors.toString().contains("id does not exist"), errors.toString());
61+
}
62+
63+
@ParameterizedTest
64+
@ValueSource(strings = {"INDEX (id)", "KEY (id)", "INDEX ((id + 1))"})
65+
void unnamedIndexesSkipNameLookupButKeepColumnValidation(String definition)
66+
throws JSQLParserException {
67+
Alter statement = parse("ALTER TABLE child ADD " + definition, Dialect.MYSQL);
68+
assertNull(statement.getAlterExpressions().get(0).getIndex().getName());
69+
List<Named> visited = new ArrayList<>();
70+
Map<ValidationCapability, Set<ValidationException>> errors =
71+
validate(statement, Set.of(CHILD, ID), visited);
72+
assertTrue(errors.isEmpty(), errors.toString());
73+
assertEquals(List.of(CHILD, ID), visited);
74+
errors = validate(statement, Set.of(CHILD), new ArrayList<>());
75+
assertTrue(errors.toString().contains("id does not exist"), errors.toString());
76+
}
77+
78+
@Test
79+
void namedExpressionIndexesValidateTheirExpressionColumnsOnce() throws JSQLParserException {
80+
Alter statement = parse("ALTER TABLE child ADD INDEX ix ((id + 1))", Dialect.MYSQL);
81+
List<Named> visited = new ArrayList<>();
82+
Map<ValidationCapability, Set<ValidationException>> errors =
83+
validate(statement, Set.of(CHILD, ID), visited);
84+
assertTrue(errors.isEmpty(), errors.toString());
85+
assertEquals(List.of(CHILD, ID, INDEX), visited);
86+
errors = validate(statement, Set.of(CHILD), new ArrayList<>());
87+
assertTrue(errors.toString().contains("id does not exist"), errors.toString());
88+
}
89+
90+
@Test
91+
void quotedNamesReachTheMetadataContractUnchanged() throws JSQLParserException {
92+
Alter statement = parse("ALTER TABLE `child table` ADD INDEX `new index` (`id.column`)",
93+
Dialect.MYSQL);
94+
Named table = new Named(NamedObject.table, "`child table`");
95+
Named column = new Named(NamedObject.column, "`id.column`");
96+
Named index = new Named(NamedObject.index, "`new index`");
97+
List<Named> visited = new ArrayList<>();
98+
Map<ValidationCapability, Set<ValidationException>> errors =
99+
validate(statement, Set.of(table, column), visited);
100+
assertTrue(errors.isEmpty(), errors.toString());
101+
assertEquals(List.of(table, index, column), visited);
102+
errors = validate(statement, Set.of(table, column, index), new ArrayList<>());
103+
assertTrue(errors.toString().contains("`new index` does exist"), errors.toString());
104+
}
105+
106+
@ParameterizedTest
107+
@ValueSource(strings = {"INDEX", "KEY"})
108+
void dropIndexStillRequiresAnExistingName(String keyword) throws JSQLParserException {
109+
Alter statement = parse("ALTER TABLE child DROP " + keyword + " ix", Dialect.MYSQL);
110+
List<Named> visited = new ArrayList<>();
111+
Map<ValidationCapability, Set<ValidationException>> errors =
112+
validate(statement, Set.of(CHILD, INDEX), visited);
113+
assertTrue(errors.isEmpty(), errors.toString());
114+
assertEquals(List.of(CHILD, INDEX), visited);
115+
errors = validate(statement, Set.of(CHILD), new ArrayList<>());
116+
assertTrue(errors.toString().contains("ix does not exist"), errors.toString());
117+
}
118+
119+
@ParameterizedTest
120+
@ValueSource(strings = {"INDEX", "KEY"})
121+
void renameRequiresTheOldNameAndIntroducesTheNewName(String keyword)
122+
throws JSQLParserException {
123+
Alter statement = parse("ALTER TABLE child RENAME " + keyword
124+
+ " `old index` TO `new index`", Dialect.MYSQL);
125+
Named oldIndex = new Named(NamedObject.index, "`old index`");
126+
Named newIndex = new Named(NamedObject.index, "`new index`");
127+
List<Named> visited = new ArrayList<>();
128+
Map<ValidationCapability, Set<ValidationException>> errors =
129+
validate(statement, Set.of(CHILD, oldIndex), visited);
130+
assertTrue(errors.isEmpty(), errors.toString());
131+
assertEquals(List.of(CHILD, oldIndex, newIndex), visited);
132+
133+
errors = validate(statement, Set.of(CHILD), new ArrayList<>());
134+
assertTrue(errors.toString().contains("`old index` does not exist"), errors.toString());
135+
errors = validate(statement, Set.of(CHILD, oldIndex, newIndex), new ArrayList<>());
136+
assertTrue(errors.toString().contains("`new index` does exist"), errors.toString());
137+
}
138+
139+
@ParameterizedTest
140+
@ValueSource(strings = {"PRIMARY KEY", "UNIQUE"})
141+
void constraintUsingIndexStillReferencesAnExistingIndex(String type)
142+
throws JSQLParserException {
143+
Alter statement = parse("ALTER TABLE child ADD CONSTRAINT c " + type + " USING INDEX ix",
144+
Dialect.POSTGRESQL);
145+
assertNull(statement.getAlterExpressions().get(0).getIndex());
146+
List<Named> visited = new ArrayList<>();
147+
Map<ValidationCapability, Set<ValidationException>> errors =
148+
validate(statement, Set.of(CHILD, INDEX), visited);
149+
assertTrue(errors.isEmpty(), errors.toString());
150+
assertEquals(List.of(CHILD, new Named(NamedObject.constraint, "c"), INDEX), visited);
151+
errors = validate(statement, Set.of(CHILD), new ArrayList<>());
152+
assertTrue(errors.toString().contains("ix does not exist"), errors.toString());
153+
}
154+
155+
@Test
156+
void buildersUseTheSameDefinitionRole() {
157+
Alter statement = new Alter();
158+
statement.setTable(new Table("child"));
159+
statement.addAlterExpression(new AlterExpression().withOperation(AlterOperation.ADD)
160+
.withIndex(new Index().withType("INDEX").withName("ix")
161+
.withColumnsNames(List.of("id"))));
162+
List<Named> visited = new ArrayList<>();
163+
Map<ValidationCapability, Set<ValidationException>> errors =
164+
validate(statement, Set.of(CHILD, ID), visited);
165+
assertTrue(errors.isEmpty(), errors.toString());
166+
assertEquals(List.of(CHILD, INDEX, ID), visited);
167+
}
168+
169+
private static Alter parse(String sql, Dialect dialect) throws JSQLParserException {
170+
return (Alter) CCJSqlParserUtil.parse(sql, parser -> parser.withDialect(dialect));
171+
}
172+
173+
private static Map<ValidationCapability, Set<ValidationException>> validate(Alter statement,
174+
Set<Named> existing, List<Named> visited) {
175+
DatabaseMetaDataValidation metadata = name -> {
176+
visited.add(name);
177+
return existing.contains(name);
178+
};
179+
return Validation.validate(statement, Validation.createValidationContext(
180+
new FeatureConfiguration(), List.of(metadata)));
181+
}
182+
}

0 commit comments

Comments
 (0)