Skip to content

Commit daa0561

Browse files
minleejaeminleejae
committed
refactor: simplify row-pattern rendering and validation
1 parent 2391715 commit daa0561

7 files changed

Lines changed: 126 additions & 122 deletions

File tree

‎.codacy.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,8 @@ engines:
44
exclude_paths:
55
# These fixtures use PostgreSQL grammar, not Transact-SQL.
66
- "src/test/resources/postgresql/**"
7+
# MATCH_RECOGNIZE fixtures use Oracle and BigQuery grammar.
8+
- "src/test/resources/net/sf/jsqlparser/statement/select/match-recognize/**"
9+
- "src/test/resources/net/sf/jsqlparser/statement/select/match-recognize-2350.sql"
710
exclude_paths:
811
- "site/**"

‎src/main/java/net/sf/jsqlparser/expression/StructType.java‎

Lines changed: 43 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -119,76 +119,57 @@ public StringBuilder appendTo(StringBuilder builder) {
119119

120120
/** Renders current argument expressions through the caller's visitor. */
121121
public StringBuilder appendTo(StringBuilder builder, Consumer<Expression> expressions) {
122-
if (dialect != Dialect.DUCKDB && keyword != null) {
123-
builder.append(keyword);
124-
}
125-
126-
if (dialect != Dialect.DUCKDB && parameters != null && !parameters.isEmpty()) {
127-
builder.append("<");
128-
int i = 0;
129-
130-
for (Map.Entry<String, ColDataType> e : parameters) {
131-
if (0 < i++) {
132-
builder.append(",");
133-
}
134-
// optional name
135-
if (e.getKey() != null && !e.getKey().isEmpty()) {
136-
builder.append(e.getKey()).append(" ");
137-
}
138-
139-
// mandatory type
140-
builder.append(e.getValue());
122+
if (dialect == Dialect.DUCKDB) {
123+
appendArguments(builder, expressions);
124+
appendTypeParameters(builder, true);
125+
} else {
126+
if (keyword != null) {
127+
builder.append(keyword);
141128
}
142-
143-
builder.append(">");
129+
appendTypeParameters(builder, false);
130+
appendArguments(builder, expressions);
144131
}
132+
return builder;
133+
}
145134

146-
if (arguments != null && !arguments.isEmpty()) {
147-
148-
if (dialect == Dialect.DUCKDB) {
149-
builder.append("{ ");
150-
int i = 0;
151-
for (SelectItem<?> e : arguments) {
152-
if (0 < i++) {
153-
builder.append(",");
154-
}
155-
builder.append(e.getAlias().getName());
156-
builder.append(":");
157-
expressions.accept(e.getExpression());
158-
}
159-
builder.append(" }");
160-
} else {
161-
builder.append("(");
162-
int i = 0;
163-
for (SelectItem<?> e : arguments) {
164-
if (0 < i++) {
165-
builder.append(",");
166-
}
167-
expressions.accept(e.getExpression());
168-
if (e.getAlias() != null) {
169-
builder.append(e.getAlias());
170-
}
171-
}
172-
173-
builder.append(")");
135+
private void appendTypeParameters(StringBuilder builder, boolean cast) {
136+
if (parameters == null || parameters.isEmpty()) {
137+
return;
138+
}
139+
builder.append(cast ? "::STRUCT( " : "<");
140+
int i = 0;
141+
for (Map.Entry<String, ColDataType> parameter : parameters) {
142+
if (i++ > 0) {
143+
builder.append(',');
144+
}
145+
if (cast || parameter.getKey() != null && !parameter.getKey().isEmpty()) {
146+
builder.append(parameter.getKey()).append(' ');
174147
}
148+
builder.append(parameter.getValue());
175149
}
150+
builder.append(cast ? ')' : '>');
151+
}
176152

177-
if (dialect == Dialect.DUCKDB && parameters != null && !parameters.isEmpty()) {
178-
builder.append("::STRUCT( ");
179-
int i = 0;
180-
181-
for (Map.Entry<String, ColDataType> e : parameters) {
182-
if (0 < i++) {
183-
builder.append(",");
184-
}
185-
builder.append(e.getKey()).append(" ");
186-
builder.append(e.getValue());
153+
private void appendArguments(StringBuilder builder, Consumer<Expression> expressions) {
154+
if (arguments == null || arguments.isEmpty()) {
155+
return;
156+
}
157+
boolean structLiteral = dialect == Dialect.DUCKDB;
158+
builder.append(structLiteral ? "{ " : "(");
159+
int i = 0;
160+
for (SelectItem<?> argument : arguments) {
161+
if (i++ > 0) {
162+
builder.append(',');
163+
}
164+
if (structLiteral) {
165+
builder.append(argument.getAlias().getName()).append(':');
166+
}
167+
expressions.accept(argument.getExpression());
168+
if (!structLiteral && argument.getAlias() != null) {
169+
builder.append(argument.getAlias());
187170
}
188-
builder.append(")");
189171
}
190-
191-
return builder;
172+
builder.append(structLiteral ? " }" : ")");
192173
}
193174

194175
@Override

‎src/main/java/net/sf/jsqlparser/statement/select/MatchRecognize.java‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,26 @@ public StringBuilder appendTo(StringBuilder builder, Consumer<FromItem> sources,
204204
Consumer<Pivot> pivots, Consumer<UnPivot> unpivots) {
205205
sources.accept(input);
206206
builder.append(" MATCH_RECOGNIZE (");
207+
appendInputExpressions(builder, expressions, ordering);
208+
appendRowOutput(builder);
209+
appendSkip(builder);
210+
builder.append("PATTERN (");
211+
pattern.appendTo(builder, expressions);
212+
builder.append(')');
213+
appendSubsets(builder);
214+
builder.append(" DEFINE ");
215+
appendList(builder, definitions, definition -> {
216+
builder.append(definition.getName()).append(" AS ");
217+
expressions.accept(definition.getExpression());
218+
});
219+
appendOptions(builder);
220+
builder.append(')');
221+
appendResultModifiers(builder, pivots, unpivots);
222+
return builder;
223+
}
224+
225+
private void appendInputExpressions(StringBuilder builder, Consumer<Expression> expressions,
226+
Consumer<OrderByElement> ordering) {
207227
if (!partitionBy.isEmpty()) {
208228
builder.append("PARTITION BY ");
209229
appendList(builder, partitionBy, expressions);
@@ -224,6 +244,9 @@ public StringBuilder appendTo(StringBuilder builder, Consumer<FromItem> sources,
224244
});
225245
builder.append(' ');
226246
}
247+
}
248+
249+
private void appendRowOutput(StringBuilder builder) {
227250
if (rowsPerMatch != null) {
228251
builder.append(rowsPerMatch == RowsPerMatch.ONE ? "ONE ROW PER MATCH "
229252
: "ALL ROWS PER MATCH ");
@@ -240,6 +263,9 @@ public StringBuilder appendTo(StringBuilder builder, Consumer<FromItem> sources,
240263
builder.append("WITH UNMATCHED ROWS ");
241264
}
242265
}
266+
}
267+
268+
private void appendSkip(StringBuilder builder) {
243269
if (skipMode != null) {
244270
builder.append("AFTER MATCH SKIP ");
245271
switch (skipMode) {
@@ -260,9 +286,9 @@ public StringBuilder appendTo(StringBuilder builder, Consumer<FromItem> sources,
260286
}
261287
builder.append(' ');
262288
}
263-
builder.append("PATTERN (");
264-
pattern.appendTo(builder, expressions);
265-
builder.append(')');
289+
}
290+
291+
private void appendSubsets(StringBuilder builder) {
266292
if (!subsets.isEmpty()) {
267293
builder.append(" SUBSET ");
268294
appendList(builder, subsets, subset -> {
@@ -271,11 +297,9 @@ public StringBuilder appendTo(StringBuilder builder, Consumer<FromItem> sources,
271297
builder.append(')');
272298
});
273299
}
274-
builder.append(" DEFINE ");
275-
appendList(builder, definitions, definition -> {
276-
builder.append(definition.getName()).append(" AS ");
277-
expressions.accept(definition.getExpression());
278-
});
300+
}
301+
302+
private void appendOptions(StringBuilder builder) {
279303
if (options != null) {
280304
builder.append(" OPTIONS (");
281305
if (options.getUseLongestMatch() != null) {
@@ -284,7 +308,10 @@ public StringBuilder appendTo(StringBuilder builder, Consumer<FromItem> sources,
284308
}
285309
builder.append(')');
286310
}
287-
builder.append(')');
311+
}
312+
313+
private void appendResultModifiers(StringBuilder builder, Consumer<Pivot> pivots,
314+
Consumer<UnPivot> unpivots) {
288315
if (getPivot() != null) {
289316
pivots.accept(getPivot());
290317
}
@@ -297,7 +324,6 @@ public StringBuilder appendTo(StringBuilder builder, Consumer<FromItem> sources,
297324
if (getSampleClause() != null) {
298325
builder.append(getSampleClause());
299326
}
300-
return builder;
301327
}
302328

303329
private static <E> void appendList(StringBuilder builder, List<E> elements,

‎src/main/java/net/sf/jsqlparser/util/TablesNamesFinder.java‎

Lines changed: 15 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -405,26 +405,14 @@ public <S> Void visit(PlainSelect plainSelect, S context) {
405405
}
406406

407407
private <S> Void visitPlainSelectBody(PlainSelect plainSelect, S context) {
408-
List<WithItem<?>> withItemsList = plainSelect.getWithItemsList();
409-
if (withItemsList != null && !withItemsList.isEmpty()) {
410-
for (WithItem<?> withItem : withItemsList) {
411-
withItem.accept((SelectVisitor<?>) this, context);
412-
}
413-
}
408+
visitWithItems(plainSelect.getWithItemsList(), context);
414409
if (plainSelect.getDistinct() != null) {
415410
visitSelectItems(plainSelect.getDistinct().getOnSelectItems(), context);
416411
}
417412
visitTables(plainSelect.getIntoTables(), context);
418413

419-
if (plainSelect.getSelectItems() != null) {
420-
for (SelectItem<?> item : plainSelect.getSelectItems()) {
421-
item.accept(this, context);
422-
}
423-
}
424-
425-
if (plainSelect.getFromItem() != null) {
426-
plainSelect.getFromItem().accept(this, context);
427-
}
414+
visitSelectItems(plainSelect.getSelectItems(), context);
415+
visitFromItem(plainSelect.getFromItem(), context);
428416

429417
if (plainSelect.getLateralViews() != null) {
430418
for (LateralView lateralView : plainSelect.getLateralViews()) {
@@ -433,27 +421,17 @@ private <S> Void visitPlainSelectBody(PlainSelect plainSelect, S context) {
433421
}
434422

435423
visitJoins(plainSelect.getJoins(), context);
436-
if (plainSelect.getPreWhere() != null) {
437-
plainSelect.getPreWhere().accept(this, context);
438-
}
439-
if (plainSelect.getWhere() != null) {
440-
plainSelect.getWhere().accept(this, context);
441-
}
424+
visitExpression(plainSelect.getPreWhere(), context);
425+
visitExpression(plainSelect.getWhere(), context);
442426

443427
visitPreferringClause(plainSelect.getPreferringClause(), context);
444428
visit(plainSelect.getGroupBy(), context);
445429

446-
if (plainSelect.getHaving() != null) {
447-
plainSelect.getHaving().accept(this, context);
448-
}
430+
visitExpression(plainSelect.getHaving(), context);
449431

450-
if (plainSelect.getQualify() != null) {
451-
plainSelect.getQualify().accept(this, context);
452-
}
432+
visitExpression(plainSelect.getQualify(), context);
453433

454-
if (plainSelect.getOracleHierarchical() != null) {
455-
plainSelect.getOracleHierarchical().accept(this, context);
456-
}
434+
visitExpression(plainSelect.getOracleHierarchical(), context);
457435

458436
if (plainSelect.getWindowDefinitions() != null) {
459437
for (WindowDefinition windowDefinition : plainSelect.getWindowDefinitions()) {
@@ -472,12 +450,7 @@ private <S> Void visitPlainSelectBody(PlainSelect plainSelect, S context) {
472450
visitOrderBy(plainSelect.getOrderByElements(), context);
473451
visitLimit(plainSelect.getLimit(), context);
474452
visitLimit(plainSelect.getLimitBy(), context);
475-
if (plainSelect.getOffset() != null) {
476-
plainSelect.getOffset().getOffset().accept(this, context);
477-
}
478-
if (plainSelect.getFetch() != null) {
479-
plainSelect.getFetch().getExpression().accept(this, context);
480-
}
453+
visitSelectPagination(plainSelect, context);
481454
visitUpdateSets(plainSelect.getSettings(), context);
482455
visitFromItem(plainSelect.getIntoTempTable(), context);
483456
return null;
@@ -493,7 +466,7 @@ public <S> Void visit(PivotQuery pivotQuery, S context) {
493466
visitOrderBy(pivotQuery.getOrderByElements(), context);
494467
visitLimit(pivotQuery.getLimit(), context);
495468

496-
visitPivotPagination(pivotQuery, context);
469+
visitSelectPagination(pivotQuery, context);
497470
return null;
498471
}
499472

@@ -505,12 +478,12 @@ private <S> void visitSelectItems(List<? extends SelectItem<?>> selectItems, S c
505478
}
506479
}
507480

508-
private <S> void visitPivotPagination(PivotQuery pivotQuery, S context) {
509-
if (pivotQuery.getOffset() != null) {
510-
pivotQuery.getOffset().getOffset().accept(this, context);
481+
private <S> void visitSelectPagination(Select select, S context) {
482+
if (select.getOffset() != null) {
483+
select.getOffset().getOffset().accept(this, context);
511484
}
512-
if (pivotQuery.getFetch() != null && pivotQuery.getFetch().getExpression() != null) {
513-
pivotQuery.getFetch().getExpression().accept(this, context);
485+
if (select.getFetch() != null && select.getFetch().getExpression() != null) {
486+
select.getFetch().getExpression().accept(this, context);
514487
}
515488
}
516489

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,8 @@
150150
@SuppressWarnings({"PMD.CyclomaticComplexity"})
151151
public class ExpressionValidator extends AbstractValidator<Expression>
152152
implements ExpressionVisitor<Void> {
153+
private Set<String> rowPatternVariables = Collections.emptySet();
154+
153155
@Override
154156
public <S> Void visit(Addition addition, S context) {
155157
visitBinaryExpression(addition, " + ");
@@ -537,8 +539,6 @@ public <S> Void visit(ParenthesedSelect selectBody, S context) {
537539
return null;
538540
}
539541

540-
private Set<String> rowPatternVariables = Collections.emptySet();
541-
542542
void validateMatchRecognizeExpressions(MatchRecognize matchRecognize) {
543543
Set<String> previous = rowPatternVariables;
544544
try {

0 commit comments

Comments
 (0)