Skip to content

Commit 9856d0e

Browse files
fudianchnfudianchn
authored
fix(feature): keep generic fallback references semantic (#2755)
Signed-off-by: 付典 <fudianchn@gmail.com>
1 parent 699b742 commit 9856d0e

4 files changed

Lines changed: 182 additions & 6 deletions

File tree

‎src/main/java/net/sf/jsqlparser/statement/StatementFeatureVisitor.java‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ public static StatementFeatures analyse(Statement statement, Predicate<String> p
190190
Analysis analysis = new Analysis(pureFunctions);
191191
StatementFeatureVisitor visitor = new StatementFeatureVisitor(analysis);
192192
statement.accept(visitor, null);
193-
analysis.failLoudIfSilent(statement.getClass().getSimpleName());
193+
analysis.failLoudIfSilent();
194194
return new StatementFeatures(analysis.certain, analysis.uncertain, analysis.unresolved);
195195
}
196196

@@ -223,7 +223,7 @@ public static StatementFeatures analyse(Statements statements,
223223
Analysis analysis = new Analysis(pureFunctions);
224224
StatementFeatureVisitor visitor = new StatementFeatureVisitor(analysis);
225225
statements.accept(visitor, null);
226-
analysis.failLoudIfSilent(Statements.class.getSimpleName());
226+
analysis.failLoudIfSilent();
227227
return new StatementFeatures(analysis.certain, analysis.uncertain, analysis.unresolved);
228228
}
229229

@@ -313,10 +313,14 @@ void suppressReads(Runnable body) {
313313
}
314314

315315
void opaque(String reference) {
316+
opaque();
317+
unresolved(reference);
318+
}
319+
320+
void opaque() {
316321
certain.add(StmtFeature.OPAQUE);
317322
uncertain.addAll(EnumSet.of(StmtFeature.READS_DATA, StmtFeature.RETURNS_RESULT_SET,
318323
StmtFeature.MODIFIES_DATA, StmtFeature.MODIFIES_SCHEMA));
319-
unresolved(reference);
320324
}
321325

322326
/**
@@ -326,9 +330,9 @@ void opaque(String reference) {
326330
* hand-maintained type table; the price is that a genuinely inert statement is also
327331
* reported opaque, which is the right direction to be wrong in.
328332
*/
329-
void failLoudIfSilent(String label) {
333+
void failLoudIfSilent() {
330334
if (certain.isEmpty()) {
331-
opaque(label);
335+
opaque();
332336
}
333337
}
334338
}

‎src/main/java/net/sf/jsqlparser/statement/StatementFeatures.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ public Set<StmtFeature> getUncertain() {
7373
/**
7474
* Why the analysis is uncertain: unresolved function names, dynamic SQL markers, called
7575
* procedure names. Resolve these against your own catalogue or allow-list.
76+
*
77+
* <p>
78+
* The generic fallback for an unclassified statement adds no Java type name. It can report
79+
* {@link StmtFeature#OPAQUE} with an empty reference set; an empty set does not prove safety.
80+
* Use {@link #isOpaque()} and {@link #may(StmtFeature)} to inspect the verdict. Explicit
81+
* markers remain, including the statement-type markers for stored routine declarations.
7682
*/
7783
public Set<String> getUnresolvedReferences() {
7884
return unresolved;

‎src/site/sphinx/usage.rst‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ Convenience methods wrap the common combinations:
574574
features.modifiesSchema(); // is(MODIFIES_SCHEMA)
575575
features.isOpaque(); // CALL, EXECUTE, dynamic SQL
576576
577-
When something is merely *possible*, the analysis tells you **why**, so you can resolve it against your own catalogue or allow-list rather than guessing:
577+
When the analysis identifies an unresolved function, procedure or dynamic SQL marker, it records the reference so you can consult your own catalogue or allow-list:
578578

579579
.. code-block:: java
580580
:caption: Safeguarding a read-only connection
@@ -587,6 +587,10 @@ When something is merely *possible*, the analysis tells you **why**, so you can
587587
// e.g. [nextval]
588588
}
589589
590+
The generic fallback for an unclassified statement preserves ``OPAQUE`` and the possible effects ``READS_DATA``, ``RETURNS_RESULT_SET``, ``MODIFIES_DATA`` and ``MODIFIES_SCHEMA``, but does not add a Java statement or container class name to ``getUnresolvedReferences()``. The reference set can therefore be empty while the effects remain unknown. Check ``isOpaque()`` and ``may(..)``; an empty reference set alone does not establish safety.
591+
592+
Existing explicit references and markers remain, including procedure and function names, ``do``, ``unsupported`` and ``explain``. Stored routine declarations also retain their existing statement-type markers, such as ``createfunction``; these are independent of the generic fallback.
593+
590594
If you can prove some functions side-effect free, hand in a predicate and the uncertainty collapses:
591595

592596
.. code-block:: java

‎src/test/java/net/sf/jsqlparser/statement/StatementFeatureFallbackTest.java‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
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.statement;
11+
12+
import static org.assertj.core.api.Assertions.assertThat;
13+
14+
import java.util.List;
15+
import net.sf.jsqlparser.expression.Function;
16+
import net.sf.jsqlparser.expression.LongValue;
17+
import net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect;
18+
import net.sf.jsqlparser.parser.CCJSqlParserUtil;
19+
import net.sf.jsqlparser.schema.Column;
20+
import net.sf.jsqlparser.statement.create.function.CreateFunction;
21+
import net.sf.jsqlparser.statement.oracle.OracleAssignment;
22+
import net.sf.jsqlparser.statement.oracle.OracleNullStatement;
23+
import org.junit.jupiter.api.Test;
24+
import org.junit.jupiter.params.ParameterizedTest;
25+
import org.junit.jupiter.params.provider.ValueSource;
26+
27+
class StatementFeatureFallbackTest {
28+
private static final class UnclassifiedStatement implements Statement {
29+
@Override
30+
public <T, S> T accept(StatementVisitor<T> visitor, S context) {
31+
return null;
32+
}
33+
}
34+
35+
private static void assertOpaque(StatementFeatures features, String... references) {
36+
assertThat(features.getCertain()).containsExactly(StmtFeature.OPAQUE);
37+
assertThat(features.getUncertain()).containsExactlyInAnyOrder(StmtFeature.READS_DATA,
38+
StmtFeature.RETURNS_RESULT_SET, StmtFeature.MODIFIES_DATA,
39+
StmtFeature.MODIFIES_SCHEMA);
40+
assertThat(features.getUnresolvedReferences()).containsExactlyInAnyOrder(references);
41+
}
42+
43+
@Test
44+
void unclassifiedStatementHasNoCatalogueReference() {
45+
Statement statement = new UnclassifiedStatement();
46+
assertOpaque(statement.getFeatures());
47+
assertOpaque(statement.getFeatures(name -> true));
48+
assertOpaque(StatementFeatureVisitor.analyse(statement));
49+
}
50+
51+
@Test
52+
void anonymousStatementHasNoEmptyReference() {
53+
Statement statement = new Statement() {
54+
@Override
55+
public <T, S> T accept(StatementVisitor<T> visitor, S context) {
56+
return null;
57+
}
58+
};
59+
assertOpaque(statement.getFeatures());
60+
}
61+
62+
@Test
63+
void inertOracleStatementRetainsConservativeFallback() {
64+
assertOpaque(new OracleNullStatement().getFeatures());
65+
assertOpaque(new OracleAssignment(new Column("x"), new LongValue(1)).getFeatures());
66+
}
67+
68+
@Test
69+
void fallbackPreservesFunctionReferenceFoundBeforeClassification() {
70+
Function function = new Function();
71+
function.setName("Unproven_Function");
72+
Statement assignment = new OracleAssignment(new Column("x"), function);
73+
assertOpaque(assignment.getFeatures(), "unproven_function");
74+
assertOpaque(assignment.getFeatures(name -> name.equals("unproven_function")));
75+
}
76+
77+
@ParameterizedTest
78+
@ValueSource(ints = {0, 1, 2})
79+
void aggregateFallbackHasNoContainerReference(int count) {
80+
Statements script = new Statements();
81+
for (int i = 0; i < count; i++) {
82+
script.add(new UnclassifiedStatement());
83+
}
84+
assertOpaque(StatementFeatureVisitor.analyse(script));
85+
assertOpaque(StatementFeatureVisitor.analyse(script, name -> true));
86+
List<StatementFeatures> each = StatementFeatureVisitor.analyseEach(script);
87+
assertThat(each).hasSize(count);
88+
each.forEach(StatementFeatureFallbackTest::assertOpaque);
89+
}
90+
91+
@Test
92+
void perStatementAnalysisDoesNotLeakReferencesOrResultPositions() throws Exception {
93+
Statements script = new Statements();
94+
script.add(CCJSqlParserUtil.parse("CALL Do_Something(1)"));
95+
script.add(new UnclassifiedStatement());
96+
script.add(CCJSqlParserUtil.parse("SELECT 1"));
97+
List<StatementFeatures> each = StatementFeatureVisitor.analyseEach(script);
98+
assertOpaque(each.get(0), "do_something");
99+
assertOpaque(each.get(1));
100+
assertThat(each.get(2).getCertain()).containsExactly(StmtFeature.RETURNS_RESULT_SET);
101+
assertThat(each.get(2).getUncertain()).isEmpty();
102+
assertThat(each.get(2).getUnresolvedReferences()).isEmpty();
103+
StatementFeatures union = StatementFeatureVisitor.analyse(script);
104+
assertThat(union.getCertain()).containsExactlyInAnyOrder(StmtFeature.OPAQUE,
105+
StmtFeature.RETURNS_RESULT_SET);
106+
assertThat(union.getUncertain()).containsExactlyInAnyOrder(StmtFeature.READS_DATA,
107+
StmtFeature.MODIFIES_DATA, StmtFeature.MODIFIES_SCHEMA);
108+
assertThat(union.getUnresolvedReferences()).containsExactly("do_something");
109+
}
110+
111+
@ParameterizedTest
112+
@ValueSource(strings = {"SELECT 1", "CREATE TABLE t (id INTEGER)"})
113+
void classifiedStatementsDoNotFallBack(String sql) throws Exception {
114+
StatementFeatures features = CCJSqlParserUtil.parse(sql).getFeatures();
115+
assertThat(features.getCertain()).containsExactly(sql.startsWith("SELECT")
116+
? StmtFeature.RETURNS_RESULT_SET
117+
: StmtFeature.MODIFIES_SCHEMA);
118+
assertThat(features.getUncertain()).isEmpty();
119+
assertThat(features.getUnresolvedReferences()).isEmpty();
120+
}
121+
122+
@Test
123+
void procedureAndDynamicSqlMarkersRemainResolvable() throws Exception {
124+
assertOpaque(CCJSqlParserUtil.parse("CALL Do_Something(1)").getFeatures(), "do_something");
125+
assertOpaque(CCJSqlParserUtil.parse("DO 'BEGIN NULL; END'",
126+
parser -> parser.withDialect(Dialect.POSTGRESQL)).getFeatures(), "do");
127+
assertOpaque(CCJSqlParserUtil.parse("some unsupported sql",
128+
parser -> parser.withUnsupportedStatements(true)).getFeatures(), "unsupported");
129+
}
130+
131+
@Test
132+
void explainMarkerRemainsExplicit() throws Exception {
133+
StatementFeatures features = CCJSqlParserUtil.parse("EXPLAIN SELECT 1").getFeatures();
134+
assertThat(features.getCertain()).containsExactly(StmtFeature.RETURNS_RESULT_SET);
135+
assertThat(features.getUncertain()).containsExactlyInAnyOrder(StmtFeature.MODIFIES_DATA,
136+
StmtFeature.READS_DATA);
137+
assertThat(features.getUnresolvedReferences()).containsExactly("explain");
138+
}
139+
140+
@Test
141+
void selectFunctionReferenceAndAllowListRemainEffective() throws Exception {
142+
Statement statement = CCJSqlParserUtil.parse("SELECT Unproven_Function(1)");
143+
StatementFeatures features = statement.getFeatures();
144+
assertThat(features.getCertain()).containsExactly(StmtFeature.RETURNS_RESULT_SET);
145+
assertThat(features.getUncertain()).containsExactlyInAnyOrder(StmtFeature.MODIFIES_DATA,
146+
StmtFeature.MODIFIES_SCHEMA);
147+
assertThat(features.getUnresolvedReferences()).containsExactly("unproven_function");
148+
StatementFeatures pure = statement.getFeatures(name -> name.equals("unproven_function"));
149+
assertThat(pure.getCertain()).containsExactly(StmtFeature.RETURNS_RESULT_SET);
150+
assertThat(pure.getUncertain()).isEmpty();
151+
assertThat(pure.getUnresolvedReferences()).isEmpty();
152+
}
153+
154+
@Test
155+
void storedRoutineExplicitMarkerIsOutsideGenericFallback() {
156+
StatementFeatures features = new CreateFunction().getFeatures();
157+
assertThat(features.getCertain()).containsExactly(StmtFeature.MODIFIES_SCHEMA);
158+
assertThat(features.getUncertain()).containsExactlyInAnyOrder(StmtFeature.MODIFIES_DATA,
159+
StmtFeature.READS_DATA);
160+
assertThat(features.getUnresolvedReferences()).containsExactly("createfunction");
161+
}
162+
}

0 commit comments

Comments
 (0)