diff --git a/src/main/java/net/sf/jsqlparser/expression/AnalyticExpression.java b/src/main/java/net/sf/jsqlparser/expression/AnalyticExpression.java index d94d4ef2eb..c01df2ee33 100644 --- a/src/main/java/net/sf/jsqlparser/expression/AnalyticExpression.java +++ b/src/main/java/net/sf/jsqlparser/expression/AnalyticExpression.java @@ -58,7 +58,7 @@ public class AnalyticExpression extends ASTNodeAccessImpl implements Expression public AnalyticExpression() {} public AnalyticExpression(Function function) { - this.name = String.join(" ", function.getMultipartName()); + this.name = function.getName(); this.allColumns = function.isAllColumns(); this.distinct = function.isDistinct(); this.unique = function.isUnique(); diff --git a/src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java b/src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java index 753a6791da..4355573442 100644 --- a/src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java +++ b/src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java @@ -12,8 +12,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.List; import java.util.Map; -import java.util.Optional; import net.sf.jsqlparser.expression.operators.arithmetic.Addition; import net.sf.jsqlparser.expression.operators.arithmetic.BitwiseAnd; import net.sf.jsqlparser.expression.operators.arithmetic.BitwiseLeftShift; @@ -119,11 +119,9 @@ public T visit(Function function, S context) { if (function.getKeep() != null) { subExpressions.add(function.getKeep()); } - if (function.getOrderByElements() != null) { - for (OrderByElement orderByElement : function.getOrderByElements()) { - subExpressions.add(orderByElement.getExpression()); - } - } + addOrderByExpressions(subExpressions, function.getOrderByElements()); + addFunctionModifiers(subExpressions, function.getHavingClause(), + function.getKeywordArguments(), function.getLimit()); return visitExpressions(function, context, subExpressions); } @@ -419,29 +417,41 @@ public T visit(AnalyticExpression analyticExpression, S context) { if (analyticExpression.getKeep() != null) { subExpressions.add(analyticExpression.getKeep()); } - if (analyticExpression.getFuncOrderBy() != null) { - for (OrderByElement element : analyticExpression.getOrderByElements()) { - subExpressions.add(element.getExpression()); + subExpressions.add(analyticExpression.getFilterExpression()); + addOrderByExpressions(subExpressions, analyticExpression.getFuncOrderBy()); + addFunctionModifiers(subExpressions, analyticExpression.getHavingClause(), + analyticExpression.getKeywordArguments(), analyticExpression.getLimit()); + if (analyticExpression.getWindowDefinition() != null) { + subExpressions.addAll(analyticExpression.getWindowDefinition().getAllExpressions()); + } + return visitExpressions(analyticExpression, context, subExpressions); + } + + private static void addOrderByExpressions(List expressions, + List orderBy) { + if (orderBy != null) { + for (OrderByElement element : orderBy) { + expressions.add(element.getExpression()); } } - if (analyticExpression.getWindowElement() != null) { - /* - * Visit expressions from the range and offset of the window element. Do this using - * optional chains, because several things down the tree can be null e.g. the - * expression. So, null-safe versions of e.g.: - * analyticExpression.getWindowElement().getOffset().getExpression().accept(this, - * parameters); - */ - Optional.ofNullable(analyticExpression.getWindowElement().getRange()) - .map(WindowRange::getStart) - .map(WindowOffset::getExpression).ifPresent(subExpressions::add); - Optional.ofNullable(analyticExpression.getWindowElement().getRange()) - .map(WindowRange::getEnd) - .map(WindowOffset::getExpression).ifPresent(subExpressions::add); - Optional.ofNullable(analyticExpression.getWindowElement().getOffset()) - .map(WindowOffset::getExpression).ifPresent(subExpressions::add); + } + + private static void addFunctionModifiers(List expressions, + Function.HavingClause having, List arguments, + net.sf.jsqlparser.statement.select.Limit limit) { + expressions.add(having); + if (arguments != null) { + for (Function.KeywordArgument argument : arguments) { + expressions.add(argument.getExpression()); + } + } + if (limit != null) { + expressions.add(limit.getOffset()); + expressions.add(limit.getRowCount()); + if (limit.getByExpressions() != null) { + expressions.addAll(limit.getByExpressions()); + } } - return visitExpressions(analyticExpression, context, subExpressions); } @Override diff --git a/src/main/java/net/sf/jsqlparser/expression/WindowDefinition.java b/src/main/java/net/sf/jsqlparser/expression/WindowDefinition.java index 60760baee0..2996821cf2 100644 --- a/src/main/java/net/sf/jsqlparser/expression/WindowDefinition.java +++ b/src/main/java/net/sf/jsqlparser/expression/WindowDefinition.java @@ -10,6 +10,7 @@ package net.sf.jsqlparser.expression; import java.io.Serializable; +import java.util.ArrayList; import java.util.List; import net.sf.jsqlparser.expression.operators.relational.ExpressionList; @@ -73,6 +74,30 @@ public WindowDefinition withWindowName(String windowName) { return this; } + /** Returns the partition, order and frame expressions for both inline and named windows. */ + public List getAllExpressions() { + List expressions = new ArrayList<>(partitionBy); + if (getOrderByElements() != null) { + for (OrderByElement element : getOrderByElements()) { + expressions.add(element.getExpression()); + } + } + if (windowElement != null) { + if (windowElement.getRange() != null) { + addOffsetExpression(expressions, windowElement.getRange().getStart()); + addOffsetExpression(expressions, windowElement.getRange().getEnd()); + } + addOffsetExpression(expressions, windowElement.getOffset()); + } + return expressions; + } + + private static void addOffsetExpression(List expressions, WindowOffset offset) { + if (offset != null && offset.getExpression() != null) { + expressions.add(offset.getExpression()); + } + } + @Override public String toString() { StringBuilder b = new StringBuilder(); diff --git a/src/main/java/net/sf/jsqlparser/statement/StatementFeatureVisitor.java b/src/main/java/net/sf/jsqlparser/statement/StatementFeatureVisitor.java index 428c4aca62..5b33177f5e 100644 --- a/src/main/java/net/sf/jsqlparser/statement/StatementFeatureVisitor.java +++ b/src/main/java/net/sf/jsqlparser/statement/StatementFeatureVisitor.java @@ -27,6 +27,8 @@ import net.sf.jsqlparser.statement.alter.AlterSubscription; import net.sf.jsqlparser.JSQLParserException; +import net.sf.jsqlparser.expression.AnalyticExpression; +import net.sf.jsqlparser.expression.TranscodingFunction; import net.sf.jsqlparser.expression.Expression; import net.sf.jsqlparser.expression.ExpressionVisitor; import net.sf.jsqlparser.expression.ExpressionVisitorAdapter; @@ -829,22 +831,32 @@ static final class FeatureExpressionVisitor extends ExpressionVisitorAdapterpossible set, with the name recorded so it can be resolved against a - * catalogue rather than guessed at here. - */ - @Override - public Void visit(Function function, S context) { - String name = function.getName() == null - ? "?" - : function.getName().toLowerCase(Locale.ROOT); + /** Records unproven functions consistently across their different expression models. */ + private void analyseFunction(String functionName) { + String name = functionName == null ? "?" : functionName.toLowerCase(Locale.ROOT); if (!analysis.pureFunctions.test(name)) { analysis.possible(StmtFeature.MODIFIES_DATA, StmtFeature.MODIFIES_SCHEMA); analysis.unresolved(name); } + } + + @Override + public Void visit(Function function, S context) { + analyseFunction(function.getName()); return super.visit(function, context); } + + @Override + public Void visit(AnalyticExpression expression, S context) { + analyseFunction(expression.getName()); + return super.visit(expression, context); + } + + @Override + public Void visit(TranscodingFunction expression, S context) { + analyseFunction(expression.getKeyword()); + return super.visit(expression, context); + } } static final class FeatureFromItemVisitor extends FromItemVisitorAdapter { diff --git a/src/main/java/net/sf/jsqlparser/statement/select/SelectVisitorAdapter.java b/src/main/java/net/sf/jsqlparser/statement/select/SelectVisitorAdapter.java index 45c10a5a56..d635a6e1e9 100644 --- a/src/main/java/net/sf/jsqlparser/statement/select/SelectVisitorAdapter.java +++ b/src/main/java/net/sf/jsqlparser/statement/select/SelectVisitorAdapter.java @@ -12,6 +12,8 @@ import net.sf.jsqlparser.expression.ExpressionVisitor; import net.sf.jsqlparser.expression.ExpressionVisitorAdapter; import net.sf.jsqlparser.expression.Function; +import net.sf.jsqlparser.expression.Expression; +import net.sf.jsqlparser.expression.WindowDefinition; import net.sf.jsqlparser.statement.OutputClause; import net.sf.jsqlparser.statement.ParenthesedStatement; import net.sf.jsqlparser.statement.Statement; @@ -197,9 +199,13 @@ public T visit(PlainSelect plainSelect, S context) { expressionVisitor.visitExpression(plainSelect.getHaving(), context); expressionVisitor.visitExpression(plainSelect.getQualify(), context); - // if (plainSelect.getWindowDefinitions() != null) { - // //@todo: implement - // } + if (plainSelect.getWindowDefinitions() != null) { + for (WindowDefinition window : plainSelect.getWindowDefinitions()) { + for (Expression expression : window.getAllExpressions()) { + expressionVisitor.visitExpression(expression, context); + } + } + } Pivot pivot = plainSelect.getPivot(); if (pivot != null) { diff --git a/src/test/java/net/sf/jsqlparser/expression/FunctionFeatureAnalysisTest.java b/src/test/java/net/sf/jsqlparser/expression/FunctionFeatureAnalysisTest.java new file mode 100644 index 0000000000..f14bf4b17d --- /dev/null +++ b/src/test/java/net/sf/jsqlparser/expression/FunctionFeatureAnalysisTest.java @@ -0,0 +1,142 @@ +/*- + * #%L + * JSQLParser library + * %% + * Copyright (C) 2004 - 2026 JSQLParser + * %% + * Dual licensed under GNU LGPL 2.1 or Apache License 2.0 + * #L% + */ +package net.sf.jsqlparser.expression; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Set; +import java.util.stream.Stream; +import net.sf.jsqlparser.JSQLParserException; +import net.sf.jsqlparser.parser.CCJSqlParserUtil; +import net.sf.jsqlparser.statement.Statement; +import net.sf.jsqlparser.statement.StatementFeatures; +import net.sf.jsqlparser.statement.StmtFeature; +import net.sf.jsqlparser.statement.select.Limit; +import net.sf.jsqlparser.statement.select.PlainSelect; +import net.sf.jsqlparser.statement.select.SelectItem; +import net.sf.jsqlparser.statement.select.SelectVisitorAdapter; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; + +class FunctionFeatureAnalysisTest { + static Stream functionForms() { + return Stream.of( + Arguments.of("SELECT pg_sleep(1) FROM t", "pg_sleep"), + Arguments.of("SELECT pg_sleep(1) OVER () FROM t", "pg_sleep"), + Arguments.of("SELECT SuM(v) OVER () FROM t", "sum"), + Arguments.of("SELECT analytics.sum(v) OVER () FROM t", "analytics.sum"), + Arguments.of("SELECT CONVERT(v USING utf8) FROM t", "convert"), + Arguments.of("SELECT CONVERT(INT, v) FROM t", "convert"), + Arguments.of("SELECT TRY_CONVERT(INT, v) FROM t", "try_convert")); + } + + @ParameterizedTest + @MethodSource("functionForms") + void appliesTheSamePurityContractToEachFunctionForm(String sql, String name) + throws JSQLParserException { + Statement statement = CCJSqlParserUtil.parse(sql); + String before = statement.toString(); + StatementFeatures unknown = statement.getFeatures(n -> false); + assertThat(unknown.getUnresolvedReferences()).containsExactly(name); + assertThat(unknown.getUncertain()) + .contains(StmtFeature.MODIFIES_DATA, StmtFeature.MODIFIES_SCHEMA); + assertThat(unknown.modifiesData()).isFalse(); + assertThat(unknown.mayModifyData()).isTrue(); + + StatementFeatures pure = statement.getFeatures(name::equals); + assertThat(pure.getUnresolvedReferences()).isEmpty(); + assertThat(pure.mayModifyData()).isFalse(); + assertThat(statement.toString()).isEqualTo(before); + assertThat(CCJSqlParserUtil.parse(before).toString()).isEqualTo(before); + } + + @ParameterizedTest + @ValueSource(strings = { + "SELECT sum(danger(v)) OVER () FROM t", + "SELECT sum(v) OVER (PARTITION BY danger(k)) FROM t", + "SELECT sum(v) OVER (ORDER BY danger(k)) FROM t", + "SELECT sum(v) FILTER (WHERE danger(k) > 0) OVER () FROM t", + "SELECT array_agg(v ORDER BY danger(k)) OVER () FROM t", + "SELECT lag(v, danger(k), 0) OVER () FROM t", + "SELECT lag(v, 1, danger(k)) OVER () FROM t", + "SELECT sum(v) OVER (ORDER BY k ROWS danger(1) PRECEDING) FROM t", + "SELECT sum(v) OVER (ORDER BY k ROWS BETWEEN danger(1) PRECEDING AND CURRENT ROW) FROM t", + "SELECT sum(v) OVER (ORDER BY k ROWS BETWEEN CURRENT ROW AND danger(1) FOLLOWING) FROM t", + "SELECT sum(v) OVER w FROM t WINDOW w AS (PARTITION BY danger(k))", + "SELECT sum(v) OVER w FROM t WINDOW w AS (ORDER BY danger(k))", + "SELECT sum(v) OVER w FROM t WINDOW w AS (ORDER BY k ROWS danger(1) PRECEDING)", + "SELECT CONVERT(INT, danger(v)) FROM t"}) + void pureOuterFunctionsDoNotHideUnprovenChildren(String sql) throws JSQLParserException { + StatementFeatures features = CCJSqlParserUtil.parse(sql) + .getFeatures(Set.of("sum", "array_agg", "lag", "convert")::contains); + assertThat(features.getUnresolvedReferences()).containsExactly("danger"); + assertThat(features.mayModifyData()).isTrue(); + assertThat(features.modifiesData()).isFalse(); + } + + @Test + void inlineWindowVisitsBothOrderListsOnceAndKeepsContext() throws JSQLParserException { + String sql = "SELECT array_agg(arg_fn(v) ORDER BY inner_fn(k)) " + + "FILTER (WHERE filter_fn(v) > 0) OVER (PARTITION BY part_fn(k) " + + "ORDER BY outer_fn(k) ROWS BETWEEN start_fn(1) PRECEDING " + + "AND end_fn(1) FOLLOWING) FROM t"; + PlainSelect select = (PlainSelect) CCJSqlParserUtil.parse(sql); + List seen = new ArrayList<>(); + Object marker = new Object(); + ExpressionVisitorAdapter expressions = new ExpressionVisitorAdapter() { + @Override + public Void visit(Function function, S context) { + assertThat(context).isSameAs(marker); + seen.add(function.getName()); + return super.visit(function, context); + } + + @Override + protected Void visitExpressions(Expression expression, S context, + Collection children) { + assertThat(context).isSameAs(marker); + return super.visitExpressions(expression, context, children); + } + }; + select.accept(new SelectVisitorAdapter<>(expressions), marker); + assertThat(seen).containsExactly("arg_fn", "filter_fn", "inner_fn", "part_fn", + "outer_fn", "start_fn", "end_fn"); + } + + @Test + void functionModifiersRemainVisibleAfterAnalyticConversion() { + Function function = new Function().withName("parent").withParameters(new LongValue(1)); + function.setHavingClause(new Function.HavingClause(Function.HavingClause.HavingType.MAX, + new Function().withName("having_fn"))); + function.setKeywordArguments(List.of(new Function.KeywordArgument("SEPARATOR", + new Function().withName("keyword_fn")))); + function.setLimit(new Limit().withRowCount(new Function().withName("limit_fn"))); + for (Expression expression : List.of(function, new AnalyticExpression(function))) { + PlainSelect select = new PlainSelect(); + select.setSelectItems(List.of(new SelectItem<>(expression))); + assertThat(select.getFeatures("parent"::equals).getUnresolvedReferences()) + .containsExactly("having_fn", "keyword_fn", "limit_fn"); + } + } + + @Test + void emptyAndUnboundedWindowsContainNoSpuriousFunctions() throws JSQLParserException { + assertThat(new WindowDefinition().getAllExpressions()).isEmpty(); + Statement statement = CCJSqlParserUtil.parse("SELECT sum(v) OVER " + + "(ORDER BY k ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) FROM t"); + assertThat(statement.getFeatures("sum"::equals).mayModifyData()).isFalse(); + } +}