diff --git a/async-query-core/build.gradle b/async-query-core/build.gradle index 09500cfa1ca..5e9ce267676 100644 --- a/async-query-core/build.gradle +++ b/async-query-core/build.gradle @@ -43,7 +43,7 @@ configurations { } dependencies { - antlr "org.antlr:antlr4:4.13.2" + antlr "org.antlr:antlr4:4.7.1" implementation project(':core') implementation 'org.json:json:20231013' diff --git a/build.gradle b/build.gradle index 7c672cc2f22..1dd948e0472 100644 --- a/build.gradle +++ b/build.gradle @@ -6,7 +6,7 @@ buildscript { ext { - opensearch_version = System.getProperty("opensearch.version", "3.6.0-SNAPSHOT") + opensearch_version = System.getProperty("opensearch.version", "3.5.0-SNAPSHOT") isSnapshot = "true" == System.getProperty("build.snapshot", "true") buildVersionQualifier = System.getProperty("build.version_qualifier", "") version_tokens = opensearch_version.tokenize('-') diff --git a/common/build.gradle b/common/build.gradle index d839466f886..43b98db6d80 100644 --- a/common/build.gradle +++ b/common/build.gradle @@ -33,7 +33,7 @@ repositories { } dependencies { - api "org.antlr:antlr4-runtime:4.13.2" + api "org.antlr:antlr4-runtime:4.7.1" api group: 'com.google.guava', name: 'guava', version: "${guava_version}" api group: 'org.apache.logging.log4j', name: 'log4j-core', version:"${versions.log4j}" api group: 'org.apache.commons', name: 'commons-lang3', version: "${commons_lang3_version}" diff --git a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java index 18cead76ee7..2d1cd73f9a0 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java @@ -82,8 +82,6 @@ import org.opensearch.sql.ast.tree.ML; import org.opensearch.sql.ast.tree.Multisearch; import org.opensearch.sql.ast.tree.MvCombine; -import org.opensearch.sql.ast.tree.MvExpand; -import org.opensearch.sql.ast.tree.NoMv; import org.opensearch.sql.ast.tree.Paginate; import org.opensearch.sql.ast.tree.Parse; import org.opensearch.sql.ast.tree.Patterns; @@ -549,16 +547,6 @@ public LogicalPlan visitMvCombine(MvCombine node, AnalysisContext context) { throw getOnlyForCalciteException("mvcombine"); } - @Override - public LogicalPlan visitNoMv(NoMv node, AnalysisContext context) { - throw getOnlyForCalciteException("nomv"); - } - - @Override - public LogicalPlan visitMvExpand(MvExpand node, AnalysisContext context) { - throw getOnlyForCalciteException("mvexpand"); - } - @Override public LogicalPlan visitGraphLookup(GraphLookup node, AnalysisContext context) { throw getOnlyForCalciteException("graphlookup"); diff --git a/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java b/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java index 229e76abb8c..45a8202e848 100644 --- a/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java @@ -70,8 +70,6 @@ import org.opensearch.sql.ast.tree.ML; import org.opensearch.sql.ast.tree.Multisearch; import org.opensearch.sql.ast.tree.MvCombine; -import org.opensearch.sql.ast.tree.MvExpand; -import org.opensearch.sql.ast.tree.NoMv; import org.opensearch.sql.ast.tree.Paginate; import org.opensearch.sql.ast.tree.Parse; import org.opensearch.sql.ast.tree.Patterns; @@ -479,14 +477,6 @@ public T visitMvCombine(MvCombine node, C context) { return visitChildren(node, context); } - public T visitNoMv(NoMv node, C context) { - return visitChildren(node, context); - } - - public T visitMvExpand(MvExpand node, C context) { - return visitChildren(node, context); - } - public T visitGraphLookup(GraphLookup node, C context) { return visitChildren(node, context); } diff --git a/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java b/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java index b2731ebbd40..8b129c6267a 100644 --- a/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java +++ b/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java @@ -63,7 +63,6 @@ import org.opensearch.sql.ast.tree.Limit; import org.opensearch.sql.ast.tree.MinSpanBin; import org.opensearch.sql.ast.tree.MvCombine; -import org.opensearch.sql.ast.tree.MvExpand; import org.opensearch.sql.ast.tree.Parse; import org.opensearch.sql.ast.tree.Patterns; import org.opensearch.sql.ast.tree.Project; @@ -478,16 +477,6 @@ public static MvCombine mvcombine(Field field, String delim) { return new MvCombine(field, delim); } - /** - * Build an MVEXPAND plan node and attach it to the input plan. - * - *

`@param` input input plan `@param` field field to expand `@param` limit optional - * per-document limit `@return` MvExpand plan attached to the input - */ - public static UnresolvedPlan mvexpand(UnresolvedPlan input, Field field, Integer limit) { - return new MvExpand(field, limit).attach(input); - } - public static List sortOptions() { return exprList(argument("desc", booleanLiteral(false))); } diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/MvExpand.java b/core/src/main/java/org/opensearch/sql/ast/tree/MvExpand.java deleted file mode 100644 index 29dc89c541b..00000000000 --- a/core/src/main/java/org/opensearch/sql/ast/tree/MvExpand.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.ast.tree; - -import com.google.common.collect.ImmutableList; -import java.util.List; -import javax.annotation.Nullable; -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.ToString; -import org.opensearch.sql.ast.AbstractNodeVisitor; -import org.opensearch.sql.ast.expression.Field; - -/** AST node representing the {@code mvexpand} PPL command: {@code mvexpand [limit=N]}. */ -@ToString -@EqualsAndHashCode(callSuper = false) -public class MvExpand extends UnresolvedPlan { - - private UnresolvedPlan child; - @Getter private final Field field; - @Getter @Nullable private final Integer limit; - - public MvExpand(Field field, @Nullable Integer limit) { - this.field = field; - this.limit = limit; - } - - @Override - public MvExpand attach(UnresolvedPlan child) { - this.child = child; - return this; - } - - @Override - public List getChild() { - return this.child == null ? ImmutableList.of() : ImmutableList.of(this.child); - } - - @Override - public T accept(AbstractNodeVisitor nodeVisitor, C context) { - return nodeVisitor.visitMvExpand(this, context); - } -} diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/NoMv.java b/core/src/main/java/org/opensearch/sql/ast/tree/NoMv.java deleted file mode 100644 index 0b245762814..00000000000 --- a/core/src/main/java/org/opensearch/sql/ast/tree/NoMv.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.ast.tree; - -import com.google.common.collect.ImmutableList; -import java.util.List; -import javax.annotation.Nullable; -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.ToString; -import org.opensearch.sql.ast.AbstractNodeVisitor; -import org.opensearch.sql.ast.expression.DataType; -import org.opensearch.sql.ast.expression.Field; -import org.opensearch.sql.ast.expression.Function; -import org.opensearch.sql.ast.expression.Let; -import org.opensearch.sql.ast.expression.Literal; - -/** - * AST node for the NOMV command. Converts multi-value fields to single-value fields by joining - * array elements with newline delimiter. - */ -@Getter -@ToString(callSuper = true) -@EqualsAndHashCode(callSuper = false) -public class NoMv extends UnresolvedPlan { - - private final Field field; - @Nullable private UnresolvedPlan child; - - public NoMv(Field field) { - this.field = field; - } - - public NoMv attach(UnresolvedPlan child) { - this.child = child; - return this; - } - - @Override - public List getChild() { - return child == null ? ImmutableList.of() : ImmutableList.of(child); - } - - @Override - public T accept(AbstractNodeVisitor nodeVisitor, C context) { - return nodeVisitor.visitNoMv(this, context); - } - - /** - * Rewrites the nomv command as an eval command using mvjoin function with null filtering. nomv - * is rewritten to: eval = coalesce(mvjoin(array_compact(), "\n"), "") - * - *

The array_compact removes null elements from the array, and coalesce ensures empty arrays - * return empty string instead of null. - * - * @return an Eval node representing the equivalent mvjoin operation with null filtering - */ - public UnresolvedPlan rewriteAsEval() { - Function arrayCompactFunc = new Function("array_compact", ImmutableList.of(field)); - - Function mvjoinFunc = - new Function( - "mvjoin", ImmutableList.of(arrayCompactFunc, new Literal("\n", DataType.STRING))); - - Function coalesceFunc = - new Function("coalesce", ImmutableList.of(mvjoinFunc, new Literal("", DataType.STRING))); - - Let letExpr = new Let(field, coalesceFunc); - - Eval eval = new Eval(ImmutableList.of(letExpr)); - if (this.child != null) { - eval.attach(this.child); - } - return eval; - } -} diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/SPath.java b/core/src/main/java/org/opensearch/sql/ast/tree/SPath.java index abfaf3cc0bc..a1c0c08a15f 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/SPath.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/SPath.java @@ -30,7 +30,7 @@ public class SPath extends UnresolvedPlan { @Nullable private final String outField; - @Nullable private final String path; + private final String path; @Override public UnresolvedPlan attach(UnresolvedPlan child) { @@ -48,20 +48,7 @@ public T accept(AbstractNodeVisitor nodeVisitor, C context) { return nodeVisitor.visitSpath(this, context); } - /** - * Rewrites this spath node to an equivalent {@link Eval} node. - * - *

In path mode, rewrites to {@code eval output = json_extract(input, path)}. In auto-extract - * mode (path is null), rewrites to {@code eval output = json_extract_all(input)}. - */ public Eval rewriteAsEval() { - if (path != null) { - return rewritePathMode(); - } - return rewriteAutoExtractMode(); - } - - private Eval rewritePathMode() { String outField = this.outField; String unquotedPath = unquoteText(this.path); if (outField == null) { @@ -75,12 +62,4 @@ private Eval rewritePathMode() { AstDSL.function( "json_extract", AstDSL.field(inField), AstDSL.stringLiteral(unquotedPath)))); } - - private Eval rewriteAutoExtractMode() { - String output = (outField != null) ? outField : inField; - return AstDSL.eval( - child, - AstDSL.let( - AstDSL.field(output), AstDSL.function("json_extract_all", AstDSL.field(inField)))); - } } diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java index 79bf924d36a..8854c80ae63 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java @@ -128,8 +128,6 @@ import org.opensearch.sql.ast.tree.ML; import org.opensearch.sql.ast.tree.Multisearch; import org.opensearch.sql.ast.tree.MvCombine; -import org.opensearch.sql.ast.tree.MvExpand; -import org.opensearch.sql.ast.tree.NoMv; import org.opensearch.sql.ast.tree.Paginate; import org.opensearch.sql.ast.tree.Parse; import org.opensearch.sql.ast.tree.Patterns; @@ -457,16 +455,7 @@ private List expandProjectFields( } matchingFields.forEach(f -> expandedFields.add(context.relBuilder.field(f))); } else if (addedFields.add(fieldName)) { - RexNode resolved = rexVisitor.analyze(field, context); - /* - * Dotted path access is resolved into ITEM(map, path) function call without aliasing. - * Re-apply the alias so the projected column retains the user-visible name. - * TODO: Introduce path navigation semantics without relying on projection-time aliasing. - */ - if (resolved.getKind() == SqlKind.ITEM) { - resolved = context.relBuilder.alias(resolved, fieldName); - } - expandedFields.add(resolved); + expandedFields.add(rexVisitor.analyze(field, context)); } } case AllFields ignored -> { @@ -934,11 +923,7 @@ public RelNode visitPatterns(Patterns node, CalcitePlanContext context) { .toList(); context.relBuilder.aggregate(context.relBuilder.groupKey(groupByList), aggCall); buildExpandRelNode( - context.relBuilder.field(node.getAlias()), - node.getAlias(), - node.getAlias(), - null, - context); + context.relBuilder.field(node.getAlias()), node.getAlias(), node.getAlias(), context); flattenParsedPattern( node.getAlias(), context.relBuilder.field(node.getAlias()), @@ -3196,7 +3181,7 @@ public RelNode visitExpand(Expand expand, CalcitePlanContext context) { RexInputRef arrayFieldRex = (RexInputRef) rexVisitor.analyze(arrayField, context); String alias = expand.getAlias(); - buildExpandRelNode(arrayFieldRex, arrayField.getField().toString(), alias, null, context); + buildExpandRelNode(arrayFieldRex, arrayField.getField().toString(), alias, context); return context.relBuilder.peek(); } @@ -3369,81 +3354,6 @@ private void restoreColumnOrderAfterArrayAgg( relBuilder.project(projections, projectionNames, /* force= */ true); } - /** - * Visits a NoMv (no multivalue) node by rewriting it as an Eval node. - * - *

The NoMv command converts multivalue (array) fields to single-value strings by joining array - * elements with newline delimiters. Internally, NoMv rewrites itself to an Eval node containing a - * mvjoin function call: {@code eval field = mvjoin(field, "\n")}. - * - *

The explicit cast to Eval is safe because {@link NoMv#rewriteAsEval()} always returns a - * newly constructed Eval instance and never returns null or other types. - * - * @param node the NoMv node to visit - * @param context the Calcite plan context containing schema and optimization information - * @return the RelNode resulting from visiting the rewritten Eval node - * @see NoMv#rewriteAsEval() - */ - @Override - public RelNode visitNoMv(NoMv node, CalcitePlanContext context) { - return visitEval((Eval) node.rewriteAsEval(), context); - } - - /** - * MVExpand command visitor. - * - *

Expands a multi-value (array) field into separate rows using Calcite's CORRELATE join with - * UNCOLLECT. Each element of the array becomes a separate row while preserving all other fields - * from the original row. - * - *

Implementation uses {@link #buildExpandRelNode} to create a correlate join between the - * original relation and an uncollected (unnested) version of the target array field. - * - *

Behavior: - * - *

- * - * @param mvExpand MVExpand command containing the field to expand and optional limit - * @param context CalcitePlanContext containing the RelBuilder and planning context - * @return RelNode representing the relation with the expanded multi-value field - * @throws SemanticCheckException if the target field does not exist in the schema - */ - @Override - public RelNode visitMvExpand(MvExpand mvExpand, CalcitePlanContext context) { - visitChildren(mvExpand, context); - - final RelBuilder relBuilder = context.relBuilder; - final Field field = mvExpand.getField(); - final String fieldName = field.getField().toString(); - - final RelDataType inputType = relBuilder.peek().getRowType(); - final RelDataTypeField inputField = - inputType.getField(fieldName, /*caseSensitive*/ true, /*elideRecord*/ false); - - if (inputField == null) { - throw new SemanticCheckException( - String.format("Field '%s' not found in the schema", fieldName)); - } - - final RexInputRef arrayFieldRex = (RexInputRef) rexVisitor.analyze(field, context); - - final RelDataType fieldType = arrayFieldRex.getType(); - if (!(SqlTypeUtil.isArray(fieldType) || SqlTypeUtil.isMultiset(fieldType))) { - // For non-array/multiset fields (scalars), mvexpand just returns the field unchanged. - // This treats single-value fields as if they were arrays with one element. - return relBuilder.peek(); - } - - buildExpandRelNode(arrayFieldRex, fieldName, fieldName, mvExpand.getLimit(), context); - - return relBuilder.peek(); - } - @Override public RelNode visitValues(Values values, CalcitePlanContext context) { if (values.getValues() == null || values.getValues().isEmpty()) { @@ -3688,11 +3598,7 @@ private void flattenParsedPattern( } private void buildExpandRelNode( - RexInputRef arrayFieldRex, - String arrayFieldName, - String alias, - @Nullable Integer perDocLimit, - CalcitePlanContext context) { + RexInputRef arrayFieldRex, String arrayFieldName, String alias, CalcitePlanContext context) { // 3. Capture the outer row in a CorrelationId Holder correlVariable = Holder.empty(); context.relBuilder.variable(correlVariable::set); @@ -3707,17 +3613,14 @@ private void buildExpandRelNode( RelNode leftNode = context.relBuilder.build(); // 5. Build join right node and expand the array field using uncollect - context - .relBuilder - // fake input, see convertUnnest and convertExpression in Calcite SqlToRelConverter - .push(LogicalValues.createOneRow(context.relBuilder.getCluster())) - .project(List.of(correlArrayFieldAccess), List.of(arrayFieldName)) - .uncollect(List.of(), false); - - if (perDocLimit != null) { - context.relBuilder.limit(0, perDocLimit); - } - RelNode rightNode = context.relBuilder.build(); + RelNode rightNode = + context + .relBuilder + // fake input, see convertUnnest and convertExpression in Calcite SqlToRelConverter + .push(LogicalValues.createOneRow(context.relBuilder.getCluster())) + .project(List.of(correlArrayFieldAccess), List.of(arrayFieldName)) + .uncollect(List.of(), false) + .build(); // 6. Perform a nested-loop join (correlate) between the original table and the expanded // array field. diff --git a/core/src/main/java/org/opensearch/sql/calcite/QualifiedNameResolver.java b/core/src/main/java/org/opensearch/sql/calcite/QualifiedNameResolver.java index 917e907a806..a6fcfda4635 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/QualifiedNameResolver.java +++ b/core/src/main/java/org/opensearch/sql/calcite/QualifiedNameResolver.java @@ -95,7 +95,7 @@ private static RexNode resolveInNonJoinCondition( private static String joinParts(List parts, int start, int length) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < length; i++) { - if (i > 0) { + if (start < i) { sb.append("."); } sb.append(parts.get(start + i)); @@ -289,7 +289,9 @@ private static RexNode resolveFieldAccess( return field; } else { String itemName = joinParts(parts, length + start, parts.size() - length); - return createItemAccess(field, itemName, context); + return context.relBuilder.alias( + createItemAccess(field, itemName, context), + String.join(QualifiedName.DELIMITER, parts.subList(start, parts.size()))); } } diff --git a/core/src/main/java/org/opensearch/sql/executor/QueryService.java b/core/src/main/java/org/opensearch/sql/executor/QueryService.java index bebd50a5e87..8edc3ad3f2c 100644 --- a/core/src/main/java/org/opensearch/sql/executor/QueryService.java +++ b/core/src/main/java/org/opensearch/sql/executor/QueryService.java @@ -64,24 +64,6 @@ public class QueryService { @Getter(lazy = true) private final CalciteRelNodeVisitor relNodeVisitor = new CalciteRelNodeVisitor(dataSourceService); - /** Helper: depending on the type of error, either re-raise or propagate to the listener. */ - private void propagateCalciteError(Throwable t, ResponseListener listener) - throws VirtualMachineError { - if (t instanceof VirtualMachineError) { - // throw and fast fail the VM errors such as OOM (same with v2). - throw (VirtualMachineError) t; - } - if (t instanceof Exception) { - listener.onFailure((Exception) t); - } else if (t instanceof ExceptionInInitializerError - && ((ExceptionInInitializerError) t).getException() instanceof Exception) { - listener.onFailure((Exception) ((ExceptionInInitializerError) t).getException()); - } else { - // Calcite may throw AssertError during query execution. - listener.onFailure(new CalciteUnsupportedException(t.getMessage(), t)); - } - } - /** Execute the {@link UnresolvedPlan}, using {@link ResponseListener} to get response.
*/ public void execute( UnresolvedPlan plan, @@ -130,7 +112,18 @@ public void executeWithCalcite( log.warn("Fallback to V2 query engine since got exception", t); executeWithLegacy(plan, queryType, listener, Optional.of(t)); } else { - propagateCalciteError(t, listener); + if (t instanceof Exception) { + listener.onFailure((Exception) t); + } else if (t instanceof ExceptionInInitializerError + && ((ExceptionInInitializerError) t).getException() instanceof Exception) { + listener.onFailure((Exception) ((ExceptionInInitializerError) t).getException()); + } else if (t instanceof VirtualMachineError) { + // throw and fast fail the VM errors such as OOM (same with v2). + throw t; + } else { + // Calcite may throw AssertError during query execution. + listener.onFailure(new CalciteUnsupportedException(t.getMessage(), t)); + } } } }, @@ -161,7 +154,12 @@ public void explainWithCalcite( log.warn("Fallback to V2 query engine since got exception", t); explainWithLegacy(plan, queryType, listener, mode, Optional.of(t)); } else { - propagateCalciteError(t, listener); + if (t instanceof Error) { + // Calcite may throw AssertError during query execution. + listener.onFailure(new CalciteUnsupportedException(t.getMessage(), t)); + } else { + listener.onFailure((Exception) t); + } } } }, @@ -176,11 +174,11 @@ public void executeWithLegacy( try { executePlan(analyze(plan, queryType), PlanContext.emptyPlanContext(), listener); } catch (Exception e) { - if (calciteFailure.isPresent()) { - // This happens if Calcite fell back to V2 due to some issue, and then V2 also failed. - // Prefer the Calcite error. - // https://github.com/opensearch-project/sql/issues/5060 - propagateCalciteError(calciteFailure.get(), listener); + if (shouldUseCalcite(queryType) && isCalciteFallbackAllowed(null)) { + // if there is a failure thrown from Calcite and execution after fallback V2 + // keeps failure, we should throw the failure from Calcite. + calciteFailure.ifPresentOrElse( + t -> listener.onFailure(new RuntimeException(t)), () -> listener.onFailure(e)); } else { listener.onFailure(e); } @@ -209,11 +207,11 @@ public void explainWithLegacy( } executionEngine.explain(plan(analyze(plan, queryType)), listener); } catch (Exception e) { - if (calciteFailure.isPresent()) { - // This happens if Calcite fell back to V2 due to some issue, and then V2 also failed. - // Prefer the Calcite error. - // https://github.com/opensearch-project/sql/issues/5060 - propagateCalciteError(calciteFailure.get(), listener); + if (shouldUseCalcite(queryType) && isCalciteFallbackAllowed(null)) { + // if there is a failure thrown from Calcite and execution after fallback V2 + // keeps failure, we should throw the failure from Calcite. + calciteFailure.ifPresentOrElse( + t -> listener.onFailure(new RuntimeException(t)), () -> listener.onFailure(e)); } else { listener.onFailure(e); } diff --git a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java index 3171a09d39a..dce558bf7cc 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java @@ -71,7 +71,6 @@ public enum BuiltinFunctionName { ARRAY(FunctionName.of("array")), ARRAY_LENGTH(FunctionName.of("array_length")), ARRAY_SLICE(FunctionName.of("array_slice"), true), - ARRAY_COMPACT(FunctionName.of("array_compact")), MAP_APPEND(FunctionName.of("map_append"), true), MAP_CONCAT(FunctionName.of("map_concat"), true), MAP_REMOVE(FunctionName.of("map_remove"), true), diff --git a/core/src/main/java/org/opensearch/sql/expression/function/CollectionUDF/ArrayFunctionImpl.java b/core/src/main/java/org/opensearch/sql/expression/function/CollectionUDF/ArrayFunctionImpl.java index 9a77a0d5a7c..2e9e53b9ac2 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/CollectionUDF/ArrayFunctionImpl.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/CollectionUDF/ArrayFunctionImpl.java @@ -35,7 +35,7 @@ */ public class ArrayFunctionImpl extends ImplementorUDF { public ArrayFunctionImpl() { - super(new ArrayImplementor(), NullPolicy.NONE); + super(new ArrayImplementor(), NullPolicy.ANY); } /** @@ -81,8 +81,7 @@ public Expression implement( /** * The asList will generate the List. We need to convert internally, otherwise, the - * calcite will directly cast like DOUBLE -> INTEGER, which throw error. Null elements are - * preserved in the array. + * calcite will directly cast like DOUBLE -> INTEGER, which throw error */ public static Object internalCast(Object... args) { List originalList = (List) args[0]; @@ -94,9 +93,7 @@ public static Object internalCast(Object... args) { originalList.stream() .map( num -> { - if (num == null) { - return null; - } else if (num instanceof BigDecimal) { + if (num instanceof BigDecimal) { return (BigDecimal) num; } else { return BigDecimal.valueOf(((Number) num).doubleValue()); @@ -107,20 +104,17 @@ public static Object internalCast(Object... args) { case DOUBLE: result = originalList.stream() - .map(i -> i == null ? null : (Object) ((Number) i).doubleValue()) + .map(i -> (Object) ((Number) i).doubleValue()) .collect(Collectors.toList()); break; case FLOAT: result = originalList.stream() - .map(i -> i == null ? null : (Object) ((Number) i).floatValue()) + .map(i -> (Object) ((Number) i).floatValue()) .collect(Collectors.toList()); break; case VARCHAR, CHAR: - result = - originalList.stream() - .map(i -> i == null ? null : (Object) i.toString()) - .collect(Collectors.toList()); + result = originalList.stream().map(i -> (Object) i.toString()).collect(Collectors.toList()); break; default: result = originalList; diff --git a/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java b/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java index 89a8f59397b..205f3a0f2e1 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java @@ -16,7 +16,6 @@ import static org.opensearch.sql.expression.function.BuiltinFunctionName.ADDTIME; import static org.opensearch.sql.expression.function.BuiltinFunctionName.AND; import static org.opensearch.sql.expression.function.BuiltinFunctionName.ARRAY; -import static org.opensearch.sql.expression.function.BuiltinFunctionName.ARRAY_COMPACT; import static org.opensearch.sql.expression.function.BuiltinFunctionName.ARRAY_LENGTH; import static org.opensearch.sql.expression.function.BuiltinFunctionName.ARRAY_SLICE; import static org.opensearch.sql.expression.function.BuiltinFunctionName.ASCII; @@ -991,7 +990,12 @@ void populate() { PPLTypeChecker.family(SqlTypeFamily.ANY)); // Register MVJOIN to use Calcite's ARRAY_JOIN - registerOperator(MVJOIN, SqlLibraryOperators.ARRAY_JOIN); + register( + MVJOIN, + (FunctionImp2) + (builder, array, delimiter) -> + builder.makeCall(SqlLibraryOperators.ARRAY_JOIN, array, delimiter), + PPLTypeChecker.family(SqlTypeFamily.ARRAY, SqlTypeFamily.CHARACTER)); // Register SPLIT with custom logic for empty delimiter // Case 1: Delimiter is not empty string, use SPLIT @@ -1044,7 +1048,6 @@ void populate() { registerOperator(MAP_REMOVE, PPLBuiltinOperators.MAP_REMOVE); registerOperator(ARRAY_LENGTH, SqlLibraryOperators.ARRAY_LENGTH); registerOperator(ARRAY_SLICE, SqlLibraryOperators.ARRAY_SLICE); - registerOperator(ARRAY_COMPACT, SqlLibraryOperators.ARRAY_COMPACT); registerOperator(FORALL, PPLBuiltinOperators.FORALL); registerOperator(EXISTS, PPLBuiltinOperators.EXISTS); registerOperator(FILTER, PPLBuiltinOperators.FILTER); @@ -1105,10 +1108,6 @@ void populate() { OperandTypes.family(SqlTypeFamily.ARRAY, SqlTypeFamily.INTEGER) .or(OperandTypes.family(SqlTypeFamily.MAP, SqlTypeFamily.ANY)), false)); - registerOperator( - INTERNAL_ITEM, - SqlStdOperatorTable.ITEM, - PPLTypeChecker.family(SqlTypeFamily.IGNORE, SqlTypeFamily.CHARACTER)); registerOperator( XOR, SqlStdOperatorTable.NOT_EQUALS, diff --git a/core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractAllFunctionImpl.java b/core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractAllFunctionImpl.java index 8168700b6da..1f91c87bb77 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractAllFunctionImpl.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractAllFunctionImpl.java @@ -5,7 +5,6 @@ package org.opensearch.sql.expression.function.jsonUDF; -import static java.util.stream.Collectors.toMap; import static org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.TYPE_FACTORY; import com.fasterxml.jackson.core.JsonFactory; @@ -52,7 +51,7 @@ public SqlReturnTypeInference getReturnTypeInference() { return ReturnTypes.explicit( TYPE_FACTORY.createMapType( TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR), - TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR), + TYPE_FACTORY.createSqlType(SqlTypeName.ANY), true)); } @@ -73,11 +72,6 @@ public Expression implement( } } - /** - * Evaluate the JSON extract-all function. Returns a {@code Map} where keys are - * dot-separated JSON paths (with {@code {}} suffix for arrays) and all values are strings. Merged - * array values use {@code [a, b, c]} format. - */ public static Object eval(Object... args) { if (args.length < 1) { return null; @@ -88,18 +82,7 @@ public static Object eval(Object... args) { return null; } - Map parsed = parseJson(jsonStr); - return parsed == null ? null : stringifyMap(parsed); - } - - // TODO: JSON parsing dominates cost; consider stringify scalars in place during parsing - // to avoid this extra pass. - private static Map stringifyMap(Map map) { - return map.entrySet().stream() - .collect( - toMap( - Map.Entry::getKey, - e -> String.valueOf(e.getValue()))); // relies on List.toString() for [a, b, c] + return parseJson(jsonStr); } private static Map parseJson(String jsonStr) { @@ -167,7 +150,7 @@ private static Map parseJson(String jsonStr) { @SuppressWarnings("unchecked") private static void appendValue(Map resultMap, String path, Object value) { Object existingValue = resultMap.get(path); - if (existingValue == null && !resultMap.containsKey(path)) { // key absent, not null value + if (existingValue == null) { resultMap.put(path, value); } else if (existingValue instanceof List) { ((List) existingValue).add(value); diff --git a/core/src/test/java/org/opensearch/sql/executor/QueryServiceTest.java b/core/src/test/java/org/opensearch/sql/executor/QueryServiceTest.java index e7db5797a59..0ee97754c66 100644 --- a/core/src/test/java/org/opensearch/sql/executor/QueryServiceTest.java +++ b/core/src/test/java/org/opensearch/sql/executor/QueryServiceTest.java @@ -91,124 +91,6 @@ public void analyzeExceptionShouldBeCached() { queryService().analyzeFail().handledByOnFailure(); } - @Test - public void testExecuteWithLegacyShouldReturnCalciteErrorWhenBothFail() { - UnsupportedOperationException calciteException = - new UnsupportedOperationException("Calcite error"); - IllegalStateException v2Exception = new IllegalStateException("V2 error"); - - ResponseListener responseListener = - new ResponseListener<>() { - @Override - public void onResponse(ExecutionEngine.QueryResponse pplQueryResponse) { - fail("Expected onFailure to be called"); - } - - @Override - public void onFailure(Exception e) { - // Should get the Calcite error directly (not wrapped), not the V2 error - assertNotNull(e); - assertTrue(e instanceof UnsupportedOperationException); - assertTrue(e.getMessage().contains("Calcite error")); - } - }; - - lenient().when(settings.getSettingValue(Key.CALCITE_ENGINE_ENABLED)).thenReturn(false); - lenient().when(analyzer.analyze(any(), any())).thenThrow(v2Exception); - - QueryService service = new QueryService(analyzer, executionEngine, planner, null, settings); - service.executeWithLegacy(ast, queryType, responseListener, Optional.of(calciteException)); - } - - @Test - public void testExplainWithLegacyShouldReturnCalciteErrorWhenBothFail() { - UnsupportedOperationException calciteException = - new UnsupportedOperationException("Calcite error"); - IllegalStateException v2Exception = new IllegalStateException("V2 error"); - - ResponseListener responseListener = - new ResponseListener<>() { - @Override - public void onResponse(ExecutionEngine.ExplainResponse explainResponse) { - fail("Expected onFailure to be called"); - } - - @Override - public void onFailure(Exception e) { - // Should get the Calcite error directly (not wrapped), not the V2 error - assertNotNull(e); - assertTrue(e instanceof UnsupportedOperationException); - assertTrue(e.getMessage().contains("Calcite error")); - } - }; - - lenient().when(settings.getSettingValue(Key.CALCITE_ENGINE_ENABLED)).thenReturn(false); - lenient().when(analyzer.analyze(any(), any())).thenThrow(v2Exception); - - QueryService service = new QueryService(analyzer, executionEngine, planner, null, settings); - service.explainWithLegacy( - ast, queryType, responseListener, ExplainMode.STANDARD, Optional.of(calciteException)); - } - - @Test - public void testExecuteWithLegacyShouldWrapCalciteErrorInCalciteUnsupportedException() { - AssertionError calciteError = new AssertionError("Calcite assertion failed"); - IllegalStateException v2Exception = new IllegalStateException("V2 error"); - - ResponseListener responseListener = - new ResponseListener<>() { - @Override - public void onResponse(ExecutionEngine.QueryResponse pplQueryResponse) { - fail("Expected onFailure to be called"); - } - - @Override - public void onFailure(Exception e) { - // Errors should be wrapped in CalciteUnsupportedException - assertNotNull(e); - assertTrue(e instanceof org.opensearch.sql.exception.CalciteUnsupportedException); - assertTrue(e.getCause() instanceof AssertionError); - assertTrue(e.getMessage().contains("Calcite assertion failed")); - } - }; - - lenient().when(settings.getSettingValue(Key.CALCITE_ENGINE_ENABLED)).thenReturn(false); - lenient().when(analyzer.analyze(any(), any())).thenThrow(v2Exception); - - QueryService service = new QueryService(analyzer, executionEngine, planner, null, settings); - service.executeWithLegacy(ast, queryType, responseListener, Optional.of(calciteError)); - } - - @Test - public void testExplainWithLegacyShouldWrapCalciteErrorInCalciteUnsupportedException() { - AssertionError calciteError = new AssertionError("Calcite assertion failed"); - IllegalStateException v2Exception = new IllegalStateException("V2 error"); - - ResponseListener responseListener = - new ResponseListener<>() { - @Override - public void onResponse(ExecutionEngine.ExplainResponse explainResponse) { - fail("Expected onFailure to be called"); - } - - @Override - public void onFailure(Exception e) { - // Errors should be wrapped in CalciteUnsupportedException - assertNotNull(e); - assertTrue(e instanceof org.opensearch.sql.exception.CalciteUnsupportedException); - assertTrue(e.getCause() instanceof AssertionError); - assertTrue(e.getMessage().contains("Calcite assertion failed")); - } - }; - - lenient().when(settings.getSettingValue(Key.CALCITE_ENGINE_ENABLED)).thenReturn(false); - lenient().when(analyzer.analyze(any(), any())).thenThrow(v2Exception); - - QueryService service = new QueryService(analyzer, executionEngine, planner, null, settings); - service.explainWithLegacy( - ast, queryType, responseListener, ExplainMode.STANDARD, Optional.of(calciteError)); - } - Helper queryService() { return new Helper(); } diff --git a/core/src/test/java/org/opensearch/sql/expression/function/CollectionUDF/ArrayFunctionImplTest.java b/core/src/test/java/org/opensearch/sql/expression/function/CollectionUDF/ArrayFunctionImplTest.java deleted file mode 100644 index 6dbc1901fa7..00000000000 --- a/core/src/test/java/org/opensearch/sql/expression/function/CollectionUDF/ArrayFunctionImplTest.java +++ /dev/null @@ -1,305 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.expression.function.CollectionUDF; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import java.util.stream.Collectors; -import org.apache.calcite.sql.type.SqlTypeName; -import org.junit.jupiter.api.Test; - -/** - * Unit tests for ArrayFunctionImpl. - * - *

These tests verify that the array() function correctly handles null elements inside arrays, - * which is critical for the NOMV command's null filtering functionality via ARRAY_COMPACT. - * - *

The array() function uses NullPolicy.NONE, meaning it accepts null arguments and preserves - * them inside the resulting array. This allows ARRAY_COMPACT to filter them out later. - */ -public class ArrayFunctionImplTest { - - @Test - public void testArrayWithNoArguments() { - Object result = ArrayFunctionImpl.internalCast(Collections.emptyList(), SqlTypeName.VARCHAR); - assertNotNull(result, "Empty array should not be null"); - assertTrue(result instanceof List, "Result should be a List"); - assertEquals(0, ((List) result).size(), "Empty array should have size 0"); - } - - @Test - public void testArrayWithSingleElement() { - Object result = ArrayFunctionImpl.internalCast(Arrays.asList("test"), SqlTypeName.VARCHAR); - assertNotNull(result); - assertTrue(result instanceof List); - assertEquals(Arrays.asList("test"), result); - } - - @Test - public void testArrayWithMultipleElements() { - Object result = - ArrayFunctionImpl.internalCast(Arrays.asList("a", "b", "c"), SqlTypeName.VARCHAR); - assertNotNull(result); - assertEquals(Arrays.asList("a", "b", "c"), result); - } - - // ==================== NULL HANDLING TESTS ==================== - // These tests are critical for NOMV command's null filtering - - @Test - public void testArrayWithNullInMiddle() { - Object result = - ArrayFunctionImpl.internalCast(Arrays.asList("a", null, "b"), SqlTypeName.VARCHAR); - assertNotNull(result, "Array with null should not return null"); - assertTrue(result instanceof List); - List list = (List) result; - assertEquals(3, list.size(), "Array should preserve null element"); - assertEquals("a", list.get(0)); - assertNull(list.get(1), "Middle element should be null"); - assertEquals("b", list.get(2)); - } - - @Test - public void testArrayWithNullAtBeginning() { - Object result = - ArrayFunctionImpl.internalCast(Arrays.asList(null, "a", "b"), SqlTypeName.VARCHAR); - assertNotNull(result, "Array with null should not return null"); - assertTrue(result instanceof List); - List list = (List) result; - assertEquals(3, list.size(), "Array should preserve null element"); - assertNull(list.get(0), "First element should be null"); - assertEquals("a", list.get(1)); - assertEquals("b", list.get(2)); - } - - @Test - public void testArrayWithNullAtEnd() { - Object result = - ArrayFunctionImpl.internalCast(Arrays.asList("a", "b", null), SqlTypeName.VARCHAR); - assertNotNull(result, "Array with null should not return null"); - assertTrue(result instanceof List); - List list = (List) result; - assertEquals(3, list.size(), "Array should preserve null element"); - assertEquals("a", list.get(0)); - assertEquals("b", list.get(1)); - assertNull(list.get(2), "Last element should be null"); - } - - @Test - public void testArrayWithMultipleNulls() { - Object result = - ArrayFunctionImpl.internalCast( - Arrays.asList("a", null, "b", null, "c"), SqlTypeName.VARCHAR); - assertNotNull(result, "Array with nulls should not return null"); - assertTrue(result instanceof List); - List list = (List) result; - assertEquals(5, list.size(), "Array should preserve all null elements"); - assertEquals("a", list.get(0)); - assertNull(list.get(1), "Second element should be null"); - assertEquals("b", list.get(2)); - assertNull(list.get(3), "Fourth element should be null"); - assertEquals("c", list.get(4)); - } - - @Test - public void testArrayWithAllNulls() { - Object result = - ArrayFunctionImpl.internalCast(Arrays.asList(null, null, null), SqlTypeName.VARCHAR); - assertNotNull(result, "Array of all nulls should not return null"); - assertTrue(result instanceof List); - List list = (List) result; - assertEquals(3, list.size(), "Array should preserve all null elements"); - assertNull(list.get(0)); - assertNull(list.get(1)); - assertNull(list.get(2)); - } - - @Test - public void testArrayWithSingleNull() { - Object result = - ArrayFunctionImpl.internalCast(Collections.singletonList(null), SqlTypeName.VARCHAR); - assertNotNull(result, "Array with single null should not return null"); - assertTrue(result instanceof List); - List list = (List) result; - assertEquals(1, list.size(), "Array should contain one null element"); - assertNull(list.get(0)); - } - - @Test - public void testArrayWithMixedTypesAndNulls() { - Object result = - ArrayFunctionImpl.internalCast( - Arrays.asList(1, null, "text", null, 3.14), SqlTypeName.VARCHAR); - assertNotNull(result, "Array with mixed types and nulls should not return null"); - assertTrue(result instanceof List); - List list = (List) result; - assertEquals(5, list.size()); - assertEquals("1", list.get(0)); // Converted to string - assertNull(list.get(1)); - assertEquals("text", list.get(2)); - assertNull(list.get(3)); - assertEquals("3.14", list.get(4)); // Converted to string - } - - // ==================== INTEGRATION WITH NOMV WORKFLOW ==================== - // These tests verify the array works correctly in the NOMV workflow: - // array(fields) -> array_compact(array) -> mvjoin(compacted, '\n') -> coalesce(result, '') - - @Test - public void testArrayOutputCanBeProcessedByArrayCompact() { - // Simulate: array(field1, null, field2) -> array_compact - Object arrayResult = - ArrayFunctionImpl.internalCast( - Arrays.asList("value1", null, "value2"), SqlTypeName.VARCHAR); - assertNotNull(arrayResult); - assertTrue(arrayResult instanceof List); - - // Verify the array has the structure expected by array_compact - List list = (List) arrayResult; - assertEquals(3, list.size(), "Array should have 3 elements before compacting"); - - // Simulate what array_compact would do (filter out nulls) - List compacted = list.stream().filter(item -> item != null).collect(Collectors.toList()); - assertEquals(2, compacted.size(), "After compacting, array should have 2 elements"); - assertEquals("value1", compacted.get(0)); - assertEquals("value2", compacted.get(1)); - } - - @Test - public void testArrayWithAllNullsForNomvWorkflow() { - // RFC Example 9: array(null, null, null) should allow NOMV to return "" - Object arrayResult = - ArrayFunctionImpl.internalCast(Arrays.asList(null, null, null), SqlTypeName.VARCHAR); - assertNotNull(arrayResult); - assertTrue(arrayResult instanceof List); - - List list = (List) arrayResult; - assertEquals(3, list.size(), "Array should preserve all nulls"); - - // Simulate array_compact - should result in empty array - List compacted = - list.stream().filter(item -> item != null).collect(java.util.stream.Collectors.toList()); - assertEquals( - 0, compacted.size(), "After compacting all nulls, array should be empty for NOMV to use"); - } - - @Test - public void testArrayPreservesNullsForRFCExample5() { - // RFC Example 5: nomv should filter nulls - // array('a', null, 'b') -> array_compact -> ['a', 'b'] -> mvjoin -> "a\nb" - Object arrayResult = - ArrayFunctionImpl.internalCast(Arrays.asList("a", null, "b"), SqlTypeName.VARCHAR); - assertNotNull(arrayResult); - - List list = (List) arrayResult; - assertEquals(3, list.size(), "Original array should have 3 elements"); - assertNull(list.get(1), "Middle element should be null"); - - // After array_compact - List compacted = - list.stream().filter(item -> item != null).collect(java.util.stream.Collectors.toList()); - assertEquals(2, compacted.size()); - assertEquals("a", compacted.get(0)); - assertEquals("b", compacted.get(1)); - } - - // ==================== EDGE CASE TESTS ==================== - - @Test - public void testArrayWithNumericTypes() { - Object result = ArrayFunctionImpl.internalCast(Arrays.asList(1, 2, 3), SqlTypeName.INTEGER); - assertNotNull(result); - assertEquals(Arrays.asList(1, 2, 3), result); - } - - @Test - public void testArrayWithMixedNumericAndString() { - Object result = ArrayFunctionImpl.internalCast(Arrays.asList(1, "two", 3), SqlTypeName.VARCHAR); - assertNotNull(result); - assertEquals(Arrays.asList("1", "two", "3"), result); - } - - @Test - public void testArrayWithEmptyStrings() { - // Empty strings should be preserved (they are not null) - Object result = - ArrayFunctionImpl.internalCast(Arrays.asList("a", "", "b"), SqlTypeName.VARCHAR); - assertNotNull(result); - List list = (List) result; - assertEquals(3, list.size()); - assertEquals("a", list.get(0)); - assertEquals("", list.get(1), "Empty string should be preserved"); - assertEquals("b", list.get(2)); - } - - @Test - public void testArrayWithBooleanValues() { - Object result = - ArrayFunctionImpl.internalCast(Arrays.asList(true, false, null), SqlTypeName.BOOLEAN); - assertNotNull(result); - List list = (List) result; - assertEquals(3, list.size()); - assertEquals(true, list.get(0)); - assertEquals(false, list.get(1)); - assertNull(list.get(2)); - } - - // ==================== TYPE CONVERSION TESTS ==================== - // Test that internalCast correctly handles type conversions while preserving nulls - - @Test - public void testArrayWithDoubleTypePreservesNulls() { - Object result = - ArrayFunctionImpl.internalCast(Arrays.asList(1.5, null, 2.7), SqlTypeName.DOUBLE); - assertNotNull(result); - List list = (List) result; - assertEquals(3, list.size()); - assertEquals(1.5, list.get(0)); - assertNull(list.get(1), "Null should be preserved during DOUBLE type conversion"); - assertEquals(2.7, list.get(2)); - } - - @Test - public void testArrayWithFloatTypePreservesNulls() { - Object result = - ArrayFunctionImpl.internalCast(Arrays.asList(1.5f, null, 2.7f), SqlTypeName.FLOAT); - assertNotNull(result); - List list = (List) result; - assertEquals(3, list.size()); - assertEquals(1.5f, list.get(0)); - assertNull(list.get(1), "Null should be preserved during FLOAT type conversion"); - assertEquals(2.7f, list.get(2)); - } - - @Test - public void testArrayWithVarcharTypePreservesNulls() { - Object result = - ArrayFunctionImpl.internalCast(Arrays.asList("a", null, "b"), SqlTypeName.VARCHAR); - assertNotNull(result); - List list = (List) result; - assertEquals(3, list.size()); - assertEquals("a", list.get(0)); - assertNull(list.get(1), "Null should be preserved during VARCHAR type conversion"); - assertEquals("b", list.get(2)); - } - - @Test - public void testArrayWithCharTypePreservesNulls() { - Object result = ArrayFunctionImpl.internalCast(Arrays.asList("x", null, "y"), SqlTypeName.CHAR); - assertNotNull(result); - List list = (List) result; - assertEquals(3, list.size()); - assertEquals("x", list.get(0)); - assertNull(list.get(1), "Null should be preserved during CHAR type conversion"); - assertEquals("y", list.get(2)); - } -} diff --git a/core/src/test/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractAllFunctionImplTest.java b/core/src/test/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractAllFunctionImplTest.java index 449e851b81d..5a010a17422 100644 --- a/core/src/test/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractAllFunctionImplTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractAllFunctionImplTest.java @@ -5,13 +5,12 @@ package org.opensearch.sql.expression.function.jsonUDF; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.anEmptyMap; -import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.List; import java.util.Map; import org.junit.jupiter.api.Test; @@ -20,12 +19,39 @@ public class JsonExtractAllFunctionImplTest { private final JsonExtractAllFunctionImpl function = new JsonExtractAllFunctionImpl(); @SuppressWarnings("unchecked") - private Map jsonExtractAll(String json) { - Object result = JsonExtractAllFunctionImpl.eval(json); - if (result == null) { - return null; + private Map assertValidMapResult(Object result) { + assertNotNull(result); + assertTrue(result instanceof Map); + return (Map) result; + } + + @SuppressWarnings("unchecked") + private List assertListValue(Map map, String key) { + Object value = map.get(key); + assertNotNull(value); + assertTrue(value instanceof List); + return (List) value; + } + + private void assertListEquals(List actual, Object... expected) { + assertEquals(expected.length, actual.size()); + for (int i = 0; i < expected.length; i++) { + assertEquals(expected[i], actual.get(i)); } - return (Map) result; + } + + private void assertMapListValue(Map map, String key, Object... expectedValues) { + List list = assertListValue(map, key); + assertListEquals(list, expectedValues); + } + + private void assertMapValue(Map map, String key, Object expectedValue) { + assertEquals(expectedValue, map.get(key)); + } + + private Map eval(String json) { + Object result = JsonExtractAllFunctionImpl.eval(json); + return assertValidMapResult(result); } @Test @@ -40,371 +66,281 @@ public void testOperandMetadata() { @Test public void testFunctionConstructor() { - assertNotNull(new JsonExtractAllFunctionImpl(), "Function should be properly initialized"); + JsonExtractAllFunctionImpl testFunction = new JsonExtractAllFunctionImpl(); + + assertNotNull(testFunction, "Function should be properly initialized"); } @Test public void testNoArguments() { - assertNull(JsonExtractAllFunctionImpl.eval()); + Object result = JsonExtractAllFunctionImpl.eval(); + + assertNull(result); } @Test public void testNullInput() { - assertNull(jsonExtractAll(null)); + Object result = JsonExtractAllFunctionImpl.eval((String) null); + + assertNull(result); } @Test public void testEmptyString() { - assertNull(jsonExtractAll("")); + Object result = JsonExtractAllFunctionImpl.eval(""); + + assertNull(result); } @Test public void testWhitespaceString() { - assertNull(jsonExtractAll(" ")); + Object result = JsonExtractAllFunctionImpl.eval(" "); + + assertNull(result); } @Test public void testEmptyJsonObject() { - assertThat(jsonExtractAll("{}"), anEmptyMap()); + Map map = eval("{}"); + + assertTrue(map.isEmpty()); } @Test - public void testSimpleJsonObject() { - assertThat( - jsonExtractAll( - """ - { - "name": "John", - "age": 30 - }\ - """), - is(Map.of("name", "John", "age", "30"))); + public void testSimpleJsonObject() throws Exception { + Map map = eval("{\"name\": \"John\", \"age\": 30}"); + + assertEquals("John", map.get("name")); + assertEquals(30, map.get("age")); + assertEquals(2, map.size()); } @Test public void testInvalidJsonReturnResults() { - assertThat(jsonExtractAll("{\"name\": \"John\", \"age\":}"), is(Map.of("name", "John"))); + Map map = eval("{\"name\": \"John\", \"age\":}"); + + assertEquals("John", map.get("name")); + assertEquals(1, map.size()); } @Test public void testNonObjectJsonArray() { - assertThat(jsonExtractAll("[1, 2, 3]"), is(Map.of("{}", "[1, 2, 3]"))); + Map map = eval("[1, 2, 3]"); + + assertMapListValue(map, "{}", 1, 2, 3); + assertEquals(1, map.size()); } @Test public void testTopLevelArrayOfObjects() { - assertThat( - jsonExtractAll( - """ - [ - {"age": 1}, - {"age": 2} - ]\ - """), - is(Map.of("{}.age", "[1, 2]"))); + Map map = eval("[{\"age\": 1}, {\"age\": 2}]"); + + assertMapListValue(map, "{}.age", 1, 2); + assertEquals(1, map.size()); } @Test public void testTopLevelArrayOfComplexObjects() { - assertThat( - jsonExtractAll( - """ - [ - {"name": "John", "age": 30}, - {"name": "Jane", "age": 25} - ]\ - """), - is(Map.of("{}.name", "[John, Jane]", "{}.age", "[30, 25]"))); + Map map = + eval("[{\"name\": \"John\", \"age\": 30}, {\"name\": \"Jane\", \"age\": 25}]"); + + assertMapListValue(map, "{}.name", "John", "Jane"); + assertMapListValue(map, "{}.age", 30, 25); + assertEquals(2, map.size()); } @Test public void testNonObjectJsonPrimitive() { - assertNull(jsonExtractAll("\"just a string\"")); + Object result = JsonExtractAllFunctionImpl.eval("\"just a string\""); + + assertNull(result); } @Test public void testNonObjectJsonNumber() { - assertNull(jsonExtractAll("42")); + Object result = JsonExtractAllFunctionImpl.eval("42"); + + assertNull(result); } @Test public void testSingleLevelNesting() { - assertThat( - jsonExtractAll( - """ - { - "user": {"name": "John"}, - "system": "linux" - }\ - """), - is(Map.of("user.name", "John", "system", "linux"))); + Map map = eval("{\"user\": {\"name\": \"John\"}, \"system\": \"linux\"}"); + + assertEquals("John", map.get("user.name")); + assertEquals("linux", map.get("system")); + assertEquals(2, map.size()); } @Test public void testMultiLevelNesting() { - assertThat( - jsonExtractAll( - """ - { - "a": { - "b": { - "c": "value" - } - } - }\ - """), - is(Map.of("a.b.c", "value"))); + Map map = eval("{\"a\": {\"b\": {\"c\": \"value\"}}}"); + + assertEquals("value", map.get("a.b.c")); + assertEquals(1, map.size()); } @Test public void testMixedNestedAndFlat() { - assertThat( - jsonExtractAll( - """ - { - "name": "John", - "address": { - "city": "NYC", - "zip": "10001" - } - }\ - """), - is(Map.of("name", "John", "address.city", "NYC", "address.zip", "10001"))); + Map map = + eval("{\"name\": \"John\", \"address\": {\"city\": \"NYC\", \"zip\": \"10001\"}}"); + + assertEquals("John", map.get("name")); + assertEquals("NYC", map.get("address.city")); + assertEquals("10001", map.get("address.zip")); + assertEquals(3, map.size()); } @Test public void testDeeplyNestedStructure() { - assertThat( - jsonExtractAll( - """ - { - "level1": { - "level2": { - "level3": { - "level4": { - "level5": "deep" - } - } - } - } - }\ - """), - is(Map.of("level1.level2.level3.level4.level5", "deep"))); + Map map = + eval("{\"level1\": {\"level2\": {\"level3\": {\"level4\": {\"level5\": \"deep\"}}}}}"); + + assertEquals("deep", map.get("level1.level2.level3.level4.level5")); + assertEquals(1, map.size()); } @Test public void testSimpleArray() { - assertThat( - jsonExtractAll( - """ - { - "tags": ["a", "b", "c"] - }\ - """), - is(Map.of("tags{}", "[a, b, c]"))); + Map map = eval("{\"tags\": [\"a\", \"b\", \"c\"]}"); + + assertMapListValue(map, "tags{}", "a", "b", "c"); + assertEquals(1, map.size()); } @Test public void testArrayOfObjects() { - assertThat( - jsonExtractAll( - """ - { - "users": [ - {"name": "John"}, - {"name": "Jane"} - ] - }\ - """), - is(Map.of("users{}.name", "[John, Jane]"))); + Map map = eval("{\"users\": [{\"name\": \"John\"}, {\"name\": \"Jane\"}]}"); + + assertMapListValue(map, "users{}.name", "John", "Jane"); + assertEquals(1, map.size()); } @Test public void testNestedArray() { - assertThat( - jsonExtractAll( - """ - { - "data": { - "items": [1, 2, 3] - } - }\ - """), - is(Map.of("data.items{}", "[1, 2, 3]"))); + Map map = eval("{\"data\": {\"items\": [1, 2, 3]}}"); + + assertMapListValue(map, "data.items{}", 1, 2, 3); + assertEquals(1, map.size()); } @Test public void testNested() { - assertThat( - jsonExtractAll( - """ - { - "data": { - "items": [[1, 2, {"hello": 3}], 4], - "other": 5 - }, - "another": [6, [7, 8], 9] - }\ - """), - is( - Map.of( - "data.items{}{}", "[1, 2]", - "data.items{}{}.hello", "3", - "data.items{}", "4", - "data.other", "5", - "another{}", "[6, 9]", - "another{}{}", "[7, 8]"))); + Map map = + eval( + "{\"data\": {\"items\": [[1, 2, {\"hello\": 3}], 4], \"other\": 5}, \"another\": [6," + + " [7, 8], 9]}"); + + assertMapListValue(map, "data.items{}{}", 1, 2); + assertMapValue(map, "data.items{}{}.hello", 3); + assertMapValue(map, "data.items{}", 4); + assertMapValue(map, "data.other", 5); + assertMapListValue(map, "another{}", 6, 9); + assertMapListValue(map, "another{}{}", 7, 8); + assertEquals(6, map.size()); } @Test public void testEmptyArray() { - assertNull(jsonExtractAll("{\"empty\": []}").get("empty{}")); + Map map = eval("{\"empty\": []}"); + + Object emptyValue = map.get("empty{}"); + assertNull(emptyValue); } @Test public void testStringValues() { - assertThat( - jsonExtractAll( - """ - { - "text": "hello world", - "empty": "" - }\ - """), - is(Map.of("text", "hello world", "empty", ""))); + Map map = eval("{\"text\": \"hello world\", \"empty\": \"\"}"); + + assertMapValue(map, "text", "hello world"); + assertMapValue(map, "empty", ""); + assertEquals(2, map.size()); } @Test public void testNumericValues() { - assertThat( - jsonExtractAll( - """ - { - "int": 42, - "long": 9223372036854775807, - "hugeNumber": 9223372036854775808, - "double": 3.14159 - }\ - """), - is( - Map.of( - "int", "42", - "long", "9223372036854775807", - "hugeNumber", "9.223372036854776E18", - "double", "3.14159"))); + Map map = + eval( + "{\"int\": 42, \"long\": 9223372036854775807, \"hugeNumber\": 9223372036854775808," + + " \"double\": 3.14159}"); + + assertEquals(4, map.size()); + assertEquals(42, map.get("int")); + assertEquals(9223372036854775807L, map.get("long")); + assertEquals(9223372036854775808.0, map.get("hugeNumber")); + assertEquals(3.14159, map.get("double")); } @Test public void testBooleanValues() { - assertThat( - jsonExtractAll( - """ - { - "isTrue": true, - "isFalse": false - }\ - """), - is(Map.of("isTrue", "true", "isFalse", "false"))); + Map map = eval("{\"isTrue\": true, \"isFalse\": false}"); + + assertEquals(true, map.get("isTrue")); + assertEquals(false, map.get("isFalse")); + assertEquals(2, map.size()); } @Test public void testNullValues() { - assertThat( - jsonExtractAll( - """ - { - "nullValue": null, - "notNull": "value" - }\ - """), - is(Map.of("nullValue", "null", "notNull", "value"))); - } + Map map = eval("{\"nullValue\": null, \"notNull\": \"value\"}"); - @Test - public void testNullValuesInArray() { - assertThat( - jsonExtractAll( - """ - [ - {"a": null}, - {"a": 1} - ]\ - """), - is(Map.of("{}.a", "[null, 1]"))); + assertNull(map.get("nullValue")); + assertEquals("value", map.get("notNull")); + assertEquals(2, map.size()); } @Test public void testMixedTypesInArray() { - assertThat( - jsonExtractAll( - """ - { - "mixed": ["string", 42, true, null, 3.14] - }\ - """), - is(Map.of("mixed{}", "[string, 42, true, null, 3.14]"))); + Map map = eval("{\"mixed\": [\"string\", 42, true, null, 3.14]}"); + + List mixed = (List) assertListValue(map, "mixed{}"); + assertEquals(5, mixed.size()); + assertEquals("string", mixed.get(0)); + assertEquals(42, mixed.get(1)); + assertEquals(true, mixed.get(2)); + assertNull(mixed.get(3)); + assertEquals(3.14, mixed.get(4)); + assertEquals(1, map.size()); } @Test public void testSpecialCharactersInKeys() { - assertThat( - jsonExtractAll( - """ - { - "key.with.dots": "value1", - "key-with-dashes": "value2", - "key_with_underscores": "value3" - }\ - """), - is( - Map.of( - "key.with.dots", "value1", - "key-with-dashes", "value2", - "key_with_underscores", "value3"))); + Map map = + eval( + "{\"key.with.dots\": \"value1\", \"key-with-dashes\": \"value2\"," + + " \"key_with_underscores\": \"value3\"}"); + + assertEquals("value1", map.get("key.with.dots")); + assertEquals("value2", map.get("key-with-dashes")); + assertEquals("value3", map.get("key_with_underscores")); + assertEquals(3, map.size()); } @Test public void testUnicodeCharacters() { - assertThat( - jsonExtractAll( - """ - { - "unicode": "こんにちは", - "emoji": "🚀", - "🚀": 1 - }\ - """), - is(Map.of("unicode", "こんにちは", "emoji", "🚀", "🚀", "1"))); + Map map = eval("{\"unicode\": \"こんにちは\", \"emoji\": \"🚀\", \"🚀\": 1}"); + + assertEquals("こんにちは", map.get("unicode")); + assertEquals("🚀", map.get("emoji")); + assertEquals(1, map.get("🚀")); + assertEquals(3, map.size()); } @Test public void testComplexNestedStructure() { - assertThat( - jsonExtractAll( - """ - { - "user": { - "profile": { - "name": "John", - "contacts": [ - {"type": "email", "value": "john@example.com"}, - {"type": "phone", "value": "123-456-7890"} - ] - }, - "preferences": { - "theme": "dark", - "notifications": true - } - } - }\ - """), - is( - Map.of( - "user.profile.name", "John", - "user.profile.contacts{}.type", "[email, phone]", - "user.profile.contacts{}.value", "[john@example.com, 123-456-7890]", - "user.preferences.theme", "dark", - "user.preferences.notifications", "true"))); + Map map = + eval( + "{\"user\": {\"profile\": {\"name\": \"John\", \"contacts\": [{\"type\": \"email\"," + + " \"value\": \"john@example.com\"}, {\"type\": \"phone\", \"value\":" + + " \"123-456-7890\"}]}, \"preferences\": {\"theme\": \"dark\", \"notifications\":" + + " true}}}"); + + assertEquals("John", map.get("user.profile.name")); + assertMapListValue(map, "user.profile.contacts{}.type", "email", "phone"); + assertMapListValue(map, "user.profile.contacts{}.value", "john@example.com", "123-456-7890"); + assertEquals("dark", map.get("user.preferences.theme")); + assertEquals(true, map.get("user.preferences.notifications")); + assertEquals(5, map.size()); } @Test @@ -416,9 +352,9 @@ public void testLargeJsonObject() { } jsonBuilder.append("}"); - Map map = jsonExtractAll(jsonBuilder.toString()); + Map map = eval(jsonBuilder.toString()); assertEquals(100, map.size()); - assertEquals("0", map.get("field0")); - assertEquals("99", map.get("field99")); + assertEquals(0, map.get("field0")); + assertEquals(99, map.get("field99")); } } diff --git a/datasources/build.gradle b/datasources/build.gradle index 8c708176558..1dd01d82fb9 100644 --- a/datasources/build.gradle +++ b/datasources/build.gradle @@ -26,10 +26,7 @@ dependencies { implementation ('com.amazonaws:aws-encryption-sdk-java:2.4.1') { exclude group: 'org.bouncycastle', module: 'bcprov-ext-jdk18on' } - - // bc-fips is provided by OpenSearch core at runtime since opensearch 3.6.0 - compileOnly "org.bouncycastle:bc-fips:${versions.bouncycastle_jce}" - testImplementation "org.bouncycastle:bc-fips:${versions.bouncycastle_jce}" + implementation "org.bouncycastle:bc-fips:${versions.bouncycastle_jce}" testImplementation group: 'junit', name: 'junit', version: '4.13.2' testImplementation('org.junit.jupiter:junit-jupiter:5.9.3') diff --git a/docs/category.json b/docs/category.json index 5e9b6f954a5..bcf73cb1a82 100644 --- a/docs/category.json +++ b/docs/category.json @@ -25,8 +25,6 @@ "user/ppl/cmd/join.md", "user/ppl/cmd/lookup.md", "user/ppl/cmd/mvcombine.md", - "user/ppl/cmd/nomv.md", - "user/ppl/cmd/mvexpand.md", "user/ppl/cmd/parse.md", "user/ppl/cmd/patterns.md", "user/ppl/cmd/rare.md", diff --git a/docs/user/ppl/cmd/mvexpand.md b/docs/user/ppl/cmd/mvexpand.md deleted file mode 100644 index 6fdd9bca365..00000000000 --- a/docs/user/ppl/cmd/mvexpand.md +++ /dev/null @@ -1,137 +0,0 @@ -# mvexpand - -## Description -The `mvexpand` command expands each value in a multivalue (array) field into a separate row. For each document, every element in the specified array field is returned as a new row. - - -## Syntax -``` -mvexpand [limit=] -``` - -- ``: The multivalue (array) field to expand. (Required) -- `limit`: Maximum number of values per document to expand. If not specified, all array elements are expanded. (Optional) - - -### Output field naming -After `mvexpand`, the expanded value remains under the same field name (for example, `tags` or `ids`). -If the array contains objects, you can reference subfields (for example, `skills.name`). - - -## Examples - -### Example 1: Basic Expansion (single document) -Input document (case "basic") contains three tag values. - -PPL query: -```ppl -source=people -| eval tags = array('error', 'warning', 'info') -| fields tags -| head 1 -| mvexpand tags -| fields tags -``` - -Expected output: -```text -fetched rows / total rows = 3/3 -+---------+ -| tags | -|---------| -| error | -| warning | -| info | -+---------+ -``` - -### Example 2: Expansion with Limit -Input document (case "ids") contains an array of integers; expand and apply limit. - -PPL query: -```ppl -source=people -| eval ids = array(1, 2, 3, 4, 5) -| fields ids -| head 1 -| mvexpand ids limit=3 -| fields ids -``` - -Expected output: -```text -fetched rows / total rows = 3/3 -+-----+ -| ids | -|-----| -| 1 | -| 2 | -| 3 | -+-----+ -``` - -### Example 3: Expand projects -This example demonstrates expanding a multivalue `projects` field into one row per project. - -PPL query: -```ppl -source=people -| head 1 -| fields projects -| mvexpand projects -| fields projects.name -``` - -Expected output: -```text -fetched rows / total rows = 3/3 -+--------------------------------+ -| projects.name | -|--------------------------------| -| AWS Redshift Spectrum querying | -| AWS Redshift security | -| AWS Aurora security | -+--------------------------------+ -``` - -### Example 4: Single-value array (case "single") -Single-element array should expand to one row. - -PPL query: -```ppl -source=people -| eval tags = array('error') -| fields tags -| head 1 -| mvexpand tags -| fields tags -``` - -Expected output: -```text -fetched rows / total rows = 1/1 -+-------+ -| tags | -|-------| -| error | -+-------+ -``` - -### Example 5: Missing Field -If the field does not exist in the input schema (for example, it is not mapped or was projected out earlier), mvexpand throws a semantic check exception. - -PPL query: -```ppl -source=people -| eval some_field = 'x' -| fields some_field -| head 1 -| mvexpand tags -| fields tags -``` - -Expected output: -```text -{'reason': 'Invalid Query', 'details': "Field 'tags' not found in the schema", 'type': 'SemanticCheckException'} -Error: Query returned no data -``` \ No newline at end of file diff --git a/docs/user/ppl/cmd/nomv.md b/docs/user/ppl/cmd/nomv.md deleted file mode 100644 index 87c17d12472..00000000000 --- a/docs/user/ppl/cmd/nomv.md +++ /dev/null @@ -1,81 +0,0 @@ -# nomv - -## Description - -The `nomv` command converts a multivalue (array) field into a single-value string field by joining all array elements with newline characters (`\n`). This operation is performed in-place, replacing the original field with its joined string representation. - -`nomv` is a transforming command: it modifies the specified field without changing the number of rows in the result set. - -### Key behaviors - -- The field must be **ARRAY type**. For scalar fields, use the `array()` function to create an array first. - ---- - -## Syntax - -```syntax -nomv -``` - -### Arguments - -- **field** (required) - The name of the field whose multivalue content should be converted to a single-value string. - ---- - -## Example 1: Basic nomv usage - -```ppl -source=accounts -| where account_number=1 -| eval names = array(firstname, lastname) -| nomv names -| fields account_number, names -``` - -Expected output: -```text -fetched rows / total rows = 1/1 -+----------------+-------+ -| account_number | names | -|----------------+-------| -| 1 | Amber | -| | Duke | -+----------------+-------+ -``` - -## Example 2: nomv with an eval-created field - -```ppl -source=accounts -| where account_number=1 -| eval location = array(city, state) -| nomv location -| fields account_number, location -``` - -Expected output: -```text -fetched rows / total rows = 1/1 -+----------------+----------+ -| account_number | location | -|----------------+----------| -| 1 | Brogan | -| | IL | -+----------------+----------+ -``` - ---- - -## Notes - -- The `nomv` command is only available when the Calcite query engine is enabled. -- This command is particularly useful when you need to export or display multivalue fields as single strings. -- The newline delimiter (`\n`) is fixed and cannot be customized. For custom delimiters, use the `mvjoin` function directly in an eval expression. -- NULL values within the array are automatically filtered out when converting the array to a string, so they do not appear in the output or contribute empty lines. - -## Related commands - -- `mvjoin()` -- Function used by nomv internally to join array elements with a custom delimiter diff --git a/docs/user/ppl/cmd/spath.md b/docs/user/ppl/cmd/spath.md index 62cb2399915..d9293113fb0 100644 --- a/docs/user/ppl/cmd/spath.md +++ b/docs/user/ppl/cmd/spath.md @@ -1,10 +1,7 @@ # spath -The `spath` command extracts fields from structured JSON data. It operates in two modes: - -- **Path-based mode**: When `path` is specified, extracts a single value at the given JSON path. -- **Auto-extract mode** (experimental): When `path` is omitted, extracts all fields from the JSON into a map. +The `spath` command extracts fields from structured text data by allowing you to select JSON values using JSON paths. > **Note**: The `spath` command is not executed on OpenSearch data nodes. It extracts fields from data after it has been returned to the coordinator node, which is slow on large datasets. We recommend indexing fields needed for filtering directly instead of using `spath` to filter nested fields. @@ -13,7 +10,7 @@ The `spath` command extracts fields from structured JSON data. It operates in tw The `spath` command has the following syntax: ```syntax -spath input= [output=] [[path=]] +spath input= [output=] [path=] ``` ## Parameters @@ -23,25 +20,11 @@ The `spath` command supports the following parameters. | Parameter | Required/Optional | Description | | --- | --- | --- | | `input` | Required | The field containing JSON data to parse. | -| `output` | Optional | The destination field in which the extracted data is stored. Default is the value of `path` in path-based mode, or the value of `input` in auto-extract mode. | -| `path` | Optional | The JSON path that identifies the data to extract. When omitted, all fields are extracted into a map (auto-extract mode). | +| `output` | Optional | The destination field in which the extracted data is stored. Default is the value of ``. | +| `` | Required | The JSON path that identifies the data to extract. | For more information about path syntax, see [json_extract](../functions/json.md#json_extract). -## Auto-extract mode (experimental) - -When `path` is omitted, the `spath` command runs in auto-extract mode. Instead of extracting a single value, it flattens the entire JSON into a `map` column using the following rules: - -- Nested objects use dotted keys: `user.name`, `user.age` -- Arrays use `{}` suffix: `tags{}`, `users{}.name` -- Duplicate logical keys merge into arrays: `c{}.b = [2, 3]` -- Null values are preserved: a JSON `null` becomes the string `"null"` in the map -- All values are stringified: numbers and booleans are converted to their string representation (for example, `30` becomes `"30"`, `true` becomes `"true"`, and arrays become `"[a, b, c]"`) - -> **Note**: Auto-extract mode processes the entire input field with no character limit. For large JSON payloads, consider using path-based extraction to target specific fields. -> -> Invalid or malformed JSON returns partial results containing any fields successfully parsed before the error. Empty JSON object (`{}`) returns an empty map. - ## Example 1: Basic field extraction The basic use of `spath` extracts a single field from JSON data. The following query extracts the `n` field from JSON objects in the `doc_n` field: @@ -140,34 +123,3 @@ fetched rows / total rows = 3/3 +-------+---+ ``` - -## Example 5: Auto-extract mode - -When `path` is omitted, `spath` extracts all fields from the JSON into a map. You can access individual values using dotted path navigation, where `doc.user.name` resolves to the map key `user.name`. For keys containing special characters like `{}`, use backtick quoting: - -```ppl -source=structured -| spath input=doc_auto output=doc -| fields doc_auto, doc.user.name, doc.user.age, doc.`tags{}`, doc.active -``` - -The query returns the following results: - -```text -fetched rows / total rows = 3/3 -+---------------------------------------------------------------------------------+---------------+--------------+-----------------+------------+ -| doc_auto | doc.user.name | doc.user.age | doc.tags{} | doc.active | -|---------------------------------------------------------------------------------+---------------+--------------+-----------------+------------| -| {"user":{"name":"John","age":30},"tags":["java","sql"],"active":true} | John | 30 | [java, sql] | true | -| {"user":{"name":"Jane","age":25},"tags":["python"],"active":null} | Jane | 25 | python | null | -| {"user":{"name":"Bob","age":35},"tags":["go","rust","sql"],"user.name":"Bobby"} | [Bob, Bobby] | 35 | [go, rust, sql] | null | -+---------------------------------------------------------------------------------+---------------+--------------+-----------------+------------+ -``` - -The flattening rules demonstrated in this example: - -- Nested objects use dotted keys: `user.name` and `user.age` are extracted from `{"user": {"name": "John", "age": 30}}` -- Arrays use `{}` suffix: `tags{}` is extracted from `{"tags": ["java", "sql"]}` -- Duplicate logical keys merge into arrays: in the third row, both `"user": {"name": "Bob"}` (nested) and `"user.name": "Bobby"` (direct dotted key) resolve to the same key `user.name`, so their values merge into `'[Bob, Bobby]'` -- All values are strings: numeric `30` becomes `'30'`, boolean `true` becomes `'true'`, and arrays become strings like `'[java, sql]'` -- Null values are preserved: in the second row, `"active": null` is kept as `'active': 'null'` in the map diff --git a/docs/user/ppl/index.md b/docs/user/ppl/index.md index fc4bf7febdd..718aa51f0fe 100644 --- a/docs/user/ppl/index.md +++ b/docs/user/ppl/index.md @@ -78,13 +78,12 @@ source=accounts | [describe command](cmd/describe.md) | 2.1 | stable (since 2.1) | Query the metadata of an index. | | [explain command](cmd/explain.md) | 3.1 | stable (since 3.1) | Explain the plan of query. | | [show datasources command](cmd/showdatasources.md) | 2.4 | stable (since 2.4) | Query datasources configured in the PPL engine. | -| [addtotals command](cmd/addtotals.md) | 3.5 | stable (since 3.5) | Adds row and column values and appends a totals column and row. | +| [addtotals command](cmd/addtotals.md) | 3.5 | stable (since 3.5) | Adds row and column values and appends a totals column and row. | | [addcoltotals command](cmd/addcoltotals.md) | 3.5 | stable (since 3.5) | Adds column values and appends a totals row. | | [transpose command](cmd/transpose.md) | 3.5 | stable (since 3.5) | Transpose rows to columns. | | [mvcombine command](cmd/mvcombine.md) | 3.5 | stable (since 3.4) | Combines values of a specified field across rows identical on all other fields. | -| [nomv command](cmd/nomv.md) | 3.6 | stable (since 3.6) | Converts a multivalue field to a single-value string by joining elements with newlines. | -| [mvexpand command](cmd/mvexpand.md) | 3.6 | stable (since 3.6) | Expand a multi-valued field into separate documents (one per value). | -| [graphlookup command](cmd/graphlookup.md) | 3.6 | experimental (since 3.6) | Performs recursive graph traversal on a collection using a BFS algorithm.| +| [graphlookup command](cmd/graphlookup.md) | 3.5 | experimental (since 3.5) | Performs recursive graph traversal on a collection using a BFS algorithm.| + - [Syntax](cmd/syntax.md) - PPL query structure and command syntax formatting * **Functions** diff --git a/doctest/test_data/structured.json b/doctest/test_data/structured.json index d96995a44f9..c0717c6f328 100644 --- a/doctest/test_data/structured.json +++ b/doctest/test_data/structured.json @@ -1,3 +1,3 @@ -{"doc_n":"{\"n\": 1}","doc_escape":"{\"a fancy field name\": true,\"a.b.c\": 0}","doc_list":"{\"list\": [1, 2, 3, 4], \"nest_out\": {\"nest_in\": \"a\"}}","doc_auto":"{\"user\":{\"name\":\"John\",\"age\":30},\"tags\":[\"java\",\"sql\"],\"active\":true}","obj_field":{"field": "a"}} -{"doc_n":"{\"n\": 2}","doc_escape":"{\"a fancy field name\": true,\"a.b.c\": 1}","doc_list":"{\"list\": [], \"nest_out\": {\"nest_in\": \"a\"}}","doc_auto":"{\"user\":{\"name\":\"Jane\",\"age\":25},\"tags\":[\"python\"],\"active\":null}","obj_field":{"field": "b"}} -{"doc_n":"{\"n\": 3}","doc_escape":"{\"a fancy field name\": false,\"a.b.c\": 2}","doc_list":"{\"list\": [5, 6], \"nest_out\": {\"nest_in\": \"a\"}}","doc_auto":"{\"user\":{\"name\":\"Bob\",\"age\":35},\"tags\":[\"go\",\"rust\",\"sql\"],\"user.name\":\"Bobby\"}","obj_field":{"field": "c"}} +{"doc_n":"{\"n\": 1}","doc_escape":"{\"a fancy field name\": true,\"a.b.c\": 0}","doc_list":"{\"list\": [1, 2, 3, 4], \"nest_out\": {\"nest_in\": \"a\"}}","obj_field":{"field": "a"}} +{"doc_n":"{\"n\": 2}","doc_escape":"{\"a fancy field name\": true,\"a.b.c\": 1}","doc_list":"{\"list\": [], \"nest_out\": {\"nest_in\": \"a\"}}","obj_field":{"field": "b"}} +{"doc_n":"{\"n\": 3}","doc_escape":"{\"a fancy field name\": false,\"a.b.c\": 2}","doc_list":"{\"list\": [5, 6], \"nest_out\": {\"nest_in\": \"a\"}}","obj_field":{"field": "c"}} \ No newline at end of file diff --git a/doctest/test_mapping/structured.json b/doctest/test_mapping/structured.json index dd255cc0c54..5c79e53dc0a 100644 --- a/doctest/test_mapping/structured.json +++ b/doctest/test_mapping/structured.json @@ -10,9 +10,6 @@ "doc_escape": { "type": "text" }, - "doc_auto": { - "type": "text" - }, "obj_field": { "properties": { "field": { "type": "text" } @@ -20,4 +17,4 @@ } } } -} +} \ No newline at end of file diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java index 47acf06e6a3..aa569629e23 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java @@ -108,9 +108,7 @@ CalciteVisualizationFormatIT.class, CalciteWhereCommandIT.class, CalcitePPLTpchIT.class, - CalciteMvCombineCommandIT.class, - CalciteNoMvCommandIT.class, - CalciteMvExpandCommandIT.class, + CalciteMvCombineCommandIT.class }) public class CalciteNoPushdownIT { private static boolean wasPushdownEnabled; diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java index 8e980d8973b..0bbc25f1d7d 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java @@ -59,7 +59,6 @@ public void init() throws Exception { loadIndex(Index.DATA_TYPE_ALIAS); loadIndex(Index.DEEP_NESTED); loadIndex(Index.CASCADED_NESTED); - loadIndex(Index.MVEXPAND_EDGE_CASES); } @Override @@ -2034,17 +2033,6 @@ public void testTopKThenSortExplain() throws IOException { + "| fields age")); } - @Test - public void testIssue5114SortExprHeadExplain() throws IOException { - enabledOnlyWhenPushdownIsEnabled(); - String query = - "source=opensearch-sql_test_index_account | eval a = rand() | sort a | fields" - + " account_number | head 5"; - var result = explainQueryYaml(query); - String expected = loadExpectedPlan("explain_issue_5114_sort_expr_head_push.yaml"); - assertYamlEqualsIgnoreId(expected, result); - } - @Test public void testGeoIpPushedInAgg() throws IOException { // This explain IT verifies that externally registered UDF can be properly pushed down @@ -2541,28 +2529,6 @@ public void testExplainMvCombine() throws IOException { assertYamlEqualsIgnoreId(expected, actual); } - @Test - public void testExplainNoMv() throws IOException { - String query = - "source=opensearch-sql_test_index_account " - + "| fields state, city, age " - + "| eval location = array(state, city) " - + "| nomv location"; - - String actual = explainQueryYaml(query); - String expected = loadExpectedPlan("explain_nomv.yaml"); - assertYamlEqualsIgnoreId(expected, actual); - } - - @Test - public void testMvexpandExplain() throws IOException { - String expected = loadExpectedPlan("explain_mvexpand.yaml"); - String actual = - explainQueryYaml( - "source=mvexpand_edge_cases | eval skills_arr = array(1, 2, 3) | mvexpand skills_arr"); - assertYamlEqualsIgnoreId(expected, actual); - } - // ==================== fetch_size explain tests ==================== @Test @@ -2740,30 +2706,4 @@ public void testFilterBooleanFieldOnlyNotTrue() throws IOException { String expected = loadExpectedPlan("explain_filter_boolean_only_not_true.yaml"); assertYamlEqualsIgnoreId(expected, result); } - - @Test - public void testNoMvBasic() throws IOException { - String query = - StringUtils.format( - "source=%s | fields firstname, age | eval names = array(firstname) | nomv names |" - + " fields names", - TEST_INDEX_BANK); - var result = explainQueryYaml(query); - Assert.assertTrue( - "Expected explain to contain ARRAY_JOIN function", - result.toLowerCase().contains("array_join")); - } - - @Test - public void testNoMvWithEval() throws IOException { - String query = - StringUtils.format( - "source=%s | eval full_name = concat(firstname, ' J.') | eval name_array =" - + " array(full_name) | nomv name_array | fields name_array", - TEST_INDEX_BANK); - var result = explainQueryYaml(query); - Assert.assertTrue( - "Expected explain to contain both CONCAT and ARRAY_JOIN", - result.toLowerCase().contains("concat") && result.toLowerCase().contains("array_join")); - } } diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteMvExpandCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteMvExpandCommandIT.java deleted file mode 100644 index 99334dcb6c2..00000000000 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteMvExpandCommandIT.java +++ /dev/null @@ -1,282 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.calcite.remote; - -import static org.opensearch.sql.util.MatcherUtils.rows; -import static org.opensearch.sql.util.MatcherUtils.schema; -import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; -import static org.opensearch.sql.util.MatcherUtils.verifyNumOfRows; -import static org.opensearch.sql.util.MatcherUtils.verifySchema; - -import org.json.JSONObject; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; -import org.opensearch.sql.ppl.PPLIntegTestCase; - -public class CalciteMvExpandCommandIT extends PPLIntegTestCase { - - private static final String INDEX = Index.MVEXPAND_EDGE_CASES.getName(); - - @Override - public void init() throws Exception { - super.init(); - enableCalcite(); - loadIndex(Index.MVEXPAND_EDGE_CASES); - } - - @Test - public void testMvexpandSingleElement() throws Exception { - String q1 = - String.format( - "source=%s | mvexpand skills | where username='single' | fields username, skills", - INDEX); - JSONObject r1 = executeQuery(q1); - - assertSingleRowNestedFieldEquals(r1, "skills", "name", "go"); - - String q2 = - String.format( - "source=%s | mvexpand skills | where username='single' | fields username, skills.name", - INDEX); - JSONObject r2 = executeQuery(q2); - verifyDataRows(r2, rows("single", "go")); - } - - /** - * Asserts the result has exactly one row and that the given column is a MAP/object containing - * nestedKey=nestedValue. - */ - private static void assertSingleRowNestedFieldEquals( - JSONObject result, String mapColumn, String nestedKey, String expectedValue) { - var dataRows = result.getJSONArray("datarows"); - Assertions.assertEquals(1, dataRows.length(), "Expected exactly one row"); - - var schema = result.getJSONArray("schema"); - - int mapIdx = -1; - for (int i = 0; i < schema.length(); i++) { - if (mapColumn.equals(schema.getJSONObject(i).getString("name"))) { - mapIdx = i; - break; - } - } - Assertions.assertTrue(mapIdx >= 0, "Column not found in schema: " + mapColumn); - - var row0 = dataRows.getJSONArray(0); - var skillsObj = row0.getJSONObject(mapIdx); // this is the MAP/object - Assertions.assertEquals(expectedValue, skillsObj.optString(nestedKey, null)); - } - - @Test - public void testMvexpandEmptyArray() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills | where username='empty' | fields username, skills.name", - INDEX); - JSONObject result = executeQuery(query); - verifyDataRows(result); - } - - @Test - public void testMvexpandNullArray() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills | where username='nullskills' | fields username," - + " skills.name", - INDEX); - JSONObject result = executeQuery(query); - verifyDataRows(result); - } - - @Test - public void testMvexpandNoArrayField() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills | where username='noskills' | fields username," - + " skills.name", - INDEX); - JSONObject result = executeQuery(query); - verifyDataRows(result); - } - - @Test - public void testMvexpandDuplicate() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills | where username='duplicate' | fields username," - + " skills.name | sort skills.name", - INDEX); - JSONObject result = executeQuery(query); - verifyDataRows(result, rows("duplicate", "dup"), rows("duplicate", "dup")); - } - - @Test - public void testMvexpandHappyMultipleElements() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills | where username='happy' | fields username, skills.name |" - + " sort skills.name", - INDEX); - JSONObject result = executeQuery(query); - verifyDataRows(result, rows("happy", "java"), rows("happy", "python"), rows("happy", "sql")); - } - - @Test - public void testMvexpandPartialElementMissingName() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills | where username='partial' | fields username, skills.name" - + " | sort skills.name", - INDEX); - JSONObject result = executeQuery(query); - verifyDataRows( - result, - rows("partial", "kotlin"), - rows("partial", (String) null), - rows("partial", (String) null)); - } - - @Test - public void testMvexpandMixedShapesKeepsAllElements() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills | where username='mixed_shapes' | fields username," - + " skills.name | sort skills.name", - INDEX); - JSONObject result = executeQuery(query); - verifyDataRows(result, rows("mixed_shapes", "elixir"), rows("mixed_shapes", "haskell")); - } - - @Test - public void testMvexpandFlattenedSchemaPresence() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills | where username='complex' | fields username," - + " skills.level, skills.name", - INDEX); - JSONObject result = executeQuery(query); - - verifySchema( - result, - schema("username", "string"), - schema("skills.level", "string"), - schema("skills.name", "string")); - - verifyDataRows( - result, - rows("complex", "expert", "ml"), - rows("complex", (String) null, "ai"), - rows("complex", "novice", (String) null)); - } - - @Test - public void testMvexpandOnNonArrayFieldMapping() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills_not_array | where username='u1' | fields username," - + " skills_not_array", - INDEX); - - JSONObject result = executeQuery(query); - - verifyNumOfRows(result, 1); - verifyDataRows(result, rows("u1", "scala")); - } - - @Test - public void testMvexpandMissingFieldReturnsEmpty() throws Exception { - // single-index version: username='noskills' doc has no "skills" field at all - String query = - String.format( - "source=%s | mvexpand skills | where username='noskills' | fields username, skills", - INDEX); - - JSONObject result = executeQuery(query); - verifyDataRows(result); - } - - @Test - public void testMvexpandLimitParameter() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills limit=3 | where username='limituser' | fields username," - + " skills.name", - INDEX); - JSONObject result = executeQuery(query); - verifyNumOfRows(result, 3); - verifyDataRows(result, rows("limituser", "a"), rows("limituser", "b"), rows("limituser", "c")); - } - - @Test - public void testMvexpandMultiDocumentLimitParameter() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills limit=2 | where username='happy' OR username='limituser'" - + " | fields username, skills.name | sort username, skills.name", - INDEX); - JSONObject result = executeQuery(query); - - verifyNumOfRows(result, 4); - - verifyDataRows( - result, - rows("happy", "java"), - rows("happy", "python"), - rows("limituser", "a"), - rows("limituser", "b")); - } - - @Test - public void testMvexpandTypeInferenceForHeterogeneousSubfields() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills | where username='hetero_types' | fields username," - + " skills.level", - INDEX); - JSONObject result = executeQuery(query); - - verifyDataRows(result, rows("hetero_types", "senior"), rows("hetero_types", "3")); - } - - @Test - public void testMvexpandLargeArrayElements() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills | where username='large' | fields username, skills.name |" - + " sort skills.name", - INDEX); - JSONObject result = executeQuery(query); - - verifyNumOfRows(result, 10); - - verifyDataRows( - result, - rows("large", "s1"), - rows("large", "s10"), - rows("large", "s2"), - rows("large", "s3"), - rows("large", "s4"), - rows("large", "s5"), - rows("large", "s6"), - rows("large", "s7"), - rows("large", "s8"), - rows("large", "s9")); - } - - @Test - public void testMvexpandOnIntegerFieldMapping() throws Exception { - String query = - String.format( - "source=%s | mvexpand skills_int | where username='u_int' | fields username," - + " skills_int", - INDEX); - - JSONObject result = executeQuery(query); - - verifyNumOfRows(result, 1); - verifyDataRows(result, rows("u_int", 5)); - } -} diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteNoMvCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteNoMvCommandIT.java deleted file mode 100644 index 3ad50cdb4b0..00000000000 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteNoMvCommandIT.java +++ /dev/null @@ -1,359 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.calcite.remote; - -import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK; -import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK_WITH_NULL_VALUES; -import static org.opensearch.sql.util.MatcherUtils.rows; -import static org.opensearch.sql.util.MatcherUtils.schema; -import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; -import static org.opensearch.sql.util.MatcherUtils.verifySchema; - -import java.io.IOException; -import org.json.JSONObject; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; -import org.opensearch.client.ResponseException; -import org.opensearch.sql.ppl.PPLIntegTestCase; - -public class CalciteNoMvCommandIT extends PPLIntegTestCase { - - @Override - public void init() throws Exception { - super.init(); - enableCalcite(); - loadIndex(Index.BANK); - loadIndex(Index.BANK_WITH_NULL_VALUES); - } - - // --------------------------- - // Sanity (precondition) - // --------------------------- - - @Test - public void testSanityDatasetIsLoaded() throws IOException { - JSONObject result = executeQuery("source=" + TEST_INDEX_BANK + " | head 5"); - int rows = result.getJSONArray("datarows").length(); - Assertions.assertTrue(rows > 0, "Expected bank dataset to have rows, got 0"); - } - - // --------------------------- - // Happy path (core nomv) - // --------------------------- - - @Test - public void testNoMvBasicUsageFromRFC() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK - + " | where account_number=1 | eval names = array(firstname, lastname) | nomv names |" - + " fields account_number, names"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("account_number", null, "bigint"), schema("names", null, "string")); - - verifyDataRows(result, rows(1, "Amber JOHnny\nDuke Willmington")); - } - - @Test - public void testNoMvEvalCreatedFieldFromRFC() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK - + " | where account_number=1 | eval location = array(city, state) | nomv location |" - + " fields account_number, location"; - - JSONObject result = executeQuery(q); - - verifySchema( - result, schema("account_number", null, "bigint"), schema("location", null, "string")); - - verifyDataRows(result, rows(1, "Brogan\nIL")); - } - - // --------------------------- - // Additional nomv tests - // --------------------------- - - @Test - public void testNoMvMultipleArraysAppliedInSequence() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK - + " | eval arr1 = array('a', 'b'), arr2 = array('x', 'y') | nomv arr1 | nomv arr2 |" - + " head 1 | fields arr1, arr2"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("arr1", null, "string"), schema("arr2", null, "string")); - - verifyDataRows(result, rows("a\nb", "x\ny")); - } - - @Test - public void testNoMvInComplexPipelineWithWhereAndSort() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK - + " | where account_number < 20 | eval arr = array(firstname, 'test') | nomv arr |" - + " sort account_number | head 3 | fields account_number, arr"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("account_number", null, "bigint"), schema("arr", null, "string")); - - verifyDataRows( - result, rows(1, "Amber JOHnny\ntest"), rows(6, "Hattie\ntest"), rows(13, "Nanette\ntest")); - } - - @Test - public void testNoMvFieldUsableInSubsequentOperations() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK - + " | where account_number = 6 | eval arr = array('test', 'data') | nomv arr | eval" - + " arr_len = length(arr) | fields account_number, arr, arr_len"; - - JSONObject result = executeQuery(q); - - verifySchema( - result, - schema("account_number", null, "bigint"), - schema("arr", null, "string"), - schema("arr_len", null, "int")); - - verifyDataRows(result, rows(6, "test\ndata", 9)); - } - - @Test - public void testNoMvWithStatsAfterAggregation() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK - + " | stats count() as cnt by age | eval age_str = cast(age as string) | eval arr =" - + " array(age_str, 'count') | nomv arr | fields cnt, age, arr | sort cnt | head 2"; - - JSONObject result = executeQuery(q); - - verifySchema( - result, - schema("cnt", null, "bigint"), - schema("age", null, "int"), - schema("arr", null, "string")); - - Assertions.assertTrue(result.getJSONArray("datarows").length() > 0); - } - - @Test - public void testNoMvWithEvalWorksOnComputedArrays() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK - + " | where account_number = 1 | eval full_name = concat(firstname, ' ', lastname) |" - + " eval arr = array(full_name, 'suffix') | nomv arr | fields full_name, arr"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("full_name", null, "string"), schema("arr", null, "string")); - - verifyDataRows( - result, rows("Amber JOHnny Duke Willmington", "Amber JOHnny Duke Willmington\nsuffix")); - } - - @Test - public void testNoMvEmptyArray() throws IOException { - String q = - "source=" + TEST_INDEX_BANK + " | eval arr = array() | nomv arr | head 1 | fields arr"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("arr", null, "string")); - - verifyDataRows(result, rows("")); - } - - @Test - public void testNoMvScalarFieldError() throws IOException { - ResponseException ex = - Assertions.assertThrows( - ResponseException.class, - () -> - executeQuery("source=" + TEST_INDEX_BANK + " | fields firstname | nomv firstname")); - - int status = ex.getResponse().getStatusLine().getStatusCode(); - Assertions.assertEquals(400, status, "Expected 400 for type mismatch"); - - String msg = ex.getMessage(); - - Assertions.assertTrue( - msg.contains("MVJOIN") || msg.contains("ARRAY") || msg.contains("type"), msg); - } - - @Test - public void testNoMvResultUsedInComparison() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK - + " | eval arr = array('test') | nomv arr | where arr = 'test' | head 1 | fields" - + " account_number, arr"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("account_number", null, "bigint"), schema("arr", null, "string")); - - Assertions.assertTrue(result.getJSONArray("datarows").length() > 0); - } - - @Test - public void testNoMvMissingFieldShouldReturn4xx() throws IOException { - ResponseException ex = - Assertions.assertThrows( - ResponseException.class, - () -> executeQuery("source=" + TEST_INDEX_BANK + " | nomv does_not_exist")); - - int status = ex.getResponse().getStatusLine().getStatusCode(); - - Assertions.assertEquals(400, status, "Unexpected status. ex=" + ex.getMessage()); - - String msg = ex.getMessage(); - Assertions.assertTrue( - msg.contains("does_not_exist") - || msg.contains("field") - || msg.contains("Field") - || msg.contains("ARRAY_COMPACT") - || msg.contains("ARRAY"), - msg); - } - - @Test - public void testNoMvWithNullInMiddleOfArray() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK_WITH_NULL_VALUES - + " | where account_number = 25 | eval arr = array(firstname, age, lastname) | nomv" - + " arr | fields account_number, arr"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("account_number", null, "bigint"), schema("arr", null, "string")); - - verifyDataRows(result, rows(25, "Virginia\nAyala")); - } - - @Test - public void testNoMvWithNullAtBeginningAndEnd() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK_WITH_NULL_VALUES - + " | where account_number = 25 | eval arr = array(age, firstname, age) | nomv arr |" - + " fields account_number, arr"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("account_number", null, "bigint"), schema("arr", null, "string")); - - verifyDataRows(result, rows(25, "Virginia")); - } - - @Test - public void testNoMvWithAllNulls() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK_WITH_NULL_VALUES - + " | where account_number = 25 | eval arr = array(age, age, age) | nomv arr | fields" - + " account_number, arr"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("account_number", null, "bigint"), schema("arr", null, "string")); - - verifyDataRows(result, rows(25, "")); - } - - @Test - public void testNoMvArrayWithAllNulls() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK_WITH_NULL_VALUES - + " | where account_number = 25 | eval arr = array(age, age, age) | nomv arr | fields" - + " account_number, arr"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("account_number", null, "bigint"), schema("arr", null, "string")); - - verifyDataRows(result, rows(25, "")); - } - - @Test - public void testNoMvMultipleRowsRowLocalBehavior() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK - + " | eval tags = array(firstname, lastname) | nomv tags | sort account_number | head" - + " 3 | fields account_number, tags"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("account_number", null, "bigint"), schema("tags", null, "string")); - - verifyDataRows( - result, - rows(1, "Amber JOHnny\nDuke Willmington"), - rows(6, "Hattie\nBond"), - rows(13, "Nanette\nBates")); - } - - @Test - public void testNoMvNonConsecutiveRowsNoGrouping() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK - + " | where account_number = 1 or account_number = 6 or account_number = 13 | eval" - + " tags = array(firstname, city) | nomv tags | sort account_number | fields" - + " account_number, tags"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("account_number", null, "bigint"), schema("tags", null, "string")); - - verifyDataRows( - result, - rows(1, "Amber JOHnny\nBrogan"), - rows(6, "Hattie\nDante"), - rows(13, "Nanette\nNogal")); - } - - @Test - public void testNoMvNullFieldValue() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK_WITH_NULL_VALUES - + " | where account_number = 6 | eval balance_str = cast(balance as string) | eval arr" - + " = array(balance_str) | nomv arr | fields account_number, arr"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("account_number", null, "bigint"), schema("arr", null, "string")); - - verifyDataRows(result, rows(6, "")); - } - - @Test - public void testNoMvArrayWithEmptyStrings() throws IOException { - String q = - "source=" - + TEST_INDEX_BANK - + " | eval tags = array('a', '', 'b') | nomv tags | head 1 | fields tags"; - - JSONObject result = executeQuery(q); - - verifySchema(result, schema("tags", null, "string")); - - verifyDataRows(result, rows("a\n\nb")); - } -} diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLSpathCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLSpathCommandIT.java index ec6f8583b23..51b5bd40304 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLSpathCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLSpathCommandIT.java @@ -8,7 +8,6 @@ import static org.opensearch.sql.util.MatcherUtils.rows; import static org.opensearch.sql.util.MatcherUtils.schema; import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; -import static org.opensearch.sql.util.MatcherUtils.verifyDataRowsInOrder; import static org.opensearch.sql.util.MatcherUtils.verifySchema; import java.io.IOException; @@ -25,7 +24,7 @@ public void init() throws Exception { loadIndex(Index.BANK); - // Simple JSON docs for path-based extraction + // Create test data for string concatenation Request request1 = new Request("PUT", "/test_spath/_doc/1?refresh=true"); request1.setJsonEntity("{\"doc\": \"{\\\"n\\\": 1}\"}"); client().performRequest(request1); @@ -37,37 +36,6 @@ public void init() throws Exception { Request request3 = new Request("PUT", "/test_spath/_doc/3?refresh=true"); request3.setJsonEntity("{\"doc\": \"{\\\"n\\\": 3}\"}"); client().performRequest(request3); - - // Auto-extract mode: flatten rules and edge cases (empty, malformed) - Request autoExtractDoc = new Request("PUT", "/test_spath_auto/_doc/1?refresh=true"); - autoExtractDoc.setJsonEntity( - "{\"nested_doc\": \"{\\\"user\\\":{\\\"name\\\":\\\"John\\\"}}\"," - + " \"array_doc\": \"{\\\"tags\\\":[\\\"java\\\",\\\"sql\\\"]}\"," - + " \"merge_doc\": \"{\\\"a\\\":{\\\"b\\\":1},\\\"a.b\\\":2}\"," - + " \"stringify_doc\": \"{\\\"n\\\":30,\\\"b\\\":true,\\\"x\\\":null}\"," - + " \"empty_doc\": \"{}\"," - + " \"malformed_doc\": \"{\\\"user\\\":{\\\"name\\\":\"}"); - client().performRequest(autoExtractDoc); - - // Auto-extract mode: 2-doc index for spath + command (eval/where/stats/sort) tests - Request cmdDoc1 = new Request("PUT", "/test_spath_cmd/_doc/1?refresh=true"); - cmdDoc1.setJsonEntity( - "{\"doc\": \"{\\\"user\\\":{\\\"name\\\":\\\"John\\\",\\\"age\\\":30}}\"}"); - client().performRequest(cmdDoc1); - - Request cmdDoc2 = new Request("PUT", "/test_spath_cmd/_doc/2?refresh=true"); - cmdDoc2.setJsonEntity( - "{\"doc\": \"{\\\"user\\\":{\\\"name\\\":\\\"Alice\\\",\\\"age\\\":25}}\"}"); - client().performRequest(cmdDoc2); - - // Auto-extract mode: null input handling (doc 1 establishes mapping, doc 2 has null) - Request nullDoc1 = new Request("PUT", "/test_spath_null/_doc/1?refresh=true"); - nullDoc1.setJsonEntity("{\"doc\": \"{\\\"n\\\": 1}\"}"); - client().performRequest(nullDoc1); - - Request nullDoc2 = new Request("PUT", "/test_spath_null/_doc/2?refresh=true"); - nullDoc2.setJsonEntity("{\"doc\": null}"); - client().performRequest(nullDoc2); } @Test @@ -77,143 +45,4 @@ public void testSimpleSpath() throws IOException { verifySchema(result, schema("result", "string")); verifyDataRows(result, rows("1"), rows("2"), rows("3")); } - - @Test - public void testSpathAutoExtract() throws IOException { - JSONObject result = executeQuery("source=test_spath | spath input=doc"); - verifySchema(result, schema("doc", "struct")); - verifyDataRows( - result, - rows(new JSONObject("{\"n\":\"1\"}")), - rows(new JSONObject("{\"n\":\"2\"}")), - rows(new JSONObject("{\"n\":\"3\"}"))); - } - - @Test - public void testSpathAutoExtractWithOutput() throws IOException { - JSONObject result = executeQuery("source=test_spath | spath input=doc output=result"); - verifySchema(result, schema("doc", "string"), schema("result", "struct")); - verifyDataRows( - result, - rows("{\"n\": 1}", new JSONObject("{\"n\":\"1\"}")), - rows("{\"n\": 2}", new JSONObject("{\"n\":\"2\"}")), - rows("{\"n\": 3}", new JSONObject("{\"n\":\"3\"}"))); - } - - @Test - public void testSpathAutoExtractNestedFields() throws IOException { - JSONObject result = - executeQuery( - "source=test_spath_auto | spath input=nested_doc output=result | fields result"); - - // Nested objects flatten to dotted keys: user.name - verifySchema(result, schema("result", "struct")); - verifyDataRows(result, rows(new JSONObject("{\"user.name\":\"John\"}"))); - } - - @Test - public void testSpathAutoExtractArraySuffix() throws IOException { - JSONObject result = - executeQuery( - "source=test_spath_auto | spath input=array_doc output=result | fields result"); - - // Arrays use {} suffix: tags{} - verifySchema(result, schema("result", "struct")); - verifyDataRows(result, rows(new JSONObject("{\"tags{}\":\"[java, sql]\"}"))); - } - - @Test - public void testSpathAutoExtractDuplicateKeysMerge() throws IOException { - JSONObject result = - executeQuery( - "source=test_spath_auto | spath input=merge_doc output=result | fields result"); - - // Duplicate logical keys merge into arrays: a.b from nested and dotted key - verifySchema(result, schema("result", "struct")); - verifyDataRows(result, rows(new JSONObject("{\"a.b\":\"[1, 2]\"}"))); - } - - @Test - public void testSpathAutoExtractStringifyAndNull() throws IOException { - JSONObject result = - executeQuery( - "source=test_spath_auto | spath input=stringify_doc output=result | fields result"); - - // All values stringified, null preserved - verifySchema(result, schema("result", "struct")); - verifyDataRows(result, rows(new JSONObject("{\"n\":\"30\",\"b\":\"true\",\"x\":\"null\"}"))); - } - - @Test - public void testSpathAutoExtractNullInput() throws IOException { - JSONObject result = - executeQuery("source=test_spath_null | spath input=doc output=result | fields result"); - - // Non-null doc extracts normally, null doc returns null - verifySchema(result, schema("result", "struct")); - verifyDataRows(result, rows(new JSONObject("{\"n\":\"1\"}")), rows((Object) null)); - } - - @Test - public void testSpathAutoExtractEmptyJson() throws IOException { - JSONObject result = - executeQuery( - "source=test_spath_auto | spath input=empty_doc output=result | fields result"); - - // Empty JSON object returns empty map - verifySchema(result, schema("result", "struct")); - verifyDataRows(result, rows(new JSONObject("{}"))); - } - - @Test - public void testSpathAutoExtractMalformedJson() throws IOException { - JSONObject result = - executeQuery( - "source=test_spath_auto | spath input=malformed_doc output=result | fields result"); - - // Malformed JSON returns partial results parsed before the error - verifySchema(result, schema("result", "struct")); - verifyDataRows(result, rows(new JSONObject("{}"))); - } - - @Test - public void testSpathAutoExtractWithEval() throws IOException { - JSONObject result = - executeQuery( - "source=test_spath_cmd | spath input=doc" - + " | eval name = doc.user.name | fields name"); - verifySchema(result, schema("name", "string")); - verifyDataRows(result, rows("Alice"), rows("John")); - } - - @Test - public void testSpathAutoExtractWithWhere() throws IOException { - JSONObject result = - executeQuery( - "source=test_spath_cmd | spath input=doc" - + " | where doc.user.name = 'John' | fields doc.user.name"); - verifySchema(result, schema("doc.user.name", "string")); - verifyDataRows(result, rows("John")); - } - - @Test - public void testSpathAutoExtractWithStats() throws IOException { - JSONObject result = - executeQuery( - "source=test_spath_cmd | spath input=doc" - + " | stats sum(doc.user.age) by doc.user.name"); - verifySchema(result, schema("sum(doc.user.age)", "double"), schema("doc.user.name", "string")); - verifyDataRows(result, rows(25, "Alice"), rows(30, "John")); - } - - @Test - public void testSpathAutoExtractWithSort() throws IOException { - // spath auto-extract + sort by path navigation on result - JSONObject result = - executeQuery( - "source=test_spath_cmd | spath input=doc" - + " | sort doc.user.name | fields doc.user.name"); - verifySchema(result, schema("doc.user.name", "string")); - verifyDataRowsInOrder(result, rows("Alice"), rows("John")); - } } diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/standalone/JsonExtractAllFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/standalone/JsonExtractAllFunctionIT.java index 7f1821c5ad8..68bf57ea8dd 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/standalone/JsonExtractAllFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/standalone/JsonExtractAllFunctionIT.java @@ -7,6 +7,7 @@ import java.sql.ResultSet; import java.sql.SQLException; +import java.util.List; import java.util.Map; import org.apache.calcite.rel.RelNode; import org.apache.calcite.rel.type.RelDataType; @@ -70,19 +71,19 @@ public void testJsonExtractAllWithSimpleObject() throws Exception { assertTrue(resultSet.next()); verifyColumns(resultSet, RESULT_FIELD); - Map map = getMap(resultSet, 1); + Map map = getMap(resultSet, 1); assertEquals("John", map.get("name")); - assertEquals("30", map.get("age")); + assertEquals(30, map.get("age")); assertEquals(2, map.size()); }); } - private Map getMap(ResultSet resultSet, int columnIndex) throws SQLException { + private Map getMap(ResultSet resultSet, int columnIndex) throws SQLException { Object result = resultSet.getObject(columnIndex); assertNotNull(result); assertTrue(result instanceof Map); - return (Map) result; + return (Map) result; } @Test @@ -108,10 +109,10 @@ public void testJsonExtractAllWithNestedObject() throws Exception { assertTrue(resultSet.next()); verifyColumns(resultSet, RESULT_FIELD); - Map map = getMap(resultSet, 1); + Map map = getMap(resultSet, 1); assertEquals("John", map.get("user.name")); - assertEquals("30", map.get("user.age")); - assertEquals("true", map.get("active")); + assertEquals(30, map.get("user.age")); + assertEquals(true, map.get("active")); assertEquals(3, map.size()); }); } @@ -139,9 +140,13 @@ public void testJsonExtractAllWithArray() throws Exception { assertTrue(resultSet.next()); verifyColumns(resultSet, RESULT_FIELD); - Map map = getMap(resultSet, 1); - assertEquals("[java, sql, opensearch]", map.get("tags{}")); - assertEquals(1, map.size()); + Map map = getMap(resultSet, 1); + List tags = getList(map, "tags{}"); + + assertEquals(3, tags.size()); + assertEquals("java", tags.get(0)); + assertEquals("sql", tags.get(1)); + assertEquals("opensearch", tags.get(2)); }); } @@ -168,8 +173,11 @@ public void testJsonExtractAllWithArrayOfObjects() throws Exception { assertTrue(resultSet.next()); verifyColumns(resultSet, RESULT_FIELD); - Map map = getMap(resultSet, 1); - assertEquals("[John, Jane]", map.get("users{}.name")); + Map map = getMap(resultSet, 1); + List names = getList(map, "users{}.name"); + assertEquals(2, names.size()); + assertEquals("John", names.get(0)); + assertEquals("Jane", names.get(1)); assertEquals(1, map.size()); // Only flattened key should exist }); } @@ -197,12 +205,24 @@ public void testJsonExtractAllWithTopLevelArray() throws Exception { assertTrue(resultSet.next()); verifyColumns(resultSet, RESULT_FIELD); - Map map = getMap(resultSet, 1); - assertEquals("[1, 2]", map.get("{}.id")); + Map map = getMap(resultSet, 1); + List ids = getList(map, "{}.id"); + assertEquals(2, ids.size()); + assertEquals(1, ids.get(0)); + assertEquals(2, ids.get(1)); assertEquals(1, map.size()); }); } + @SuppressWarnings("unchecked") + private List getList(Map map, String key) { + Object value = map.get(key); + assertNotNull(value); + assertTrue(value instanceof List); + + return (List) value; + } + @Test public void testJsonExtractAllWithEmptyObject() throws Exception { String jsonString = "{}"; @@ -226,7 +246,7 @@ public void testJsonExtractAllWithEmptyObject() throws Exception { assertTrue(resultSet.next()); verifyColumns(resultSet, RESULT_FIELD); - Map map = getMap(resultSet, 1); + Map map = getMap(resultSet, 1); assertTrue(map.isEmpty()); }); } @@ -254,7 +274,7 @@ public void testJsonExtractAllWithInvalidJson() throws Exception { assertTrue(resultSet.next()); verifyColumns(resultSet, RESULT_FIELD); - Map map = getMap(resultSet, 1); + Map map = getMap(resultSet, 1); assertEquals("John", map.get("name")); assertEquals(1, map.size()); }); diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index 099b2f7e0cb..5910bc8d476 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -686,11 +686,6 @@ public enum Index { "_doc", getNestedSimpleIndexMapping(), "src/test/resources/nested_simple.json"), - MVEXPAND_EDGE_CASES( - "mvexpand_edge_cases", - "mvexpand_edge_cases", - getMappingFile("mvexpand_edge_cases_mapping.json"), - "src/test/resources/mvexpand_edge_cases.json"), DEEP_NESTED( TestsConstants.TEST_INDEX_DEEP_NESTED, "_doc", diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java index 574f7b1eedb..b5e49bbe022 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java @@ -95,7 +95,6 @@ public class TestsConstants { public static final String TEST_INDEX_LOGS = TEST_INDEX + "_logs"; public static final String TEST_INDEX_OTEL_LOGS = TEST_INDEX + "_otel_logs"; public static final String TEST_INDEX_TIME_DATE_NULL = TEST_INDEX + "_time_date_null"; - public static final String TEST_INDEX_MVEXPAND_EDGE_CASES = "mvexpand_edge_cases"; public static final String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"; public static final String TS_DATE_FORMAT = "yyyy-MM-dd HH:mm:ss.SSS"; diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/NewAddedCommandsIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/NewAddedCommandsIT.java index a5164ec7fef..c5a1d08c37b 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/NewAddedCommandsIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/NewAddedCommandsIT.java @@ -10,11 +10,9 @@ import static org.opensearch.sql.common.setting.Settings.Key.CALCITE_ENGINE_ENABLED; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DOG; -import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_MVEXPAND_EDGE_CASES; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_STRINGS; import java.io.IOException; -import org.json.JSONArray; import org.json.JSONObject; import org.junit.jupiter.api.Test; import org.opensearch.client.ResponseException; @@ -27,7 +25,6 @@ public void init() throws Exception { loadIndex(Index.BANK); loadIndex(Index.DOG); loadIndex(Index.STRINGS); - loadIndex(Index.MVEXPAND_EDGE_CASES); } @Test @@ -255,233 +252,4 @@ public void testMvCombineUnsupportedInV2() throws IOException { } verifyQuery(result); } - - @Test - public void testNoMvUnsupportedInV2() throws IOException { - JSONObject result; - try { - result = - executeQuery( - String.format( - "source=%s | fields account_number, firstname | eval names = array(firstname) |" - + " nomv names", - TEST_INDEX_BANK)); - } catch (ResponseException e) { - result = new JSONObject(TestUtils.getResponseBody(e.getResponse())); - } - verifyQuery(result); - } - - @Test - public void testMvExpandCommandBasicExpansion() throws IOException { - JSONObject result; - try { - result = - executeQuery( - String.format( - "search source=%s | mvexpand skills | where username='happy' | fields username," - + " skills.name | sort skills.name", - TEST_INDEX_MVEXPAND_EDGE_CASES)); - } catch (ResponseException e) { - result = new JSONObject(TestUtils.getResponseBody(e.getResponse())); - } - - if (isCalciteEnabled()) { - assertThat(result.getJSONArray("datarows").length(), equalTo(3)); - - JSONArray datarows = result.getJSONArray("datarows"); - assertThat(datarows.getJSONArray(0).getString(0), equalTo("happy")); - assertThat(datarows.getJSONArray(0).getString(1), equalTo("java")); - assertThat(datarows.getJSONArray(1).getString(1), equalTo("python")); - assertThat(datarows.getJSONArray(2).getString(1), equalTo("sql")); - } else { - JSONObject error = result.getJSONObject("error"); - assertThat( - error.getString("details"), - containsString( - "is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true")); - assertThat(error.getString("type"), equalTo("UnsupportedOperationException")); - } - } - - @Test - public void testMvExpandCommandNullInput() throws IOException { - JSONObject result; - try { - result = - executeQuery( - String.format( - "search source=%s | mvexpand skills | where username='nullskills' | fields" - + " username, skills.name", - TEST_INDEX_MVEXPAND_EDGE_CASES)); - } catch (ResponseException e) { - result = new JSONObject(TestUtils.getResponseBody(e.getResponse())); - } - - if (isCalciteEnabled()) { - assertThat(result.getJSONArray("datarows").length(), equalTo(0)); - } else { - JSONObject error = result.getJSONObject("error"); - assertThat( - error.getString("details"), - containsString( - "is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true")); - assertThat(error.getString("type"), equalTo("UnsupportedOperationException")); - } - } - - @Test - public void testMvExpandCommandEmptyArray() throws IOException { - JSONObject result; - try { - result = - executeQuery( - String.format( - "search source=%s | mvexpand skills | where username='empty' | fields username," - + " skills.name", - TEST_INDEX_MVEXPAND_EDGE_CASES)); - } catch (ResponseException e) { - result = new JSONObject(TestUtils.getResponseBody(e.getResponse())); - } - - if (isCalciteEnabled()) { - assertThat(result.getJSONArray("datarows").length(), equalTo(0)); - } else { - JSONObject error = result.getJSONObject("error"); - assertThat( - error.getString("details"), - containsString( - "is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true")); - assertThat(error.getString("type"), equalTo("UnsupportedOperationException")); - } - } - - @Test - public void testMvExpandCommandNonArrayField() throws IOException { - JSONObject result; - try { - result = - executeQuery( - String.format( - "search source=%s | mvexpand skills_not_array | where username='u1' | fields" - + " username, skills_not_array", - TEST_INDEX_MVEXPAND_EDGE_CASES)); - } catch (ResponseException e) { - result = new JSONObject(TestUtils.getResponseBody(e.getResponse())); - } - - if (isCalciteEnabled()) { - assertThat(result.getJSONArray("datarows").length(), equalTo(1)); - assertThat(result.getJSONArray("datarows").getJSONArray(0).getString(1), equalTo("scala")); - } else { - JSONObject error = result.getJSONObject("error"); - assertThat( - error.getString("details"), - containsString( - "is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true")); - assertThat(error.getString("type"), equalTo("UnsupportedOperationException")); - } - } - - @Test - public void testMvExpandCommandLimitBoundary() throws IOException { - JSONObject result; - try { - result = - executeQuery( - String.format( - "search source=%s | mvexpand skills limit=3 | where username='limituser' | fields" - + " username, skills.name", - TEST_INDEX_MVEXPAND_EDGE_CASES)); - } catch (ResponseException e) { - result = new JSONObject(TestUtils.getResponseBody(e.getResponse())); - } - - if (isCalciteEnabled()) { - assertThat(result.getJSONArray("datarows").length(), equalTo(3)); - } else { - JSONObject error = result.getJSONObject("error"); - assertThat( - error.getString("details"), - containsString( - "is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true")); - assertThat(error.getString("type"), equalTo("UnsupportedOperationException")); - } - } - - @Test - public void testMvExpandCommandMultiDocument() throws IOException { - JSONObject result; - try { - result = - executeQuery( - String.format( - "search source=%s | mvexpand skills | where username='happy' OR username='single'" - + " | fields username, skills.name | sort username, skills.name", - TEST_INDEX_MVEXPAND_EDGE_CASES)); - } catch (ResponseException e) { - result = new JSONObject(TestUtils.getResponseBody(e.getResponse())); - } - - if (isCalciteEnabled()) { - assertThat(result.getJSONArray("datarows").length(), equalTo(4)); - - JSONArray datarows = result.getJSONArray("datarows"); - assertThat(datarows.getJSONArray(0).getString(0), equalTo("happy")); - assertThat(datarows.getJSONArray(3).getString(0), equalTo("single")); - } else { - JSONObject error = result.getJSONObject("error"); - assertThat( - error.getString("details"), - containsString( - "is supported only when " + CALCITE_ENGINE_ENABLED.getKeyValue() + "=true")); - assertThat(error.getString("type"), equalTo("UnsupportedOperationException")); - } - } - - @Test - public void testMvExpandInvalidLimitZero() throws IOException { - if (!isCalciteEnabled()) { - return; // Skip test when Calcite is disabled - } - try { - executeQuery( - String.format( - "search source=%s | mvexpand skills limit=0 | fields username, skills.name", - TEST_INDEX_MVEXPAND_EDGE_CASES)); - fail("Expected IllegalArgumentException for limit=0"); - } catch (ResponseException e) { - JSONObject result = new JSONObject(TestUtils.getResponseBody(e.getResponse())); - JSONObject error = result.getJSONObject("error"); - String details = error.getString("details"); - assertThat( - "Error message should mention limit or positive", - details.toLowerCase(), - containsString("limit")); - assertThat(error.getString("type"), equalTo("IllegalArgumentException")); - } - } - - @Test - public void testMvExpandInvalidLimitNegative() throws IOException { - if (!isCalciteEnabled()) { - return; // Skip test when Calcite is disabled - } - try { - executeQuery( - String.format( - "search source=%s | mvexpand skills limit=-1 | fields username, skills.name", - TEST_INDEX_MVEXPAND_EDGE_CASES)); - fail("Expected SyntaxCheckException for negative limit"); - } catch (ResponseException e) { - JSONObject result = new JSONObject(TestUtils.getResponseBody(e.getResponse())); - JSONObject error = result.getJSONObject("error"); - String details = error.getString("details"); - assertThat( - "Error message should mention parsing error", - details.toLowerCase(), - containsString("extraneous")); - assertThat(error.getString("type"), equalTo("SyntaxCheckException")); - } - } } diff --git a/integ-test/src/test/java/org/opensearch/sql/security/CalciteCrossClusterSearchIT.java b/integ-test/src/test/java/org/opensearch/sql/security/CalciteCrossClusterSearchIT.java index 13dbdce4bce..571d915517e 100644 --- a/integ-test/src/test/java/org/opensearch/sql/security/CalciteCrossClusterSearchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/security/CalciteCrossClusterSearchIT.java @@ -31,8 +31,6 @@ protected void init() throws Exception { loadIndex(Index.ACCOUNT, remoteClient()); loadIndex(Index.TIME_TEST_DATA); loadIndex(Index.TIME_TEST_DATA, remoteClient()); - loadIndex(Index.MVEXPAND_EDGE_CASES); - loadIndex(Index.MVEXPAND_EDGE_CASES, remoteClient()); enableCalcite(); } @@ -420,45 +418,4 @@ public void testCrossClusterFieldFormat() throws IOException { verifyDataRows( result, rows("Hattie", 36, 5686, "$5,686"), rows("Nanette", 28, 32838, "$32,838")); } - - /** CrossClusterSearchIT Test for nomv. */ - @Test - public void testCrossClusterNoMv() throws IOException { - JSONObject result = - executeQuery( - String.format( - "search source=%s | where firstname='Hattie' " - + "| eval names = array(firstname, lastname) | nomv names " - + "| fields firstname, names", - TEST_INDEX_BANK_REMOTE)); - - verifyColumn(result, columnName("firstname"), columnName("names")); - verifySchema(result, schema("firstname", "string"), schema("names", "string")); - - verifyDataRows(result, rows("Hattie", "Hattie\nBond")); - } - - @Test - public void testCrossClusterMvExpandBasic() throws IOException { - JSONObject result = - executeQuery( - String.format( - "search source=%s | mvexpand skills | where username='happy' | fields username," - + " skills.name | sort skills.name", - TEST_INDEX_MVEXPAND_REMOTE)); - verifySchema(result, schema("username", "string"), schema("skills.name", "string")); - verifyDataRows(result, rows("happy", "java"), rows("happy", "python"), rows("happy", "sql")); - } - - @Test - public void testCrossClusterMvExpandWithLimit() throws IOException { - JSONObject result = - executeQuery( - String.format( - "search source=%s | mvexpand skills limit=2 | where username='limituser' | fields" - + " username, skills.name | sort skills.name", - TEST_INDEX_MVEXPAND_REMOTE)); - verifySchema(result, schema("username", "string"), schema("skills.name", "string")); - verifyDataRows(result, rows("limituser", "a"), rows("limituser", "b")); - } } diff --git a/integ-test/src/test/java/org/opensearch/sql/security/CrossClusterTestBase.java b/integ-test/src/test/java/org/opensearch/sql/security/CrossClusterTestBase.java index dc4d7d0dafd..d9de95c663b 100644 --- a/integ-test/src/test/java/org/opensearch/sql/security/CrossClusterTestBase.java +++ b/integ-test/src/test/java/org/opensearch/sql/security/CrossClusterTestBase.java @@ -8,7 +8,6 @@ import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DOG; -import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_MVEXPAND_EDGE_CASES; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_TIME_DATA; import org.opensearch.sql.ppl.PPLIntegTestCase; @@ -37,8 +36,6 @@ public class CrossClusterTestBase extends PPLIntegTestCase { REMOTE_CLUSTER + ":" + TEST_INDEX_ACCOUNT; protected static final String TEST_INDEX_TIME_DATA_REMOTE = REMOTE_CLUSTER + ":" + TEST_INDEX_TIME_DATA; - protected static final String TEST_INDEX_MVEXPAND_REMOTE = - REMOTE_CLUSTER + ":" + TEST_INDEX_MVEXPAND_EDGE_CASES; @Override protected void init() throws Exception { diff --git a/integ-test/src/test/resources/expectedOutput/calcite/explain_issue_5114_sort_expr_head_push.yaml b/integ-test/src/test/resources/expectedOutput/calcite/explain_issue_5114_sort_expr_head_push.yaml deleted file mode 100644 index 84d91a3885f..00000000000 --- a/integ-test/src/test/resources/expectedOutput/calcite/explain_issue_5114_sort_expr_head_push.yaml +++ /dev/null @@ -1,9 +0,0 @@ -calcite: - logical: | - LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT]) - LogicalProject(account_number=[$0]) - LogicalSort(sort0=[$17], dir0=[ASC-nulls-first], fetch=[5]) - LogicalProject(account_number=[$0], firstname=[$1], address=[$2], balance=[$3], gender=[$4], city=[$5], employer=[$6], state=[$7], age=[$8], email=[$9], lastname=[$10], _id=[$11], _index=[$12], _score=[$13], _maxscore=[$14], _sort=[$15], _routing=[$16], a=[RAND()]) - CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]]) - physical: | - CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]], PushDownContext=[[PROJECT->[account_number], SORT_EXPR->[RAND() ASCENDING NULLS_FIRST], LIMIT->5, LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={"from":0,"size":5,"timeout":"1m","_source":{"includes":["account_number"],"excludes":[]},"sort":[{"_script":{"script":{"source":"{\"langType\":\"calcite\",\"script\":\"rO0ABXQAbnsKICAib3AiOiB7CiAgICAibmFtZSI6ICJSQU5EIiwKICAgICJraW5kIjogIk9USEVSX0ZVTkNUSU9OIiwKICAgICJzeW50YXgiOiAiRlVOQ1RJT04iCiAgfSwKICAib3BlcmFuZHMiOiBbXQp9\"}","lang":"opensearch_compounded_script","params":{"MISSING_MAX":false,"utcTimestamp": 0,"SOURCES":[],"DIGESTS":[]}},"type":"number","order":"asc"}}]}, requestedTotalSize=5, pageSize=null, startFrom=0)]) diff --git a/integ-test/src/test/resources/expectedOutput/calcite/explain_mvexpand.yaml b/integ-test/src/test/resources/expectedOutput/calcite/explain_mvexpand.yaml deleted file mode 100644 index c07f8d5f063..00000000000 --- a/integ-test/src/test/resources/expectedOutput/calcite/explain_mvexpand.yaml +++ /dev/null @@ -1,19 +0,0 @@ -calcite: - logical: | - LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT]) - LogicalProject(skills=[$0], skills_int=[$3], skills_not_array=[$4], username=[$5], skills_arr=[$13]) - LogicalCorrelate(correlation=[$cor0], joinType=[inner], requiredColumns=[{12}]) - LogicalProject(skills=[$0], skills.level=[$1], skills.name=[$2], skills_int=[$3], skills_not_array=[$4], username=[$5], _id=[$6], _index=[$7], _score=[$8], _maxscore=[$9], _sort=[$10], _routing=[$11], skills_arr=[array(1, 2, 3)]) - CalciteLogicalIndexScan(table=[[OpenSearch, mvexpand_edge_cases]]) - Uncollect - LogicalProject(skills_arr=[$cor0.skills_arr]) - LogicalValues(tuples=[[{ 0 }]]) - physical: | - EnumerableLimit(fetch=[10000]) - EnumerableCalc(expr#0..5=[{inputs}], proj#0..3=[{exprs}], skills_arr=[$t5]) - EnumerableCorrelate(correlation=[$cor0], joinType=[inner], requiredColumns=[{4}]) - EnumerableCalc(expr#0..3=[{inputs}], expr#4=[1], expr#5=[2], expr#6=[3], expr#7=[array($t4, $t5, $t6)], proj#0..3=[{exprs}], skills_arr=[$t7]) - CalciteEnumerableIndexScan(table=[[OpenSearch, mvexpand_edge_cases]], PushDownContext=[[PROJECT->[skills, skills_int, skills_not_array, username]], OpenSearchRequestBuilder(sourceBuilder={"from":0,"timeout":"1m","_source":{"includes":["skills","skills_int","skills_not_array","username"],"excludes":[]}}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)]) - EnumerableUncollect - EnumerableCalc(expr#0=[{inputs}], expr#1=[$cor0], expr#2=[$t1.skills_arr], skills_arr=[$t2]) - EnumerableValues(tuples=[[{ 0 }]]) diff --git a/integ-test/src/test/resources/expectedOutput/calcite/explain_nomv.yaml b/integ-test/src/test/resources/expectedOutput/calcite/explain_nomv.yaml deleted file mode 100644 index e522ceb639e..00000000000 --- a/integ-test/src/test/resources/expectedOutput/calcite/explain_nomv.yaml +++ /dev/null @@ -1,10 +0,0 @@ -calcite: - logical: | - LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT]) - LogicalProject(state=[$7], city=[$5], age=[$8], location=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array($7, $5)), ' - '), '':VARCHAR)]) - CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]]) - physical: | - EnumerableCalc(expr#0..2=[{inputs}], expr#3=[array($t0, $t1)], expr#4=[ARRAY_COMPACT($t3)], expr#5=[' - '], expr#6=[ARRAY_JOIN($t4, $t5)], expr#7=['':VARCHAR], expr#8=[COALESCE($t6, $t7)], proj#0..2=[{exprs}], location=[$t8]) - CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]], PushDownContext=[[PROJECT->[state, city, age], LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={"from":0,"size":10000,"timeout":"1m","_source":{"includes":["state","city","age"],"excludes":[]}}, requestedTotalSize=10000, pageSize=null, startFrom=0)]) diff --git a/integ-test/src/test/resources/expectedOutput/calcite_no_pushdown/explain_mvexpand.yaml b/integ-test/src/test/resources/expectedOutput/calcite_no_pushdown/explain_mvexpand.yaml deleted file mode 100644 index 5efc2d80da9..00000000000 --- a/integ-test/src/test/resources/expectedOutput/calcite_no_pushdown/explain_mvexpand.yaml +++ /dev/null @@ -1,19 +0,0 @@ -calcite: - logical: | - LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT]) - LogicalProject(skills=[$0], skills_int=[$3], skills_not_array=[$4], username=[$5], skills_arr=[$13]) - LogicalCorrelate(correlation=[$cor0], joinType=[inner], requiredColumns=[{12}]) - LogicalProject(skills=[$0], skills.level=[$1], skills.name=[$2], skills_int=[$3], skills_not_array=[$4], username=[$5], _id=[$6], _index=[$7], _score=[$8], _maxscore=[$9], _sort=[$10], _routing=[$11], skills_arr=[array(1, 2, 3)]) - CalciteLogicalIndexScan(table=[[OpenSearch, mvexpand_edge_cases]]) - Uncollect - LogicalProject(skills_arr=[$cor0.skills_arr]) - LogicalValues(tuples=[[{ 0 }]]) - physical: | - EnumerableLimit(fetch=[10000]) - EnumerableCalc(expr#0..5=[{inputs}], proj#0..3=[{exprs}], skills_arr=[$t5]) - EnumerableCorrelate(correlation=[$cor0], joinType=[inner], requiredColumns=[{4}]) - EnumerableCalc(expr#0..11=[{inputs}], expr#12=[1], expr#13=[2], expr#14=[3], expr#15=[array($t12, $t13, $t14)], skills=[$t0], skills_int=[$t3], skills_not_array=[$t4], username=[$t5], skills_arr=[$t15]) - CalciteEnumerableIndexScan(table=[[OpenSearch, mvexpand_edge_cases]]) - EnumerableUncollect - EnumerableCalc(expr#0=[{inputs}], expr#1=[$cor0], expr#2=[$t1.skills_arr], skills_arr=[$t2]) - EnumerableValues(tuples=[[{ 0 }]]) diff --git a/integ-test/src/test/resources/expectedOutput/calcite_no_pushdown/explain_nomv.yaml b/integ-test/src/test/resources/expectedOutput/calcite_no_pushdown/explain_nomv.yaml deleted file mode 100644 index ace49cb6b96..00000000000 --- a/integ-test/src/test/resources/expectedOutput/calcite_no_pushdown/explain_nomv.yaml +++ /dev/null @@ -1,11 +0,0 @@ -calcite: - logical: | - LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT]) - LogicalProject(state=[$7], city=[$5], age=[$8], location=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array($7, $5)), ' - '), '':VARCHAR)]) - CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]]) - physical: | - EnumerableLimit(fetch=[10000]) - EnumerableCalc(expr#0..16=[{inputs}], expr#17=[array($t7, $t5)], expr#18=[ARRAY_COMPACT($t17)], expr#19=[' - '], expr#20=[ARRAY_JOIN($t18, $t19)], expr#21=['':VARCHAR], expr#22=[COALESCE($t20, $t21)], state=[$t7], city=[$t5], age=[$t8], location=[$t22]) - CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]]) diff --git a/integ-test/src/test/resources/indexDefinitions/mvexpand_edge_cases_mapping.json b/integ-test/src/test/resources/indexDefinitions/mvexpand_edge_cases_mapping.json deleted file mode 100644 index a0b5519d176..00000000000 --- a/integ-test/src/test/resources/indexDefinitions/mvexpand_edge_cases_mapping.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "mappings": { - "properties": { - "username": { "type": "keyword" }, - "skills": { - "type": "nested", - "properties": { - "name": { "type": "keyword" }, - "level": { "type": "keyword" } - } - }, - "skills_not_array": { "type": "keyword" }, - "skills_int": { "type": "integer" } - } - } -} diff --git a/integ-test/src/test/resources/mvexpand_edge_cases.json b/integ-test/src/test/resources/mvexpand_edge_cases.json deleted file mode 100644 index c7632bb1e24..00000000000 --- a/integ-test/src/test/resources/mvexpand_edge_cases.json +++ /dev/null @@ -1,30 +0,0 @@ -{"index":{}} -{"username":"happy","skills":[{"name":"python"},{"name":"java"},{"name":"sql"}]} -{"index":{}} -{"username":"single","skills":[{"name":"go"}]} -{"index":{}} -{"username":"empty","skills":[]} -{"index":{}} -{"username":"nullskills","skills":null} -{"index":{}} -{"username":"noskills"} -{"index":{}} -{"username":"missingattr","skills":[{"name":"c"},{"level":"advanced"}]} -{"index":{}} -{"username":"complex","skills":[{"name":"ml","level":"expert"},{"name":"ai"},{"level":"novice"}]} -{"index":{}} -{"username":"duplicate","skills":[{"name":"dup"},{"name":"dup"}]} -{"index":{}} -{"username":"large","skills":[{"name":"s1"},{"name":"s2"},{"name":"s3"},{"name":"s4"},{"name":"s5"},{"name":"s6"},{"name":"s7"},{"name":"s8"},{"name":"s9"},{"name":"s10"}]} -{"index":{}} -{"username":"partial","skills":[{"name":"kotlin"},{"level":"intermediate"},{"level":"advanced"}]} -{"index":{}} -{"username":"mixed_shapes","skills":[{"name":"elixir"},{"name":"haskell"}]} -{"index":{}} -{"username":"hetero_types","skills":[{"level":"senior"},{"level":"3"}]} -{"index":{}} -{"username":"limituser","skills":[{"name":"a"},{"name":"b"},{"name":"c"},{"name":"d"},{"name":"e"}]} -{"index":{}} -{"username":"u1","skills_not_array":"scala"} -{"index":{}} -{"username":"u_int","skills_int":5} diff --git a/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5060.yml b/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5060.yml deleted file mode 100644 index 87932a07a10..00000000000 --- a/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5060.yml +++ /dev/null @@ -1,66 +0,0 @@ -# Issue: https://github.com/opensearch-project/sql/issues/5060 -# PR: https://github.com/opensearch-project/sql/pull/5133 -# When Calcite falls back to V2 and V2 also fails, return the original Calcite error instead of V2's. -# -# The AD command forces a V2 fallback, then join is only supported in V3 (Calcite). -# This test verifies that when both Calcite and V2 fail, the error message correctly shows -# the Calcite error (CalciteUnsupportedException) rather than the V2 error. - -setup: - - do: - query.settings: - body: - transient: - plugins.calcite.enabled: true - - do: - indices.create: - index: test_join_ad_error_5133 - body: - mappings: - properties: - "event.id": - type: keyword - "@timestamp": - type: date - message: - type: text - - do: - bulk: - index: test_join_ad_error_5133 - refresh: true - body: - - '{"index": {}}' - - '{"event.id": "evt1", "@timestamp": "2025-01-15T10:30:00Z", "message": "test message 1"}' - - '{"index": {}}' - - '{"event.id": "evt2", "@timestamp": "2025-01-15T10:31:00Z", "message": "test message 2"}' - ---- -teardown: - - do: - query.settings: - body: - transient: - plugins.calcite.enabled: false - - do: - indices.delete: - index: test_join_ad_error_5133 - ---- -"Join with AD command should return Calcite error when both Calcite and V2 fail": - - skip: - features: - - headers - - allowed_warnings - # Before the fix: Returns V2 error "Join is supported only when plugins.calcite.enabled=true" (status 500) - # After the fix: Returns Calcite error "AD command is unsupported in Calcite" (status 400) - - do: - allowed_warnings: - - 'Loading the fielddata on the _id field is deprecated and will be removed in future versions. If you require sorting or aggregating on this field you should also include the id in the body of your documents, and map this field as a keyword field that has [doc_values] enabled' - catch: bad_request - headers: - Content-Type: 'application/json' - ppl: - body: - query: source=test_join_ad_error_5133 | join `event.id` [source = test_join_ad_error_5133] | ad time_field='@timestamp' - - match: { "$body": "/CalciteUnsupportedException/" } - - match: { "$body": "/AD\\s+command\\s+is\\s+unsupported\\s+in\\s+Calcite/" } diff --git a/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5114.yml b/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5114.yml deleted file mode 100644 index 8b463de6b06..00000000000 --- a/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5114.yml +++ /dev/null @@ -1,97 +0,0 @@ -setup: - - do: - query.settings: - body: - transient: - plugins.calcite.enabled: true - - - do: - indices.create: - index: issue5114 - body: - settings: - number_of_shards: 1 - number_of_replicas: 0 - mappings: - properties: - id: - type: integer - name: - type: keyword - reportsTo: - type: keyword - - - do: - bulk: - refresh: true - body: - - '{"index": {"_index": "issue5114", "_id": "1"}}' - - '{"id": 1, "name": "Dev", "reportsTo": "Eliot"}' - - '{"index": {"_index": "issue5114", "_id": "2"}}' - - '{"id": 2, "name": "Eliot", "reportsTo": "Ron"}' - - '{"index": {"_index": "issue5114", "_id": "3"}}' - - '{"id": 3, "name": "Ron", "reportsTo": "Andrew"}' - - '{"index": {"_index": "issue5114", "_id": "4"}}' - - '{"id": 4, "name": "Andrew", "reportsTo": null}' - - '{"index": {"_index": "issue5114", "_id": "5"}}' - - '{"id": 5, "name": "Asya", "reportsTo": "Ron"}' - - '{"index": {"_index": "issue5114", "_id": "6"}}' - - '{"id": 6, "name": "Dan", "reportsTo": "Andrew"}' - ---- -teardown: - - do: - indices.delete: - index: issue5114 - ignore_unavailable: true - - do: - query.settings: - body: - transient: - plugins.calcite.enabled: false - ---- -"Issue 5114: head should be preserved for non-order-equivalent deterministic sort expression": - - skip: - features: - - headers - - do: - headers: - Content-Type: 'application/json' - ppl: - body: - query: source=issue5114 | eval a = abs(id) + 1 | sort a | fields id | head 5 - - - match: { total: 5 } - - match: { schema: [ { name: id, type: int } ] } - ---- -"Issue 5114: head should be preserved for non-deterministic sort expression": - - skip: - features: - - headers - - do: - headers: - Content-Type: 'application/json' - ppl: - body: - query: source=issue5114 | eval a = rand() | sort a | fields id | head 5 - - - match: { total: 5 } - - match: { schema: [ { name: id, type: int } ] } - ---- -"Issue 5114 control: order-equivalent expression remains correct": - - skip: - features: - - headers - - do: - headers: - Content-Type: 'application/json' - ppl: - body: - query: source=issue5114 | eval a = id + 1 | sort a | fields id | head 5 - - - match: { total: 5 } - - match: { schema: [ { name: id, type: int } ] } - - match: { datarows: [ [ 1 ], [ 2 ], [ 3 ], [ 4 ], [ 5 ] ] } diff --git a/legacy/build.gradle b/legacy/build.gradle index 74653d9cb36..fd6d7c8f65c 100644 --- a/legacy/build.gradle +++ b/legacy/build.gradle @@ -120,8 +120,8 @@ dependencies { api project(':opensearch') // ANTLR gradle plugin and runtime dependency - antlr "org.antlr:antlr4:4.13.2" - implementation "org.antlr:antlr4-runtime:4.13.2" + antlr "org.antlr:antlr4:4.7.1" + implementation "org.antlr:antlr4-runtime:4.7.1" compileOnly group: 'javax.servlet', name: 'servlet-api', version:'2.5' testImplementation group: 'org.hamcrest', name: 'hamcrest-core', version:'2.2' diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/planner/rules/SortExprIndexScanRule.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/planner/rules/SortExprIndexScanRule.java index 46e40729d59..b7c25912bca 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/planner/rules/SortExprIndexScanRule.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/planner/rules/SortExprIndexScanRule.java @@ -11,7 +11,6 @@ import java.util.Map; import java.util.Optional; import java.util.function.Predicate; -import org.apache.calcite.adapter.enumerable.EnumerableLimitSort; import org.apache.calcite.plan.RelOptRuleCall; import org.apache.calcite.rel.RelFieldCollation; import org.apache.calcite.rel.RelFieldCollation.Direction; @@ -48,11 +47,6 @@ protected SortExprIndexScanRule(SortExprIndexScanRule.Config config) { @Override protected void onMatchImpl(RelOptRuleCall call) { final Sort sort = call.rel(0); - // EnumerableLimitSort carries fetch semantics; this rule doesn't preserve it on physical - // scans because limit pushdown path is logical-only. - if (sort instanceof EnumerableLimitSort) { - return; - } final Project project = call.rel(1); final AbstractCalciteIndexScan scan = call.rel(2); diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/CalciteEnumerableGraphLookup.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/CalciteEnumerableGraphLookup.java index dd9e3c3f6bc..6307a741468 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/CalciteEnumerableGraphLookup.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/CalciteEnumerableGraphLookup.java @@ -212,32 +212,27 @@ private static class GraphLookupEnumerator implements Enumerator<@Nullable Objec "Source must be Scannable, got: " + graphLookup.getSource().getClass()); } - try { - List sourceFields = graphLookup.getSource().getRowType().getFieldNames(); - this.lookupFields = graphLookup.getLookup().getRowType().getFieldNames(); - this.startFieldIndex = sourceFields.indexOf(graphLookup.getStartField()); - this.fromFieldIdx = lookupFields.indexOf(graphLookup.fromField); - this.toFieldIdx = lookupFields.indexOf(graphLookup.toField); - - // Push down user-specified filter to the lookup scan - if (graphLookup.filter != null) { - List schema = graphLookup.getLookup().getRowType().getFieldNames(); - Map fieldTypes = this.lookupScan.getOsIndex().getAllFieldTypes(); - try { - QueryBuilder filterQuery = - PredicateAnalyzer.analyze(graphLookup.filter, schema, fieldTypes); - this.lookupScan.pushDownContext.add( - PushDownType.FILTER, - null, - (OSRequestBuilderAction) rb -> rb.pushDownFilterForCalcite(filterQuery)); - } catch (PredicateAnalyzer.ExpressionNotAnalyzableException e) { - throw new RuntimeException( - "Cannot push down filter for graphLookup: " + e.getMessage(), e); - } + List sourceFields = graphLookup.getSource().getRowType().getFieldNames(); + this.lookupFields = graphLookup.getLookup().getRowType().getFieldNames(); + this.startFieldIndex = sourceFields.indexOf(graphLookup.getStartField()); + this.fromFieldIdx = lookupFields.indexOf(graphLookup.fromField); + this.toFieldIdx = lookupFields.indexOf(graphLookup.toField); + + // Push down user-specified filter to the lookup scan + if (graphLookup.filter != null) { + List schema = graphLookup.getLookup().getRowType().getFieldNames(); + Map fieldTypes = this.lookupScan.getOsIndex().getAllFieldTypes(); + try { + QueryBuilder filterQuery = + PredicateAnalyzer.analyze(graphLookup.filter, schema, fieldTypes); + this.lookupScan.pushDownContext.add( + PushDownType.FILTER, + null, + (OSRequestBuilderAction) rb -> rb.pushDownFilterForCalcite(filterQuery)); + } catch (PredicateAnalyzer.ExpressionNotAnalyzableException e) { + throw new RuntimeException( + "Cannot push down filter for graphLookup: " + e.getMessage(), e); } - } catch (Exception e) { - sourceEnumerator.close(); - throw e; } } @@ -483,15 +478,12 @@ private List queryLookupTable( (OSRequestBuilderAction) requestBuilder -> requestBuilder.pushDownFilterForCalcite(finalQuery)); Iterator<@Nullable Object> res = newScan.scan().iterator(); - try { - List results = new ArrayList<>(); - while (res.hasNext()) { - results.add(res.next()); - } - return results; - } finally { - closeIterator(res); + List results = new ArrayList<>(); + while (res.hasNext()) { + results.add(res.next()); } + closeIterator(res); + return results; } private static void closeIterator(@Nullable Iterator iterator) { diff --git a/ppl/build.gradle b/ppl/build.gradle index caf5223103c..ef7973b1e37 100644 --- a/ppl/build.gradle +++ b/ppl/build.gradle @@ -48,11 +48,11 @@ configurations { } dependencies { - antlr "org.antlr:antlr4:4.13.2" + antlr "org.antlr:antlr4:4.7.1" runtimeOnly group: 'org.reflections', name: 'reflections', version: '0.9.12' - implementation "org.antlr:antlr4-runtime:4.13.2" + implementation "org.antlr:antlr4-runtime:4.7.1" implementation group: 'com.google.guava', name: 'guava', version: "${guava_version}" api group: 'org.json', name: 'json', version: '20231013' implementation group: 'org.apache.logging.log4j', name: 'log4j-core', version:"${versions.log4j}" diff --git a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 index fde9845e081..e456914ff59 100644 --- a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 @@ -68,7 +68,6 @@ USE_PIT: 'USEPIT'; ROW: 'ROW'; COL: 'COL'; EXPAND: 'EXPAND'; -MVEXPAND: 'MVEXPAND'; SIMPLE_PATTERN: 'SIMPLE_PATTERN'; BRAIN: 'BRAIN'; VARIABLE_COUNT_THRESHOLD: 'VARIABLE_COUNT_THRESHOLD'; @@ -87,7 +86,6 @@ AGGREGATION: 'AGGREGATION'; APPENDPIPE: 'APPENDPIPE'; COLUMN_NAME: 'COLUMN_NAME'; MVCOMBINE: 'MVCOMBINE'; -NOMV: 'NOMV'; //Native JOIN KEYWORDS diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index 7caf2be8070..1749c6ebf4c 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -80,7 +80,6 @@ commands | addcoltotalsCommand | appendCommand | expandCommand - | mvexpandCommand | flattenCommand | reverseCommand | regexCommand @@ -92,7 +91,6 @@ commands | replaceCommand | mvcombineCommand | fieldformatCommand - | nomvCommand | graphLookupCommand ; @@ -125,7 +123,6 @@ commandName | ML | FILLNULL | EXPAND - | MVEXPAND | FLATTEN | TRENDLINE | TIMECHART @@ -140,7 +137,6 @@ commandName | APPENDPIPE | REPLACE | MVCOMBINE - | NOMV | TRANSPOSE | GRAPHLOOKUP ; @@ -561,14 +557,6 @@ mvcombineCommand : MVCOMBINE fieldExpression (DELIM EQUAL stringLiteral)? ; -nomvCommand - : NOMV fieldExpression - ; - -mvexpandCommand - : MVEXPAND fieldExpression (LIMIT EQUAL INTEGER_LITERAL)? - ; - flattenCommand : FLATTEN fieldExpression (AS aliases = identifierSeq)? ; diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java index ccee54f5d64..45d9a14bd89 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java @@ -95,8 +95,6 @@ import org.opensearch.sql.ast.tree.MinSpanBin; import org.opensearch.sql.ast.tree.Multisearch; import org.opensearch.sql.ast.tree.MvCombine; -import org.opensearch.sql.ast.tree.MvExpand; -import org.opensearch.sql.ast.tree.NoMv; import org.opensearch.sql.ast.tree.Parse; import org.opensearch.sql.ast.tree.Patterns; import org.opensearch.sql.ast.tree.Project; @@ -912,25 +910,6 @@ public UnresolvedPlan visitMvcombineCommand(OpenSearchPPLParser.MvcombineCommand return new MvCombine(field, delim); } - @Override - public UnresolvedPlan visitNomvCommand(OpenSearchPPLParser.NomvCommandContext ctx) { - Field field = (Field) internalVisitExpression(ctx.fieldExpression()); - return new NoMv(field); - } - - @Override - public UnresolvedPlan visitMvexpandCommand(OpenSearchPPLParser.MvexpandCommandContext ctx) { - Field field = (Field) expressionBuilder.visit(ctx.fieldExpression()); - Integer limit = - ctx.INTEGER_LITERAL() != null ? Integer.parseInt(ctx.INTEGER_LITERAL().getText()) : null; - - if (limit != null && limit <= 0) { - throw new IllegalArgumentException("Limit must be a positive number, got: " + limit); - } - - return new MvExpand(field, limit); - } - @Override public UnresolvedPlan visitGrokCommand(OpenSearchPPLParser.GrokCommandContext ctx) { UnresolvedExpression sourceField = internalVisitExpression(ctx.source_field); @@ -968,6 +947,9 @@ public UnresolvedPlan visitSpathCommand(OpenSearchPPLParser.SpathCommandContext if (inField == null) { throw new IllegalArgumentException("`input` parameter is required for `spath`"); } + if (path == null) { + throw new IllegalArgumentException("`path` parameter is required for `spath`"); + } return new SPath(inField, outField, path); } diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/utils/PPLQueryDataAnonymizer.java b/ppl/src/main/java/org/opensearch/sql/ppl/utils/PPLQueryDataAnonymizer.java index 3a716d1ecee..90f4ce92724 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/utils/PPLQueryDataAnonymizer.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/utils/PPLQueryDataAnonymizer.java @@ -84,8 +84,6 @@ import org.opensearch.sql.ast.tree.MinSpanBin; import org.opensearch.sql.ast.tree.Multisearch; import org.opensearch.sql.ast.tree.MvCombine; -import org.opensearch.sql.ast.tree.MvExpand; -import org.opensearch.sql.ast.tree.NoMv; import org.opensearch.sql.ast.tree.Parse; import org.opensearch.sql.ast.tree.Patterns; import org.opensearch.sql.ast.tree.Project; @@ -512,24 +510,6 @@ public String visitMvCombine(MvCombine node, String context) { return StringUtils.format("%s | mvcombine delim=%s %s", child, MASK_LITERAL, field); } - @Override - public String visitNoMv(NoMv node, String context) { - String child = node.getChild().getFirst().accept(this, context); - String field = visitExpression(node.getField()); - - return StringUtils.format("%s | nomv %s", child, field); - } - - @Override - public String visitMvExpand(MvExpand node, String context) { - String child = node.getChild().get(0).accept(this, context); - String field = MASK_COLUMN; // Always anonymize field names - if (node.getLimit() != null) { - return StringUtils.format("%s | mvexpand %s limit=%s", child, field, MASK_LITERAL); - } - return StringUtils.format("%s | mvexpand %s", child, field); - } - /** Build {@link LogicalSort}. */ @Override public String visitSort(Sort node, String context) { diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLFunctionTypeTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLFunctionTypeTest.java index 4383acf40e0..9513558952f 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLFunctionTypeTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLFunctionTypeTest.java @@ -298,6 +298,6 @@ public void testValuesFunctionWithArrayArgType() { public void testMvjoinRejectsNonStringValues() { verifyQueryThrowsException( "source=EMP | eval result = mvjoin(42, ',') | fields result | head 1", - "MVJOIN function expects {[ARRAY,STRING]|[ARRAY,STRING,STRING]}, but got [INTEGER,STRING]"); + "MVJOIN function expects {[ARRAY,STRING]}, but got [INTEGER,STRING]"); } } diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLMvExpandTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLMvExpandTest.java deleted file mode 100644 index ac37dab12a9..00000000000 --- a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLMvExpandTest.java +++ /dev/null @@ -1,279 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.ppl.calcite; - -import com.google.common.collect.ImmutableList; -import java.util.Arrays; -import java.util.List; -import org.apache.calcite.config.CalciteConnectionConfig; -import org.apache.calcite.plan.RelTraitDef; -import org.apache.calcite.rel.RelCollations; -import org.apache.calcite.rel.RelNode; -import org.apache.calcite.rel.type.RelDataType; -import org.apache.calcite.rel.type.RelDataTypeFactory; -import org.apache.calcite.rel.type.RelProtoDataType; -import org.apache.calcite.schema.Schema; -import org.apache.calcite.schema.SchemaPlus; -import org.apache.calcite.schema.Statistic; -import org.apache.calcite.schema.Statistics; -import org.apache.calcite.schema.Table; -import org.apache.calcite.sql.SqlCall; -import org.apache.calcite.sql.SqlNode; -import org.apache.calcite.sql.parser.SqlParser; -import org.apache.calcite.sql.type.SqlTypeName; -import org.apache.calcite.test.CalciteAssert; -import org.apache.calcite.tools.Frameworks; -import org.apache.calcite.tools.Programs; -import org.checkerframework.checker.nullness.qual.Nullable; -import org.junit.Assert; -import org.junit.Test; - -public class CalcitePPLMvExpandTest extends CalcitePPLAbstractTest { - - public CalcitePPLMvExpandTest() { - super(CalciteAssert.SchemaSpec.SCOTT_WITH_TEMPORAL); - } - - /** - * There is no existing table with arrays. We create one for test purpose. - * - *

This mirrors CalcitePPLExpandTest.TableWithArray. - */ - public static class TableWithArray implements Table { - protected final RelProtoDataType protoRowType = - factory -> - factory - .builder() - .add("DEPTNO", SqlTypeName.INTEGER) - .add( - "EMPNOS", - factory.createArrayType(factory.createSqlType(SqlTypeName.INTEGER), -1)) - .add( - "TAGS", - factory.createMultisetType(factory.createSqlType(SqlTypeName.VARCHAR), -1)) - .build(); - - @Override - public RelDataType getRowType(RelDataTypeFactory typeFactory) { - return protoRowType.apply(typeFactory); - } - - @Override - public Statistic getStatistic() { - return Statistics.of(0d, ImmutableList.of(), RelCollations.createSingleton(0)); - } - - @Override - public Schema.TableType getJdbcTableType() { - return Schema.TableType.TABLE; - } - - @Override - public boolean isRolledUp(String column) { - return false; - } - - @Override - public boolean rolledUpColumnValidInsideAgg( - String column, - SqlCall call, - @Nullable SqlNode parent, - @Nullable CalciteConnectionConfig config) { - return false; - } - } - - @Override - protected Frameworks.ConfigBuilder config(CalciteAssert.SchemaSpec... schemaSpecs) { - final SchemaPlus rootSchema = Frameworks.createRootSchema(true); - final SchemaPlus schema = CalciteAssert.addSchema(rootSchema, schemaSpecs); - schema.add("DEPT", new TableWithArray()); - return Frameworks.newConfigBuilder() - .parserConfig(SqlParser.Config.DEFAULT) - .defaultSchema(schema) - .traitDefs((List) null) - .programs(Programs.heuristicJoinOrder(Programs.RULE_SET, true, 2)); - } - - @Test - public void testMvExpandBasic() { - String ppl = "source=DEPT | mvexpand EMPNOS"; - RelNode root = getRelNode(ppl); - String expectedLogical = - "LogicalProject(DEPTNO=[$0], TAGS=[$2], EMPNOS=[$3])\n" - + " LogicalCorrelate(correlation=[$cor0], joinType=[inner], requiredColumns=[{1}])\n" - + " LogicalTableScan(table=[[scott, DEPT]])\n" - + " Uncollect\n" - + " LogicalProject(EMPNOS=[$cor0.EMPNOS])\n" - + " LogicalValues(tuples=[[{ 0 }]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT `$cor0`.`DEPTNO`, `$cor0`.`TAGS`, `t00`.`EMPNOS`\n" - + "FROM `scott`.`DEPT` `$cor0`,\n" - + "LATERAL UNNEST((SELECT `$cor0`.`EMPNOS`\n" - + "FROM (VALUES (0)) `t` (`ZERO`))) `t00` (`EMPNOS`)"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testMvExpandWithLimitParameter() { - String ppl = "source=DEPT | mvexpand EMPNOS limit=2"; - RelNode root = getRelNode(ppl); - - assertContains(root, "LogicalCorrelate"); - assertContains(root, "Uncollect"); - assertAnyContains(root, "fetch=", "LIMIT", "RowNumber", "Window"); - - String expectedSparkSql = - "SELECT `$cor0`.`DEPTNO`, `$cor0`.`TAGS`, `t1`.`EMPNOS`\n" - + "FROM `scott`.`DEPT` `$cor0`,\n" - + "LATERAL (SELECT `EMPNOS`\n" - + "FROM UNNEST((SELECT `$cor0`.`EMPNOS`\n" - + "FROM (VALUES (0)) `t` (`ZERO`))) `t0` (`EMPNOS`)\n" - + "LIMIT 2) `t1`"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testMvExpandProjectNested() { - String ppl = "source=DEPT | mvexpand EMPNOS | fields DEPTNO, EMPNOS"; - RelNode root = getRelNode(ppl); - - assertContains(root, "LogicalCorrelate"); - assertContains(root, "Uncollect"); - assertContains(root, "LogicalProject"); - - String expectedSparkSql = - "SELECT `$cor0`.`DEPTNO`, `t00`.`EMPNOS`\n" - + "FROM `scott`.`DEPT` `$cor0`,\n" - + "LATERAL UNNEST((SELECT `$cor0`.`EMPNOS`\n" - + "FROM (VALUES (0)) `t` (`ZERO`))) `t00` (`EMPNOS`)"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testMvExpandEmptyOrNullArray() { - RelNode root = getRelNode("source=DEPT | where isnull(EMPNOS) | mvexpand EMPNOS"); - assertContains(root, "LogicalCorrelate"); - assertContains(root, "Uncollect"); - } - - @Test - public void testMvExpandWithDuplicates() { - RelNode root = getRelNode("source=DEPT | where DEPTNO in (10, 10, 20) | mvexpand EMPNOS"); - assertContains(root, "LogicalCorrelate"); - assertContains(root, "Uncollect"); - } - - @Test - public void testMvExpandLargeArray() { - RelNode root = getRelNode("source=DEPT | where DEPTNO = 999 | mvexpand EMPNOS"); - assertContains(root, "LogicalCorrelate"); - assertContains(root, "Uncollect"); - } - - @Test - public void testMvExpandPrimitiveArray() { - RelNode root = getRelNode("source=DEPT | mvexpand EMPNOS"); - assertContains(root, "LogicalCorrelate"); - assertContains(root, "Uncollect"); - } - - @Test - public void testMvExpandInvalidLimitZero() { - String ppl = "source=DEPT | mvexpand EMPNOS limit=0"; - Exception ex = Assert.assertThrows(Exception.class, () -> getRelNode(ppl)); - String msg = String.valueOf(ex.getMessage()); - Assert.assertTrue( - "Expected error message for limit=0. Actual: " + msg, - msg.toLowerCase().contains("limit") || msg.toLowerCase().contains("positive")); - } - - @Test - public void testMvExpandInvalidLimitNegative() { - String ppl = "source=DEPT | mvexpand EMPNOS limit=-1"; - Exception ex = Assert.assertThrows(Exception.class, () -> getRelNode(ppl)); - String msg = String.valueOf(ex.getMessage()); - Assert.assertTrue( - "Expected parsing error for negative limit. Actual: " + msg, - msg.toLowerCase().contains("extraneous") - || msg.toLowerCase().contains("unexpected") - || msg.toLowerCase().contains("expecting")); - } - - @Test - public void testMvExpandNonArrayField() { - String ppl = "source=DEPT | mvexpand DEPTNO"; - RelNode root = getRelNode(ppl); - - Assert.assertNotNull("Query should produce a valid plan", root); - - String plan = root.explain(); - Assert.assertTrue( - "Plan should contain LogicalTableScan", - plan.contains("LogicalTableScan") || plan.contains("LogicalProject")); - - Assert.assertFalse( - "Non-array field should not generate Uncollect operation", plan.contains("Uncollect")); - } - - @Test - public void testMvExpandMultisetField() { - // Test that MULTISET types are handled the same as ARRAY types - // This verifies the fix for the MULTISET handling issue identified in code review - String ppl = "source=DEPT | mvexpand TAGS"; - RelNode root = getRelNode(ppl); - - // MULTISET fields should generate the same plan structure as ARRAY fields - assertContains(root, "LogicalCorrelate"); - assertContains(root, "Uncollect"); - - String expectedSparkSql = - "SELECT `$cor0`.`DEPTNO`, `$cor0`.`EMPNOS`, `t00`.`TAGS`\n" - + "FROM `scott`.`DEPT` `$cor0`,\n" - + "LATERAL UNNEST((SELECT `$cor0`.`TAGS`\n" - + "FROM (VALUES (0)) `t` (`ZERO`))) `t00` (`TAGS`)"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testMvExpandMultisetWithLimit() { - // Test that MULTISET types work correctly with limit parameter - String ppl = "source=DEPT | mvexpand TAGS limit=3"; - RelNode root = getRelNode(ppl); - - assertContains(root, "LogicalCorrelate"); - assertContains(root, "Uncollect"); - assertAnyContains(root, "fetch=", "LIMIT", "RowNumber", "Window"); - - String expectedSparkSql = - "SELECT `$cor0`.`DEPTNO`, `$cor0`.`EMPNOS`, `t1`.`TAGS`\n" - + "FROM `scott`.`DEPT` `$cor0`,\n" - + "LATERAL (SELECT `TAGS`\n" - + "FROM UNNEST((SELECT `$cor0`.`TAGS`\n" - + "FROM (VALUES (0)) `t` (`ZERO`))) `t0` (`TAGS`)\n" - + "LIMIT 3) `t1`"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - private static void assertContains(RelNode root, String token) { - String plan = root.explain(); - Assert.assertTrue( - "Expected plan to contain [" + token + "] but got:\n" + plan, plan.contains(token)); - } - - private static void assertAnyContains(RelNode root, String... tokens) { - String plan = root.explain(); - for (String token : tokens) { - if (plan.contains(token)) { - return; - } - } - Assert.fail( - "Expected plan to contain one of " + Arrays.toString(tokens) + " but got:\n" + plan); - } -} diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLNoMvTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLNoMvTest.java deleted file mode 100644 index 5d7669d20a1..00000000000 --- a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLNoMvTest.java +++ /dev/null @@ -1,481 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.ppl.calcite; - -import static org.junit.Assert.assertThrows; - -import org.apache.calcite.rel.RelNode; -import org.apache.calcite.rel.rel2sql.RelToSqlConverter; -import org.apache.calcite.rel.rel2sql.SqlImplementor; -import org.apache.calcite.sql.SqlNode; -import org.apache.calcite.test.CalciteAssert; -import org.junit.Test; - -public class CalcitePPLNoMvTest extends CalcitePPLAbstractTest { - - private static final String LS = System.lineSeparator(); - - public CalcitePPLNoMvTest() { - super(CalciteAssert.SchemaSpec.SCOTT_WITH_TEMPORAL); - } - - /** - * Override to avoid normalizing the '\n' delimiter inside ARRAY_JOIN. The base class's - * normalization replaces ALL \n with System.lineSeparator(), which incorrectly changes the - * delimiter from '\n' to '\r\n' on Windows. The delimiter should always be '\n' regardless of - * platform - it's a data value, not a line separator. - */ - @Override - public void verifyPPLToSparkSQL(RelNode rel, String expected) { - // Don't normalize - expect strings are written with explicit System.lineSeparator() - SqlImplementor.Result result = getConverter().visitRoot(rel); - final SqlNode sqlNode = result.asStatement(); - final String sql = sqlNode.toSqlString(OpenSearchSparkSqlDialect.DEFAULT).getSql(); - org.hamcrest.MatcherAssert.assertThat(sql, org.hamcrest.CoreMatchers.is(expected)); - } - - // Helper to access converter from parent - private RelToSqlConverter getConverter() { - return new RelToSqlConverter(OpenSearchSparkSqlDialect.DEFAULT); - } - - @Test - public void testNoMvBasic() { - String ppl = - "source=EMP | eval arr = array('web', 'production', 'east') | nomv arr | head 1 | fields" - + " arr"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(arr=[$8])\n" - + " LogicalSort(fetch=[1])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7]," - + " arr=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array('web':VARCHAR, 'production':VARCHAR," - + " 'east':VARCHAR)), '\n" - + "'), '':VARCHAR)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY('web', 'production', 'east')), '\n" - + "'), '') `arr`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "LIMIT 1"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testNoMvMultipleDocuments() { - String ppl = - "source=EMP | eval arr = array('web', 'production') | nomv arr | head 2 | fields" - + " EMPNO, arr"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(EMPNO=[$0], arr=[$8])\n" - + " LogicalSort(fetch=[2])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7]," - + " arr=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array('web':VARCHAR, 'production':VARCHAR))," - + " '\n" - + "'), '':VARCHAR)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT `EMPNO`, COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY('web', 'production')), '\n" - + "'), '') `arr`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "LIMIT 2"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testNoMvWithMultipleFields() { - String ppl = - "source=EMP | eval arr1 = array('a', 'b'), arr2 = array('x', 'y') | nomv arr1 | nomv arr2 |" - + " head 1 | fields arr1, arr2"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(arr1=[$8], arr2=[$9])\n" - + " LogicalSort(fetch=[1])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7]," - + " arr1=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array('a', 'b')), '\n" - + "'), '':VARCHAR)], arr2=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array('x', 'y')), '\n" - + "'), '':VARCHAR)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY('a', 'b')), '\n" - + "'), '') `arr1`, COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY('x', 'y')), '\n" - + "'), '') `arr2`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "LIMIT 1"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testNoMvWithArrayFromFields() { - String ppl = - "source=EMP | eval tags = array(ENAME, JOB) | nomv tags | head 1 | fields EMPNO, tags"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(EMPNO=[$0], tags=[$8])\n" - + " LogicalSort(fetch=[1])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7], tags=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array($1," - + " $2)), '\n" - + "'), '':VARCHAR)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT `EMPNO`, COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY(`ENAME`, `JOB`)), '\n" - + "'), '') `tags`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "LIMIT 1"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testNoMvInPipeline() { - String ppl = - "source=EMP | where DEPTNO = 10 | eval names = array(ENAME, JOB) | nomv names | head 1 |" - + " fields EMPNO, names"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(EMPNO=[$0], names=[$8])\n" - + " LogicalSort(fetch=[1])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7]," - + " names=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array($1, $2)), '\n" - + "'), '':VARCHAR)])\n" - + " LogicalFilter(condition=[=($7, 10)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT `EMPNO`, COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY(`ENAME`, `JOB`)), '\n" - + "'), '') `names`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "WHERE `DEPTNO` = 10" - + LS - + "LIMIT 1"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testNoMvNonExistentField() { - String ppl = "source=EMP | eval arr = array('a', 'b') | nomv does_not_exist | head 1"; - - Exception ex = assertThrows(Exception.class, () -> getRelNode(ppl)); - - String msg = String.valueOf(ex.getMessage()); - org.junit.Assert.assertTrue( - "Expected error message to mention missing field or type error. Actual: " + msg, - msg.toLowerCase().contains("does_not_exist") - || msg.toLowerCase().contains("field") - || msg.contains("ARRAY_COMPACT") - || msg.contains("ARRAY")); - } - - @Test - public void testNoMvScalarFieldError() { - String ppl = "source=EMP | nomv EMPNO | head 1"; - - Exception ex = assertThrows(Exception.class, () -> getRelNode(ppl)); - - String msg = String.valueOf(ex.getMessage()); - org.junit.Assert.assertTrue( - "Expected error for non-array field. Actual: " + msg, - msg.toLowerCase().contains("array") || msg.toLowerCase().contains("type")); - } - - @Test - public void testNoMvNonDirectFieldReferenceError() { - String ppl = "source=EMP | eval arr = array('a', 'b') | nomv upper(arr) | head 1"; - - Exception ex = assertThrows(Exception.class, () -> getRelNode(ppl)); - - String msg = String.valueOf(ex.getMessage()); - org.junit.Assert.assertTrue( - "Expected parser error for non-direct field reference. Actual: " + msg, - msg.contains("(") - || msg.toLowerCase().contains("syntax") - || msg.toLowerCase().contains("parse")); - } - - @Test - public void testNoMvWithNestedArray() { - String ppl = - "source=EMP | eval arr = array('a', 'b', 'c') | nomv arr | eval arr_len = length(arr) |" - + " head 1 | fields EMPNO, arr, arr_len"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(EMPNO=[$0], arr=[$8], arr_len=[$9])\n" - + " LogicalSort(fetch=[1])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7], arr=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array('a'," - + " 'b', 'c')), '\n" - + "'), '':VARCHAR)], arr_len=[CHAR_LENGTH(COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array('a'," - + " 'b', 'c')), '\n" - + "'), '':VARCHAR))])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT `EMPNO`, COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY('a', 'b', 'c')), '\n" - + "'), '') `arr`, CHAR_LENGTH(COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY('a', 'b', 'c'))," - + " '\n" - + "'), '')) `arr_len`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "LIMIT 1"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testNoMvWithConcatInArray() { - String ppl = - "source=EMP | eval full_name = concat(ENAME, ' - ', JOB), arr = array(full_name) | nomv" - + " arr | head 1 | fields EMPNO, arr"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(EMPNO=[$0], arr=[$9])\n" - + " LogicalSort(fetch=[1])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7], full_name=[CONCAT($1, ' - ':VARCHAR, $2)]," - + " arr=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array(CONCAT($1, ' - ':VARCHAR, $2))), '\n" - + "'), '':VARCHAR)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT `EMPNO`, COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY(CONCAT(`ENAME`, ' - ', `JOB`)))," - + " '\n" - + "'), '') `arr`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "LIMIT 1"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testNoMvSingleElementArray() { - String ppl = "source=EMP | eval arr = array('single') | nomv arr | head 1 | fields EMPNO, arr"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(EMPNO=[$0], arr=[$8])\n" - + " LogicalSort(fetch=[1])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7]," - + " arr=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array('single':VARCHAR)), '\n" - + "'), '':VARCHAR)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT `EMPNO`, COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY('single')), '\n'), '') `arr`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "LIMIT 1"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testNoMvEmptyArray() { - String ppl = "source=EMP | eval arr = array() | nomv arr | head 1 | fields EMPNO, arr"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(EMPNO=[$0], arr=[$8])\n" - + " LogicalSort(fetch=[1])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7], arr=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array())," - + " '\n" - + "'), '':VARCHAR)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT `EMPNO`, COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY()), '\n'), '') `arr`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "LIMIT 1"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testNoMvLargeArray() { - String ppl = - "source=EMP | eval arr = array('1', '2', '3', '4', '5', '6', '7', '8', '9', '10') | nomv" - + " arr | head 1 | fields arr"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(arr=[$8])\n" - + " LogicalSort(fetch=[1])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7], arr=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array('1'," - + " '2', '3', '4', '5', '6', '7', '8', '9', '10':VARCHAR)), '\n" - + "'), '':VARCHAR)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY('1', '2', '3', '4', '5', '6', '7', '8'," - + " '9', '10')), '\n" - + "'), '') `arr`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "LIMIT 1"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testNoMvChainedWithOtherOperations() { - String ppl = - "source=EMP | eval arr = array('a', 'b') | nomv arr | eval arr_upper = upper(arr) | head" - + " 1 | fields arr, arr_upper"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(arr=[$8], arr_upper=[$9])\n" - + " LogicalSort(fetch=[1])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7], arr=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array('a'," - + " 'b')), '\n" - + "'), '':VARCHAR)], arr_upper=[UPPER(COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array('a'," - + " 'b')), '\n" - + "'), '':VARCHAR))])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY('a', 'b')), '\n" - + "'), '') `arr`, UPPER(COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY('a', 'b')), '\n" - + "'), '')) `arr_upper`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "LIMIT 1"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testNoMvWithNullableField() { - String ppl = - "source=EMP | eval arr = array(ENAME, COMM) | nomv arr | head 1 | fields EMPNO, arr"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(EMPNO=[$0], arr=[$8])\n" - + " LogicalSort(fetch=[1])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7], arr=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array($1," - + " $6)), '\n" - + "'), '':VARCHAR)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT `EMPNO`, COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY(`ENAME`, `COMM`)), '\n" - + "'), '') `arr`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "LIMIT 1"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testNoMvWithMultipleNullableFields() { - String ppl = "source=EMP | eval arr = array(MGR, COMM) | nomv arr | head 1 | fields EMPNO, arr"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(EMPNO=[$0], arr=[$8])\n" - + " LogicalSort(fetch=[1])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7], arr=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array($3," - + " $6)), '\n" - + "'), '':VARCHAR)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT `EMPNO`, COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY(`MGR`, `COMM`)), '\n'), '') `arr`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "LIMIT 1"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } - - @Test - public void testNoMvWithMixedNullableAndNonNullableFields() { - String ppl = - "source=EMP | eval arr = array(ENAME, COMM, JOB) | nomv arr | head 1 | fields EMPNO, arr"; - - RelNode root = getRelNode(ppl); - - String expectedLogical = - "LogicalProject(EMPNO=[$0], arr=[$8])\n" - + " LogicalSort(fetch=[1])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4]," - + " SAL=[$5], COMM=[$6], DEPTNO=[$7], arr=[COALESCE(ARRAY_JOIN(ARRAY_COMPACT(array($1," - + " $6, $2)), '\n" - + "'), '':VARCHAR)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n"; - verifyLogical(root, expectedLogical); - - String expectedSparkSql = - "SELECT `EMPNO`, COALESCE(ARRAY_JOIN(ARRAY_COMPACT(ARRAY(`ENAME`, `COMM`, `JOB`)), '\n" - + "'), '') `arr`" - + LS - + "FROM `scott`.`EMP`" - + LS - + "LIMIT 1"; - verifyPPLToSparkSQL(root, expectedSparkSql); - } -} diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLSpathTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLSpathTest.java index 9967b10543e..57b11d83150 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLSpathTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLSpathTest.java @@ -5,6 +5,7 @@ package org.opensearch.sql.ppl.calcite; +import org.apache.calcite.rel.RelNode; import org.apache.calcite.test.CalciteAssert; import org.junit.Test; @@ -15,133 +16,33 @@ public CalcitePPLSpathTest() { } @Test - public void testSpathPathMode() { - withPPLQuery("source=EMP | spath src.path input=ENAME") - .expectLogical( - "LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5]," - + " COMM=[$6], DEPTNO=[$7], src.path=[JSON_EXTRACT($1, 'src.path':VARCHAR)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n") - .expectSparkSQL( - "SELECT `EMPNO`, `ENAME`, `JOB`, `MGR`, `HIREDATE`, `SAL`, `COMM`, `DEPTNO`," - + " JSON_EXTRACT(`ENAME`, 'src.path') `src.path`\n" - + "FROM `scott`.`EMP`"); - } - - @Test - public void testSpathPathModeWithOutput() { - withPPLQuery("source=EMP | spath src.path input=ENAME output=custom | fields custom") - .expectLogical( - "LogicalProject(custom=[JSON_EXTRACT($1, 'src.path':VARCHAR)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n") - .expectSparkSQL( - "SELECT JSON_EXTRACT(`ENAME`, 'src.path') `custom`\n" + "FROM `scott`.`EMP`"); - } - - @Test - public void testSpathAutoExtractMode() { - withPPLQuery("source=EMP | spath input=ENAME") - .expectLogical( - "LogicalProject(EMPNO=[$0], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5]," - + " COMM=[$6], DEPTNO=[$7], ENAME=[JSON_EXTRACT_ALL($1)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n") - .expectSparkSQL( - "SELECT `EMPNO`, `JOB`, `MGR`, `HIREDATE`, `SAL`, `COMM`, `DEPTNO`," - + " JSON_EXTRACT_ALL(`ENAME`) `ENAME`\n" - + "FROM `scott`.`EMP`"); - } + public void testSimpleEval() { + String ppl = "source=EMP | spath src.path input=ENAME"; + RelNode root = getRelNode(ppl); + String expectedLogical = + "LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5]," + + " COMM=[$6], DEPTNO=[$7], src.path=[JSON_EXTRACT($1, 'src.path':VARCHAR)])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n"; + verifyLogical(root, expectedLogical); - @Test - public void testSpathAutoExtractModeWithOutput() { - withPPLQuery("source=EMP | spath input=ENAME output=result | fields result") - .expectLogical( - "LogicalProject(result=[JSON_EXTRACT_ALL($1)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n") - .expectSparkSQL("SELECT JSON_EXTRACT_ALL(`ENAME`) `result`\n" + "FROM `scott`.`EMP`"); + String expectedSparkSql = + "SELECT `EMPNO`, `ENAME`, `JOB`, `MGR`, `HIREDATE`, `SAL`, `COMM`, `DEPTNO`," + + " JSON_EXTRACT(`ENAME`, 'src.path') `src.path`\n" + + "FROM `scott`.`EMP`"; + verifyPPLToSparkSQL(root, expectedSparkSql); } @Test - public void testSpathAutoExtractModeWithEval() { - withPPLQuery( - "source=EMP | spath input=ENAME output=result" - + " | eval age = result.user.age + 1 | fields age") - .expectLogical( - "LogicalProject(age=[+(SAFE_CAST(ITEM(JSON_EXTRACT_ALL($1)," - + " 'user.age')), 1.0E0)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n") - .expectSparkSQL( - "SELECT TRY_CAST(JSON_EXTRACT_ALL(`ENAME`)['user.age']" - + " AS DOUBLE) + 1.0E0 `age`\n" - + "FROM `scott`.`EMP`"); - } + public void testEvalWithOutput() { + String ppl = "source=EMP | spath src.path input=ENAME output=custom | fields custom"; + RelNode root = getRelNode(ppl); + String expectedLogical = + "LogicalProject(custom=[JSON_EXTRACT($1, 'src.path':VARCHAR)])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n"; + verifyLogical(root, expectedLogical); - @Test - public void testSpathAutoExtractModeWithStats() { - withPPLQuery("source=EMP | spath input=ENAME output=result | stats count() by result.user.name") - .expectLogical( - "LogicalProject(count()=[$1], result.user.name=[$0])\n" - + " LogicalAggregate(group=[{0}], count()=[COUNT()])\n" - + " LogicalProject(result.user.name=[ITEM(JSON_EXTRACT_ALL($1)," - + " 'user.name')])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n") - .expectSparkSQL( - "SELECT COUNT(*) `count()`," - + " JSON_EXTRACT_ALL(`ENAME`)['user.name'] `result.user.name`\n" - + "FROM `scott`.`EMP`\n" - + "GROUP BY JSON_EXTRACT_ALL(`ENAME`)['user.name']"); - } - - @Test - public void testSpathAutoExtractModeWithWhere() { - withPPLQuery("source=EMP | spath input=ENAME output=result" + " | where result.active = 'true'") - .expectLogical( - "LogicalFilter(condition=[=(ITEM($8, 'active')," - + " 'true')])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3]," - + " HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7]," - + " result=[JSON_EXTRACT_ALL($1)])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n") - .expectSparkSQL( - "SELECT *\n" - + "FROM (SELECT `EMPNO`, `ENAME`, `JOB`, `MGR`, `HIREDATE`," - + " `SAL`, `COMM`, `DEPTNO`, JSON_EXTRACT_ALL(`ENAME`) `result`\n" - + "FROM `scott`.`EMP`) `t`\n" - + "WHERE `result`['active'] = 'true'"); - } - - @Test - public void testSpathAutoExtractModeWithFields() { - withPPLQuery( - "source=EMP | spath input=ENAME output=result" - + " | fields result.user.name, result.user.age") - .expectLogical( - "LogicalProject(result.user.name=[ITEM(JSON_EXTRACT_ALL($1), 'user.name')]," - + " result.user.age=[ITEM(JSON_EXTRACT_ALL($1), 'user.age')])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n") - .expectSparkSQL( - "SELECT JSON_EXTRACT_ALL(`ENAME`)['user.name'] `result.user.name`," - + " JSON_EXTRACT_ALL(`ENAME`)['user.age'] `result.user.age`\n" - + "FROM `scott`.`EMP`"); - } - - @Test - public void testSpathAutoExtractModeWithSort() { - withPPLQuery("source=EMP | spath input=ENAME output=result" + " | sort result.user.name") - .expectLogical( - "LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3]," - + " HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], result=[$8])\n" - + " LogicalSort(sort0=[$9], dir0=[ASC-nulls-first])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3]," - + " HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7]," - + " result=[JSON_EXTRACT_ALL($1)]," - + " $f9=[ITEM(JSON_EXTRACT_ALL($1), 'user.name')])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n") - .expectSparkSQL( - "SELECT `EMPNO`, `ENAME`, `JOB`, `MGR`, `HIREDATE`," - + " `SAL`, `COMM`, `DEPTNO`, `result`\n" - + "FROM (SELECT `EMPNO`, `ENAME`, `JOB`, `MGR`, `HIREDATE`," - + " `SAL`, `COMM`, `DEPTNO`, JSON_EXTRACT_ALL(`ENAME`) `result`," - + " JSON_EXTRACT_ALL(`ENAME`)['user.name'] `$f9`\n" - + "FROM `scott`.`EMP`\n" - + "ORDER BY 10) `t0`"); + String expectedSparkSql = + "SELECT JSON_EXTRACT(`ENAME`, 'src.path') `custom`\n" + "FROM `scott`.`EMP`"; + verifyPPLToSparkSQL(root, expectedSparkSql); } } diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java index 3c736e0cc13..f7cadaaf57d 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java @@ -938,16 +938,6 @@ public void testSpathWithNoPathKeyword() { "source=t | spath input=f simple.nested", spath(relation("t"), "f", null, "simple.nested")); } - @Test - public void testSpathWithNoPath() { - assertEqual("source=t | spath input=f", spath(relation("t"), "f", null, null)); - } - - @Test - public void testSpathWithNoPathButOutput() { - assertEqual("source=t | spath input=f output=o", spath(relation("t"), "f", "o", null)); - } - @Test public void testKmeansCommand() { assertEqual( diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/utils/PPLQueryDataAnonymizerTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/utils/PPLQueryDataAnonymizerTest.java index a20f1bf1b42..0398d30bf17 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/utils/PPLQueryDataAnonymizerTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/utils/PPLQueryDataAnonymizerTest.java @@ -1077,13 +1077,6 @@ public void testSpath() { "search source=t | spath input=json_attr output=out path=foo.bar | fields id, out")); } - @Test - public void testSpathNoPath() { - assertEquals( - "source=table | spath input=identifier", - anonymize("search source=t | spath input=json_attr")); - } - @Test public void testMvfind() { assertEquals( @@ -1105,21 +1098,4 @@ public void testMvcombineCommandWithDelim() { "source=table | mvcombine delim=*** identifier", anonymize("source=t | mvcombine age delim=','")); } - - @Test - public void testNoMvCommand() { - assertEquals("source=table | nomv identifier", anonymize("source=t | nomv firstname")); - } - - @Test - public void testMvexpandCommand() { - assertEquals("source=table | mvexpand identifier", anonymize("source=t | mvexpand skills")); - } - - @Test - public void testMvexpandCommandWithLimit() { - assertEquals( - "source=table | mvexpand identifier limit=***", - anonymize("source=t | mvexpand skills limit=5")); - } } diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/utils/SPathRewriteTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/utils/SPathRewriteTest.java index 0bc9357278d..73d282d1f64 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/utils/SPathRewriteTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/utils/SPathRewriteTest.java @@ -55,6 +55,11 @@ public void testSpathMissingInputArgumentHandling() { plan("source = t | spath path=a output=a"); } + @Test(expected = IllegalArgumentException.class) + public void testSpathMissingPathArgumentHandling() { + plan("source = t | spath input=a output=a"); + } + @Test public void testSpathArgumentDeshuffle() { assertEquals(plan("source = t | spath path=a input=a"), plan("source = t | spath input=a a")); @@ -76,20 +81,4 @@ public void testSpathEscapedSpaces() { assertEquals(ev, sp.rewriteAsEval()); } - - @Test - public void testSpathAutoExtractMode() { - SPath sp = (SPath) plan("source = t | spath input=a"); - assertEquals( - eval(relation("t"), let(field("a"), function("json_extract_all", field("a")))), - sp.rewriteAsEval()); - } - - @Test - public void testSpathAutoExtractModeWithOutput() { - SPath sp = (SPath) plan("source = t | spath input=a output=o"); - assertEquals( - eval(relation("t"), let(field("o"), function("json_extract_all", field("a")))), - sp.rewriteAsEval()); - } } diff --git a/scripts/build.sh b/scripts/build.sh index 336d1ebb62b..4c4aaf128ee 100755 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -68,7 +68,7 @@ fi mkdir -p $OUTPUT -./gradlew assemble --no-daemon --refresh-dependencies -DskipTests=true -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER -Pcrypto.standard=FIPS-140-3 +./gradlew assemble --no-daemon --refresh-dependencies -DskipTests=true -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER zipPath=$(find . -path \*build/distributions/*.zip) distributions="$(dirname "${zipPath}")" @@ -77,7 +77,7 @@ echo "COPY ${distributions}/*.zip" mkdir -p $OUTPUT/plugins cp ${distributions}/*.zip ./$OUTPUT/plugins -./gradlew publishToMavenLocal -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER -Pcrypto.standard=FIPS-140-3 -./gradlew publishPluginZipPublicationToZipStagingRepository -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER -Pcrypto.standard=FIPS-140-3 +./gradlew publishToMavenLocal -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER +./gradlew publishPluginZipPublicationToZipStagingRepository -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER mkdir -p $OUTPUT/maven/org/opensearch cp -r ./build/local-staging-repo/org/opensearch/. $OUTPUT/maven/org/opensearch diff --git a/sql/build.gradle b/sql/build.gradle index 8c551d7cbd3..8391d1538d9 100644 --- a/sql/build.gradle +++ b/sql/build.gradle @@ -43,9 +43,9 @@ configurations { } dependencies { - antlr "org.antlr:antlr4:4.13.2" + antlr "org.antlr:antlr4:4.7.1" - implementation "org.antlr:antlr4-runtime:4.13.2" + implementation "org.antlr:antlr4-runtime:4.7.1" implementation group: 'com.google.guava', name: 'guava', version: "${guava_version}" implementation group: 'org.json', name: 'json', version:'20231013' implementation project(':common')