Skip to content

Commit 4a3d439

Browse files
committed
Revert "Support spath with dynamic fields (opensearch-project#5058)"
This reverts commit 633d760. Signed-off-by: Chen Dai <daichen@amazon.com>
1 parent 57ec9ae commit 4a3d439

28 files changed

Lines changed: 250 additions & 1937 deletions

File tree

common/src/main/java/org/opensearch/sql/common/utils/DebugUtils.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
import java.util.Collection;
99
import java.util.Map;
1010
import java.util.stream.Collectors;
11+
import org.apache.logging.log4j.LogManager;
12+
import org.apache.logging.log4j.Logger;
1113

1214
/**
1315
* Utility class for debugging operations. This class is only for debugging purpose, and not
@@ -16,6 +18,7 @@
1618
public class DebugUtils {
1719
// Update this to true while you are debugging. (Safe guard to avoid usage in production code. )
1820
private static final boolean IS_DEBUG = false;
21+
private static final Logger logger = LogManager.getLogger(DebugUtils.class);
1922

2023
public static <T> T debug(T obj, String message) {
2124
verifyDebug();
@@ -36,7 +39,7 @@ private static void verifyDebug() {
3639
}
3740

3841
private static void print(String format, Object... args) {
39-
System.out.println(String.format(format, args));
42+
logger.info(String.format(format, args));
4043
}
4144

4245
private static String getCalledFrom(int pos) {

core/src/main/java/org/opensearch/sql/ast/analysis/FieldResolutionResult.java

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,17 @@ public class FieldResolutionResult {
2828
@NonNull private final Set<String> regularFields;
2929
@NonNull private final Wildcard wildcard;
3030

31-
public FieldResolutionResult(Collection<String> regularFields) {
31+
public FieldResolutionResult(Set<String> regularFields) {
3232
this.regularFields = new HashSet<>(regularFields);
3333
this.wildcard = NULL_WILDCARD;
3434
}
3535

36-
public FieldResolutionResult(Collection<String> regularFields, Wildcard wildcard) {
36+
public FieldResolutionResult(Set<String> regularFields, Wildcard wildcard) {
3737
this.regularFields = new HashSet<>(regularFields);
3838
this.wildcard = wildcard;
3939
}
4040

41-
public FieldResolutionResult(Collection<String> regularFields, String wildcard) {
41+
public FieldResolutionResult(Set<String> regularFields, String wildcard) {
4242
this.regularFields = new HashSet<>(regularFields);
4343
this.wildcard = getWildcard(wildcard);
4444
}
@@ -53,12 +53,12 @@ private static Wildcard getWildcard(String wildcard) {
5353
}
5454
}
5555

56-
public FieldResolutionResult(Collection<String> regularFields, Collection<String> wildcards) {
56+
public FieldResolutionResult(Set<String> regularFields, Set<String> wildcards) {
5757
this.regularFields = new HashSet<>(regularFields);
5858
this.wildcard = createOrWildcard(wildcards);
5959
}
6060

61-
private static Wildcard createOrWildcard(Collection<String> patterns) {
61+
private static Wildcard createOrWildcard(Set<String> patterns) {
6262
if (patterns == null || patterns.isEmpty()) {
6363
return NULL_WILDCARD;
6464
}
@@ -70,50 +70,38 @@ private static Wildcard createOrWildcard(Collection<String> patterns) {
7070
return new OrWildcard(wildcards);
7171
}
7272

73-
/** Returns unmodifiable view of regular fields. */
7473
public Set<String> getRegularFieldsUnmodifiable() {
7574
return Collections.unmodifiableSet(regularFields);
7675
}
7776

78-
/** Checks if result contains any wildcard patterns. */
7977
public boolean hasWildcards() {
8078
return wildcard != NULL_WILDCARD;
8179
}
8280

83-
/** Checks if result contains partial wildcard patterns (not '*'). */
84-
public boolean hasPartialWildcards() {
85-
return wildcard != NULL_WILDCARD && wildcard != ANY_WILDCARD;
86-
}
87-
88-
/** Checks if result contains regular fields. */
8981
public boolean hasRegularFields() {
9082
return !regularFields.isEmpty();
9183
}
9284

93-
/** Creates new result excluding specified fields. */
9485
public FieldResolutionResult exclude(Collection<String> fields) {
9586
Set<String> combinedFields = new HashSet<>(this.regularFields);
9687
combinedFields.removeAll(fields);
9788
return new FieldResolutionResult(combinedFields, this.wildcard);
9889
}
9990

