Skip to content

Commit 01d7c3e

Browse files
authored
Analyse analytic and conversion functions with complete window traversal (#2579)
1 parent db61f4b commit 01d7c3e

6 files changed

Lines changed: 235 additions & 40 deletions

File tree

src/main/java/net/sf/jsqlparser/expression/AnalyticExpression.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public class AnalyticExpression extends ASTNodeAccessImpl implements Expression
5858
public AnalyticExpression() {}
5959

6060
public AnalyticExpression(Function function) {
61-
this.name = String.join(" ", function.getMultipartName());
61+
this.name = function.getName();
6262
this.allColumns = function.isAllColumns();
6363
this.distinct = function.isDistinct();
6464
this.unique = function.isUnique();

src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
import java.util.ArrayList;
1313
import java.util.Arrays;
1414
import java.util.Collection;
15+
import java.util.List;
1516
import java.util.Map;
16-
import java.util.Optional;
1717
import net.sf.jsqlparser.expression.operators.arithmetic.Addition;
1818
import net.sf.jsqlparser.expression.operators.arithmetic.BitwiseAnd;
1919
import net.sf.jsqlparser.expression.operators.arithmetic.BitwiseLeftShift;
@@ -119,11 +119,9 @@ public <S> T visit(Function function, S context) {
119119
if (function.getKeep() != null) {
120120
subExpressions.add(function.getKeep());
121121
}
122-
if (function.getOrderByElements() != null) {
123-
for (OrderByElement orderByElement : function.getOrderByElements()) {
124-
subExpressions.add(orderByElement.getExpression());
125-
}
126-
}
122+
addOrderByExpressions(subExpressions, function.getOrderByElements());
123+
addFunctionModifiers(subExpressions, function.getHavingClause(),
124+
function.getKeywordArguments(), function.getLimit());
127125
return visitExpressions(function, context, subExpressions);
128126
}
129127

@@ -419,29 +417,41 @@ public <S> T visit(AnalyticExpression analyticExpression, S context) {
419417
if (analyticExpression.getKeep() != null) {
420418
subExpressions.add(analyticExpression.getKeep());
421419
}
422-
if (analyticExpression.getFuncOrderBy() != null) {
423-
for (OrderByElement element : analyticExpression.getOrderByElements()) {
424-
subExpressions.add(element.getExpression());
420+
subExpressions.add(analyticExpression.getFilterExpression());
421+
addOrderByExpressions(subExpressions, analyticExpression.getFuncOrderBy());
422+
addFunctionModifiers(subExpressions, analyticExpression.getHavingClause(),
423+
analyticExpression.getKeywordArguments(), analyticExpression.getLimit());
424+
if (analyticExpression.getWindowDefinition() != null) {
425+
subExpressions.addAll(analyticExpression.getWindowDefinition().getAllExpressions());
426+
}
427+
return visitExpressions(analyticExpression, context, subExpressions);
428+
}
429+
430+
private static void addOrderByExpressions(List<Expression> expressions,
431+
List<OrderByElement> orderBy) {
432+
if (orderBy != null) {
433+
for (OrderByElement element : orderBy) {
434+
expressions.add(element.getExpression());
425435
}
426436
}
427-
if (analyticExpression.getWindowElement() != null) {
428-
/*
429-
* Visit expressions from the range and offset of the window element. Do this using
430-
* optional chains, because several things down the tree can be null e.g. the
431-
* expression. So, null-safe versions of e.g.:
432-
* analyticExpression.getWindowElement().getOffset().getExpression().accept(this,
433-
* parameters);
434-
*/
435-
Optional.ofNullable(analyticExpression.getWindowElement().getRange())
436-
.map(WindowRange::getStart)
437-
.map(WindowOffset::getExpression).ifPresent(subExpressions::add);
438-
Optional.ofNullable(analyticExpression.getWindowElement().getRange())
439-
.map(WindowRange::getEnd)
440-
.map(WindowOffset::getExpression).ifPresent(subExpressions::add);
441-
Optional.ofNullable(analyticExpression.getWindowElement().getOffset())
442-
.map(WindowOffset::getExpression).ifPresent(subExpressions::add);
437+
}
438+
439+
private static void addFunctionModifiers(List<Expression> expressions,
440+
Function.HavingClause having, List<Function.KeywordArgument> arguments,
441+
net.sf.jsqlparser.statement.select.Limit limit) {
442+
expressions.add(having);
443+
if (arguments != null) {
444+
for (Function.KeywordArgument argument : arguments) {
445+
expressions.add(argument.getExpression());
446+
}
447+
}
448+
if (limit != null) {
449+
expressions.add(limit.getOffset());
450+
expressions.add(limit.getRowCount());
451+
if (limit.getByExpressions() != null) {
452+
expressions.addAll(limit.getByExpressions());
453+
}
443454
}
444-
return visitExpressions(analyticExpression, context, subExpressions);
445455
}
446456

447457
@Override

src/main/java/net/sf/jsqlparser/expression/WindowDefinition.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
package net.sf.jsqlparser.expression;
1111

1212
import java.io.Serializable;
13+
import java.util.ArrayList;
1314
import java.util.List;
1415

1516
import net.sf.jsqlparser.expression.operators.relational.ExpressionList;
@@ -73,6 +74,30 @@ public WindowDefinition withWindowName(String windowName) {
7374
return this;
7475
}
7576

77+
/** Returns the partition, order and frame expressions for both inline and named windows. */
78+
public List<Expression> getAllExpressions() {
79+
List<Expression> expressions = new ArrayList<>(partitionBy);
80+
if (getOrderByElements() != null) {
81+
for (OrderByElement element : getOrderByElements()) {
82+
expressions.add(element.getExpression());
83+
}
84+
}
85+
if (windowElement != null) {
86+
if (windowElement.getRange() != null) {
87+
addOffsetExpression(expressions, windowElement.getRange().getStart());
88+
addOffsetExpression(expressions, windowElement.getRange().getEnd());
89+
}
90+
addOffsetExpression(expressions, windowElement.getOffset());
91+
}
92+
return expressions;
93+
}
94+
95+
private static void addOffsetExpression(List<Expression> expressions, WindowOffset offset) {
96+
if (offset != null && offset.getExpression() != null) {
97+
expressions.add(offset.getExpression());
98+
}
99+
}
100+
76101
@Override
77102
public String toString() {
78103
StringBuilder b = new StringBuilder();

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

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import net.sf.jsqlparser.statement.alter.AlterSubscription;
2828

2929
import net.sf.jsqlparser.JSQLParserException;
30+
import net.sf.jsqlparser.expression.AnalyticExpression;
31+
import net.sf.jsqlparser.expression.TranscodingFunction;
3032
import net.sf.jsqlparser.expression.Expression;
3133
import net.sf.jsqlparser.expression.ExpressionVisitor;
3234
import net.sf.jsqlparser.expression.ExpressionVisitorAdapter;
@@ -829,22 +831,32 @@ static final class FeatureExpressionVisitor extends ExpressionVisitorAdapter<Voi
829831
this.analysis = analysis;
830832
}
831833

832-
/**
833-
* Volatility is not a syntactic property. Everything the caller has not proven pure stays
834-
* in the <em>possible</em> set, with the name recorded so it can be resolved against a
835-
* catalogue rather than guessed at here.
836-
*/
837-
@Override
838-
public <S> Void visit(Function function, S context) {
839-
String name = function.getName() == null
840-
? "?"
841-
: function.getName().toLowerCase(Locale.ROOT);
834+
/** Records unproven functions consistently across their different expression models. */
835+
private void analyseFunction(String functionName) {
836+
String name = functionName == null ? "?" : functionName.toLowerCase(Locale.ROOT);
842837
if (!analysis.pureFunctions.test(name)) {
843838
analysis.possible(StmtFeature.MODIFIES_DATA, StmtFeature.MODIFIES_SCHEMA);
844839
analysis.unresolved(name);
845840
}
841+
}
842+
843+
@Override
844+
public <S> Void visit(Function function, S context) {
845+
analyseFunction(function.getName());
846846
return super.visit(function, context);
847847
}
848+
849+
@Override
850+
public <S> Void visit(AnalyticExpression expression, S context) {
851+
analyseFunction(expression.getName());
852+
return super.visit(expression, context);
853+
}
854+
855+
@Override
856+
public <S> Void visit(TranscodingFunction expression, S context) {
857+
analyseFunction(expression.getKeyword());
858+
return super.visit(expression, context);
859+
}
848860
}
849861

850862
static final class FeatureFromItemVisitor extends FromItemVisitorAdapter<Void> {

src/main/java/net/sf/jsqlparser/statement/select/SelectVisitorAdapter.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
import net.sf.jsqlparser.expression.ExpressionVisitor;
1414
import net.sf.jsqlparser.expression.ExpressionVisitorAdapter;
1515
import net.sf.jsqlparser.expression.Function;
16+
import net.sf.jsqlparser.expression.Expression;
17+
import net.sf.jsqlparser.expression.WindowDefinition;
1618
import net.sf.jsqlparser.statement.OutputClause;
1719
import net.sf.jsqlparser.statement.ParenthesedStatement;
1820
import net.sf.jsqlparser.statement.Statement;
@@ -198,9 +200,13 @@ public <S> T visit(PlainSelect plainSelect, S context) {
198200
expressionVisitor.visitExpression(plainSelect.getHaving(), context);
199201
expressionVisitor.visitExpression(plainSelect.getQualify(), context);
200202

201-
// if (plainSelect.getWindowDefinitions() != null) {
202-
// //@todo: implement
203-
// }
203+
if (plainSelect.getWindowDefinitions() != null) {
204+
for (WindowDefinition window : plainSelect.getWindowDefinitions()) {
205+
for (Expression expression : window.getAllExpressions()) {
206+
expressionVisitor.visitExpression(expression, context);
207+
}
208+
}
209+
}
204210

205211
Pivot pivot = plainSelect.getPivot();
206212
if (pivot != null) {
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
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.expression;
11+
12+
import static org.assertj.core.api.Assertions.assertThat;
13+
14+
import java.util.ArrayList;
15+
import java.util.Collection;
16+
import java.util.List;
17+
import java.util.Set;
18+
import java.util.stream.Stream;
19+
import net.sf.jsqlparser.JSQLParserException;
20+
import net.sf.jsqlparser.parser.CCJSqlParserUtil;
21+
import net.sf.jsqlparser.statement.Statement;
22+
import net.sf.jsqlparser.statement.StatementFeatures;
23+
import net.sf.jsqlparser.statement.StmtFeature;
24+
import net.sf.jsqlparser.statement.select.Limit;
25+
import net.sf.jsqlparser.statement.select.PlainSelect;
26+
import net.sf.jsqlparser.statement.select.SelectItem;
27+
import net.sf.jsqlparser.statement.select.SelectVisitorAdapter;
28+
import org.junit.jupiter.api.Test;
29+
import org.junit.jupiter.params.ParameterizedTest;
30+
import org.junit.jupiter.params.provider.Arguments;
31+
import org.junit.jupiter.params.provider.MethodSource;
32+
import org.junit.jupiter.params.provider.ValueSource;
33+
34+
class FunctionFeatureAnalysisTest {
35+
static Stream<Arguments> functionForms() {
36+
return Stream.of(
37+
Arguments.of("SELECT pg_sleep(1) FROM t", "pg_sleep"),
38+
Arguments.of("SELECT pg_sleep(1) OVER () FROM t", "pg_sleep"),
39+
Arguments.of("SELECT SuM(v) OVER () FROM t", "sum"),
40+
Arguments.of("SELECT analytics.sum(v) OVER () FROM t", "analytics.sum"),
41+
Arguments.of("SELECT CONVERT(v USING utf8) FROM t", "convert"),
42+
Arguments.of("SELECT CONVERT(INT, v) FROM t", "convert"),
43+
Arguments.of("SELECT TRY_CONVERT(INT, v) FROM t", "try_convert"));
44+
}
45+
46+
@ParameterizedTest
47+
@MethodSource("functionForms")
48+
void appliesTheSamePurityContractToEachFunctionForm(String sql, String name)
49+
throws JSQLParserException {
50+
Statement statement = CCJSqlParserUtil.parse(sql);
51+
String before = statement.toString();
52+
StatementFeatures unknown = statement.getFeatures(n -> false);
53+
assertThat(unknown.getUnresolvedReferences()).containsExactly(name);
54+
assertThat(unknown.getUncertain())
55+
.contains(StmtFeature.MODIFIES_DATA, StmtFeature.MODIFIES_SCHEMA);
56+
assertThat(unknown.modifiesData()).isFalse();
57+
assertThat(unknown.mayModifyData()).isTrue();
58+
59+
StatementFeatures pure = statement.getFeatures(name::equals);
60+
assertThat(pure.getUnresolvedReferences()).isEmpty();
61+
assertThat(pure.mayModifyData()).isFalse();
62+
assertThat(statement.toString()).isEqualTo(before);
63+
assertThat(CCJSqlParserUtil.parse(before).toString()).isEqualTo(before);
64+
}
65+
66+
@ParameterizedTest
67+
@ValueSource(strings = {
68+
"SELECT sum(danger(v)) OVER () FROM t",
69+
"SELECT sum(v) OVER (PARTITION BY danger(k)) FROM t",
70+
"SELECT sum(v) OVER (ORDER BY danger(k)) FROM t",
71+
"SELECT sum(v) FILTER (WHERE danger(k) > 0) OVER () FROM t",
72+
"SELECT array_agg(v ORDER BY danger(k)) OVER () FROM t",
73+
"SELECT lag(v, danger(k), 0) OVER () FROM t",
74+
"SELECT lag(v, 1, danger(k)) OVER () FROM t",
75+
"SELECT sum(v) OVER (ORDER BY k ROWS danger(1) PRECEDING) FROM t",
76+
"SELECT sum(v) OVER (ORDER BY k ROWS BETWEEN danger(1) PRECEDING AND CURRENT ROW) FROM t",
77+
"SELECT sum(v) OVER (ORDER BY k ROWS BETWEEN CURRENT ROW AND danger(1) FOLLOWING) FROM t",
78+
"SELECT sum(v) OVER w FROM t WINDOW w AS (PARTITION BY danger(k))",
79+
"SELECT sum(v) OVER w FROM t WINDOW w AS (ORDER BY danger(k))",
80+
"SELECT sum(v) OVER w FROM t WINDOW w AS (ORDER BY k ROWS danger(1) PRECEDING)",
81+
"SELECT CONVERT(INT, danger(v)) FROM t"})
82+
void pureOuterFunctionsDoNotHideUnprovenChildren(String sql) throws JSQLParserException {
83+
StatementFeatures features = CCJSqlParserUtil.parse(sql)
84+
.getFeatures(Set.of("sum", "array_agg", "lag", "convert")::contains);
85+
assertThat(features.getUnresolvedReferences()).containsExactly("danger");
86+
assertThat(features.mayModifyData()).isTrue();
87+
assertThat(features.modifiesData()).isFalse();
88+
}
89+
90+
@Test
91+
void inlineWindowVisitsBothOrderListsOnceAndKeepsContext() throws JSQLParserException {
92+
String sql = "SELECT array_agg(arg_fn(v) ORDER BY inner_fn(k)) "
93+
+ "FILTER (WHERE filter_fn(v) > 0) OVER (PARTITION BY part_fn(k) "
94+
+ "ORDER BY outer_fn(k) ROWS BETWEEN start_fn(1) PRECEDING "
95+
+ "AND end_fn(1) FOLLOWING) FROM t";
96+
PlainSelect select = (PlainSelect) CCJSqlParserUtil.parse(sql);
97+
List<String> seen = new ArrayList<>();
98+
Object marker = new Object();
99+
ExpressionVisitorAdapter<Void> expressions = new ExpressionVisitorAdapter<Void>() {
100+
@Override
101+
public <S> Void visit(Function function, S context) {
102+
assertThat(context).isSameAs(marker);
103+
seen.add(function.getName());
104+
return super.visit(function, context);
105+
}
106+
107+
@Override
108+
protected <S> Void visitExpressions(Expression expression, S context,
109+
Collection<Expression> children) {
110+
assertThat(context).isSameAs(marker);
111+
return super.visitExpressions(expression, context, children);
112+
}
113+
};
114+
select.accept(new SelectVisitorAdapter<>(expressions), marker);
115+
assertThat(seen).containsExactly("arg_fn", "filter_fn", "inner_fn", "part_fn",
116+
"outer_fn", "start_fn", "end_fn");
117+
}
118+
119+
@Test
120+
void functionModifiersRemainVisibleAfterAnalyticConversion() {
121+
Function function = new Function().withName("parent").withParameters(new LongValue(1));
122+
function.setHavingClause(new Function.HavingClause(Function.HavingClause.HavingType.MAX,
123+
new Function().withName("having_fn")));
124+
function.setKeywordArguments(List.of(new Function.KeywordArgument("SEPARATOR",
125+
new Function().withName("keyword_fn"))));
126+
function.setLimit(new Limit().withRowCount(new Function().withName("limit_fn")));
127+
for (Expression expression : List.of(function, new AnalyticExpression(function))) {
128+
PlainSelect select = new PlainSelect();
129+
select.setSelectItems(List.of(new SelectItem<>(expression)));
130+
assertThat(select.getFeatures("parent"::equals).getUnresolvedReferences())
131+
.containsExactly("having_fn", "keyword_fn", "limit_fn");
132+
}
133+
}
134+
135+
@Test
136+
void emptyAndUnboundedWindowsContainNoSpuriousFunctions() throws JSQLParserException {
137+
assertThat(new WindowDefinition().getAllExpressions()).isEmpty();
138+
Statement statement = CCJSqlParserUtil.parse("SELECT sum(v) OVER "
139+
+ "(ORDER BY k ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) FROM t");
140+
assertThat(statement.getFeatures("sum"::equals).mayModifyData()).isFalse();
141+
}
142+
}

0 commit comments

Comments
 (0)