100-
/** Creates new result combining this result with additional fields (union). */
101-
public FieldResolutionResult or(Collection<String> fields) {
91+
public FieldResolutionResult or(Set<String> fields) {
10292
Set<String> combinedFields = new HashSet<>(this.regularFields);
10393
combinedFields.addAll(fields);
10494
return new FieldResolutionResult(combinedFields, this.wildcard);
10595
}
10696

107-
private Set<String> and(Collection<String> fields) {
97+
private Set<String> and(Set<String> fields) {
10898
return fields.stream()
10999
.filter(field -> this.getRegularFields().contains(field) || this.wildcard.matches(field))
110100
.collect(Collectors.toSet());
111101
}
112102

113-
/** Creates new result intersecting this result with another (intersection). */
114103
public FieldResolutionResult and(FieldResolutionResult other) {
115-
Set<String> combinedFields = new HashSet<>();
116-
combinedFields.addAll(this.and(other.regularFields));
104+
Set<String> combinedFields = this.and(other.regularFields);
117105
combinedFields.addAll(other.and(this.regularFields));
118106

119107
Wildcard combinedWildcard = this.wildcard.and(other.wildcard);
@@ -123,7 +111,6 @@ public FieldResolutionResult and(FieldResolutionResult other) {
123111

124112
/** Interface for wildcard pattern matching. */
125113
public interface Wildcard {
126-
/** Checks if field name matches wildcard pattern. */
127114
boolean matches(String fieldName);
128115

129116
default Wildcard and(Wildcard other) {

core/src/main/java/org/opensearch/sql/ast/analysis/FieldResolutionVisitor.java

Lines changed: 17 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,6 @@
7777
*/
7878
public class FieldResolutionVisitor extends AbstractNodeVisitor<Node, FieldResolutionContext> {
7979

80-
private static final String ALL_FIELDS = "*";
81-
8280
/**
8381
* Analyzes PPL query plan to determine required fields at each node.
8482
*
@@ -114,10 +112,10 @@ private void acceptAndVerifyNodeVisited(Node node, FieldResolutionContext contex
114112

115113
@Override
116114
public Node visitProject(Project node, FieldResolutionContext context) {
117-
boolean isSingleSelectAll =
118-
node.getProjectList().size() == 1 && node.getProjectList().get(0) instanceof AllFields;
115+
boolean isSelectAll =
116+
node.getProjectList().stream().anyMatch(expr -> expr instanceof AllFields);
119117

120-
if (isSingleSelectAll) {
118+
if (isSelectAll) {
121119
visitChildren(node, context);
122120
} else {
123121
Set<String> projectFields = new HashSet<>();
@@ -183,14 +181,15 @@ public Node visitSpath(SPath node, FieldResolutionContext context) {
183181
return visitEval(node.rewriteAsEval(), context);
184182
} else {
185183
// set requirements for spath command;
184+
context.setResult(node, context.getCurrentRequirements());
186185
FieldResolutionResult requirements = context.getCurrentRequirements();
187-
context.setResult(node, requirements);
188-
if (requirements.hasPartialWildcards()) {
186+
if (requirements.hasWildcards()) {
189187
throw new IllegalArgumentException(
190-
"Spath command cannot be used with partial wildcard such as `prefix*`.");
188+
"Spath command cannot extract arbitrary fields. Please project fields explicitly by"
189+
+ " fields command without wildcard or stats command.");
191190
}
192191

193-
context.pushRequirements(requirements.or(Set.of(node.getInField())));
192+
context.pushRequirements(context.getCurrentRequirements().or(Set.of(node.getInField())));
194193
visitChildren(node, context);
195194
context.popRequirements();
196195
return node;
@@ -240,8 +239,6 @@ private Set<String> extractFieldsFromExpression(UnresolvedExpression expr) {
240239

241240
if (expr instanceof Field field) {
242241
fields.add(field.getField().toString());
243-
} else if (expr instanceof AllFields) {
244-
fields.add(ALL_FIELDS);
245242
} else if (expr instanceof QualifiedName name) {
246243
fields.add(name.toString());
247244
} else if (expr instanceof Alias alias) {
@@ -495,56 +492,43 @@ public Node visitStreamWindow(StreamWindow node, FieldResolutionContext context)
495492

496493
@Override
497494
public Node visitFillNull(FillNull node, FieldResolutionContext context) {
498-
if (node.isAgainstAllFields()) {
499-
throw new IllegalArgumentException("Fields need to be specified with fillnull command");
500-
}
501-
Set<String> fields = new HashSet<>();
502-
node.getFields().forEach(field -> fields.addAll(extractFieldsFromExpression(field)));
503-
504-
context.pushRequirements(context.getCurrentRequirements().or(fields));
505495
visitChildren(node, context);
506-
context.popRequirements();
507496
return node;
508497
}
509498

510499
@Override
511500
public Node visitAppendCol(AppendCol node, FieldResolutionContext context) {
512-
throw new IllegalArgumentException(
513-
"AppendCol command cannot be used together with spath command");
501+
visitChildren(node, context);
502+
return node;
514503
}
515504

516505
@Override
517506
public Node visitAppend(Append node, FieldResolutionContext context) {
518-
// dispatch requirements to subsearch and main
519-
acceptAndVerifyNodeVisited(node.getSubSearch(), context);
520507
visitChildren(node, context);
521508
return node;
522509
}
523510

524511
@Override
525512
public Node visitMultisearch(Multisearch node, FieldResolutionContext context) {
526-
throw new IllegalArgumentException(
527-
"Multisearch command cannot be used together with spath command");
513+
visitChildren(node, context);
514+
return node;
528515
}
529516

530517
@Override
531518
public Node visitLookup(Lookup node, FieldResolutionContext context) {
532-
throw new IllegalArgumentException("Lookup command cannot be used together with spath command");
519+
visitChildren(node, context);
520+
return node;
533521
}
534522

535523
@Override
536524
public Node visitValues(Values node, FieldResolutionContext context) {
537-
throw new IllegalArgumentException("Values command cannot be used together with spath command");
525+
visitChildren(node, context);
526+
return node;
538527
}
539528

540529
@Override
541530
public Node visitReplace(Replace node, FieldResolutionContext context) {
542-
Set<String> fields = new HashSet<>();
543-
node.getFieldList().forEach(field -> fields.addAll(extractFieldsFromExpression(field)));
544-
545-
context.pushRequirements(context.getCurrentRequirements().or(fields));
546531
visitChildren(node, context);
547-
context.popRequirements();
548532
return node;
549533
}
550534

@@ -665,10 +649,6 @@ private Set<String> extractFieldsFromAggregation(UnresolvedExpression expr) {
665649
}
666650
}
667651
}
668-
return excludeAllFieldsWildcard(fields);
669-
}
670-
671-
private Set<String> excludeAllFieldsWildcard(Set<String> fields) {
672-
return fields.stream().filter(f -> !f.equals(ALL_FIELDS)).collect(Collectors.toSet());
652+
return fields;
673653
}
674654
}

core/src/main/java/org/opensearch/sql/ast/tree/FillNull.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,6 @@ public List<Field> getFields() {
6363
return getReplacementPairs().stream().map(Pair::getLeft).toList();
6464
}
6565

66-
public boolean isAgainstAllFields() {
67-
return !replacementForAll.isEmpty() && getReplacementPairs().isEmpty();
68-
}
69-
7066
@Override
7167
public FillNull attach(UnresolvedPlan child) {
7268
this.child = child;

core/src/main/java/org/opensearch/sql/ast/tree/Join.java

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import org.opensearch.sql.ast.AbstractNodeVisitor;
1818
import org.opensearch.sql.ast.expression.Argument;
1919
import org.opensearch.sql.ast.expression.Field;
20-
import org.opensearch.sql.ast.expression.Literal;
2120
import org.opensearch.sql.ast.expression.UnresolvedExpression;
2221

2322
@ToString
@@ -88,14 +87,6 @@ public <T, C> T accept(AbstractNodeVisitor<T, C> nodeVisitor, C context) {
8887
return nodeVisitor.visitJoin(this, context);
8988
}
9089

91-
/**
92-
* @return `overwrite` option value in argumentMap
93-
*/
94-
public boolean isOverwrite() {
95-
return getArgumentMap().get("overwrite") == null // 'overwrite' default value is true
96-
|| getArgumentMap().get("overwrite").equals(Literal.TRUE);
97-
}
98-
9990
public enum JoinType {
10091
INNER,
10192
LEFT,

0 commit comments

Comments
 (0)