From 5b6d9dea102e4b41da7dc09a296d3f81c4040f60 Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Wed, 30 Sep 2026 12:35:18 -0700 Subject: [PATCH 1/8] Read a flat_object field in PPL A flat_object field is invisible to PPL: parseMapping skips the type, so describe does not list it and any reference to it fails with "field not found" (#1604, #3067). The field is now registered and presented as a struct keyed by the dotted path of each leaf, read from _source. A flat_object gives its leaves no mapping of their own -- the index files every leaf value as a keyword term with the path folded in -- so a leaf has no type either: every value reads as text, and a path holding more than one value reads as their JSON. That is the rule spath already applies to a JSON string, and typeof on a leaf reports string. {"a": {"b": 1}} -> a.b: "1" {"a.b": 1} -> a.b: "1" {"a": [1, 2]} -> a: "[\"1\",\"2\"]" [{"a": 1}, {"a": 2}] -> a: "[\"1\",\"2\"]" {"nested": [{"k": 1}, {"k": 2}]} -> nested.k: "[\"1\",\"2\"]" {"a": {"b": 1}, "a.b": 2} -> a.b: "[\"1\",\"2\"]" Every way of writing a path reaches the same leaf, because the index files one term either way, and a path written more than once holds every value rather than the last -- the index answers a lookup for any of them, so dropping one would make a projection disagree with a filter. What a query may do with a leaf is read it, and cast it: source=logs | fields attributes source=logs | fields attributes.duration_ms source=logs | eval ms = cast(attributes.duration_ms as double) A cast is reading: a leaf is text, so converting it is the parse a caller would do anyway and the value comes from _source either way. It is how a query asks for a leaf as a number, which is what the field type leaves to the caller. Everything else that touches a leaf is rejected while the plan is built, by FlatObjectScopeValidator, with a message that names the field as the query writes it and a suggestion linking the field type's documentation: filtering by it, aggregating it, grouping or sorting by it, computing a value from it, expanding it. None of this is a PPL restriction -- the limitations are the field type's own, and answering any of it would mean opening every record, the cost this type exists to avoid. Cannot sort by attributes.duration_ms code UNSUPPORTED_OPERATION suggestion A flat_object field can only be read. Its leaves have no mapping of their own, so nothing else can be answered without opening every record. See https://docs.opensearch.org/latest/mappings/ supported-field-types/flat-object/ Filtering a leaf is the next step and is left to a follow-up, which adds the index lookups the field type does support and relaxes this check to admit them. The validator runs from CalciteRelNodeVisitor.analyze, so the unified query pipeline -- which calls the visitor directly rather than through QueryService -- is held to the same rules. It finds the field by its mapping type rather than by the type it takes in the plan, because a map of text to text is also what spath produces for an extracted document; and it tracks a column a projection derived from a leaf, so a command that sorts or groups by a leaf, projecting it into a column of its own first, is caught too. Two smaller changes come with it. expand over anything that is not a column of the index now raises the error mvexpand already raises, rather than the ClassCastException the cast threw. And RexStandardizer refuses to build a script over a flat_object field: it has no usable doc values, so a script could only read _source, and the field type does not support Painless for retrieving subfield values either. Signed-off-by: Jialiang Liang --- .../sql/calcite/CalciteRelNodeVisitor.java | 26 +- .../sql/calcite/FlatObjectScopeValidator.java | 319 ++++++++++++++++++ .../calcite/utils/OpenSearchTypeFactory.java | 9 + .../calcite/FlatObjectScopeValidatorTest.java | 196 +++++++++++ docs/user/general/datatypes.rst | 2 + docs/user/ppl/general/datatypes.md | 19 ++ .../rest-api-spec/test/issues/1604.yml | 160 +++++++++ .../data/type/OpenSearchDataType.java | 3 + .../data/type/OpenSearchFlatObjectType.java | 43 +++ .../sql/opensearch/data/utils/Content.java | 3 + .../opensearch/data/utils/ObjectContent.java | 5 + .../data/utils/OpenSearchJsonContent.java | 5 + .../data/value/FlatObjectValues.java | 164 +++++++++ .../value/OpenSearchExprValueFactory.java | 8 +- .../storage/serde/RexStandardizer.java | 9 + .../data/type/OpenSearchDataTypeTest.java | 12 + .../type/OpenSearchFlatObjectTypeTest.java | 66 ++++ .../data/value/FlatObjectValuesTest.java | 117 +++++++ 18 files changed, 1163 insertions(+), 3 deletions(-) create mode 100644 core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java create mode 100644 core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java create mode 100644 integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/1604.yml create mode 100644 opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchFlatObjectType.java create mode 100644 opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/FlatObjectValues.java create mode 100644 opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchFlatObjectTypeTest.java create mode 100644 opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/FlatObjectValuesTest.java 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 e66e3319b57..dbbe07b1c5e 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java @@ -190,6 +190,7 @@ import org.opensearch.sql.calcite.utils.WildcardUtils; import org.opensearch.sql.common.error.ErrorCode; import org.opensearch.sql.common.error.ErrorReport; +import org.opensearch.sql.common.error.QueryProcessingStage; import org.opensearch.sql.common.patterns.PatternUtils; import org.opensearch.sql.common.utils.StringUtils; import org.opensearch.sql.data.type.ExprCoreType; @@ -236,7 +237,11 @@ public CalciteRelNodeVisitor(DataSourceService dataSourceService) { } public RelNode analyze(UnresolvedPlan unresolved, CalcitePlanContext context) { - return unresolved.accept(this, context); + RelNode plan = unresolved.accept(this, context); + // Here rather than in QueryService.analyze, so the unified query pipeline -- which calls this + // method directly -- is held to the same rules. + FlatObjectScopeValidator.validate(plan); + return plan; } @Override @@ -4456,7 +4461,24 @@ public RelNode visitExpand(Expand expand, CalcitePlanContext context) { // 2. Get the field to expand and an optional alias. Field arrayField = expand.getField(); - RexInputRef arrayFieldRex = (RexInputRef) rexVisitor.analyze(arrayField, context); + RexNode resolved = rexVisitor.analyze(arrayField, context); + if (!(resolved instanceof RexInputRef arrayFieldRex)) { + // Anything that is not a column of the input -- a leaf of an object or a flat_object field, + // an expression -- has no column to correlate the expansion with. Raised the way the + // mvexpand check below does, rather than as the ClassCastException the cast used to throw. + String name = StringUtils.unquoteIdentifier(arrayField.getField().toString()); + throw ErrorReport.wrap( + new IllegalArgumentException(StringUtils.format("Cannot expand [%s]", name))) + .code(ErrorCode.UNSUPPORTED_OPERATION) + .stage(QueryProcessingStage.ANALYZING) + .location("while resolving the field for expand") + .context("command", "expand") + .context("field", name) + .suggestion( + "expand takes a field of the index, not a value computed from one. Expand the field" + + " itself, or project the value first.") + .build(); + } String alias = expand.getAlias(); buildExpandRelNode(arrayFieldRex, arrayField.getField().toString(), alias, null, context); diff --git a/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java b/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java new file mode 100644 index 00000000000..17799e79fc1 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java @@ -0,0 +1,319 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.calcite; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import org.apache.calcite.plan.RelOptTable; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.RelVisitor; +import org.apache.calcite.rel.core.Aggregate; +import org.apache.calcite.rel.core.Filter; +import org.apache.calcite.rel.core.Project; +import org.apache.calcite.rel.core.Sort; +import org.apache.calcite.rel.type.RelDataTypeField; +import org.apache.calcite.rex.RexCall; +import org.apache.calcite.rex.RexInputRef; +import org.apache.calcite.rex.RexLiteral; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexShuttle; +import org.apache.calcite.rex.RexVisitorImpl; +import org.apache.calcite.sql.SqlKind; +import org.checkerframework.checker.nullness.qual.Nullable; +import org.opensearch.sql.calcite.plan.AbstractOpenSearchTable; +import org.opensearch.sql.calcite.utils.PlanUtils; +import org.opensearch.sql.common.error.ErrorCode; +import org.opensearch.sql.common.error.ErrorReport; +import org.opensearch.sql.common.error.QueryProcessingStage; +import org.opensearch.sql.common.utils.StringUtils; + +/** + * Keeps a query to what a {@code flat_object} field can answer. + * + *

A flat_object gives its leaves no mapping of their own: the index files every leaf value as a + * keyword term with the path folded in, and the engine reads the field from {@code _source}. So a + * leaf can be read, and that is all PPL does with it today -- anything computed from a leaf would + * have to open every record, which is the cost this field type exists to avoid. Such a query is + * rejected here, while the plan is built, rather than answered slowly. + * + *

A leaf is {@code ITEM(, '')}. The columns are found by their mapping + * type rather than by the type they take in the plan, because that type -- a map of text to text -- + * is also what {@code spath} produces for an extracted document. A column that a projection derived + * from a leaf is tracked too, so that a command which sorts or groups by a leaf, projecting it into + * a column of its own first, is caught. + */ +public final class FlatObjectScopeValidator { + + private static final String DOC_URL = + "https://docs.opensearch.org/latest/mappings/supported-field-types/flat-object/"; + + private static final String SUGGESTION = + "A flat_object field can only be read. Its leaves have no mapping of their own, so nothing" + + " else can be answered without opening every record. See " + + DOC_URL; + + private FlatObjectScopeValidator() {} + + /** Throws if the plan does anything with a flat_object field other than read it. */ + public static void validate(RelNode plan) { + Set fields = flatObjectFields(plan); + if (!fields.isEmpty()) { + validate(plan, fields); + } + } + + /** The check itself, over a known set of flat_object column names. */ + static void validate(RelNode plan, Set fields) { + new RelVisitor() { + @Override + public void visit(RelNode node, int ordinal, @Nullable RelNode parent) { + super.visit(node, ordinal, parent); + check(node, fields); + } + }.go(plan); + } + + /** The plan's flat_object columns, by mapping type; empty when the query touches none. */ + private static Set flatObjectFields(RelNode plan) { + RelOptTable table = PlanUtils.findTable(plan); + AbstractOpenSearchTable osTable = + table == null ? null : table.unwrap(AbstractOpenSearchTable.class); + if (osTable == null) { + return Set.of(); + } + Set fields = new HashSet<>(); + osTable + .getFieldTypes() + .forEach( + (name, type) -> { + if ("flat_object".equalsIgnoreCase(type.legacyTypeName())) { + fields.add(name); + } + }); + return fields; + } + + private static void check(RelNode node, Set fields) { + List inputFields = inputFields(node); + Set leafColumns = leafColumns(node, fields); + if (node instanceof Project project) { + // Reading a leaf, or the whole field, is the one thing allowed. + for (RexNode expression : project.getProjects()) { + if (isRead(expression, inputFields, fields, leafColumns)) { + continue; + } + List leaves = leavesIn(expression, inputFields, fields, leafColumns); + if (!leaves.isEmpty()) { + throw unsupported("Cannot compute a value from %s", String.join(", ", leaves)); + } + } + return; + } + // Every other node: a leaf may pass through by position, but nothing the node does may use one. + List used = new ArrayList<>(); + node.accept( + new RexShuttle() { + @Override + public RexNode visitInputRef(RexInputRef ref) { + if (isLeafColumn(ref.getIndex(), inputFields, fields, leafColumns)) { + used.add(displayName(node, inputFields, ref.getIndex(), fields)); + } + return ref; + } + + @Override + public RexNode visitCall(RexCall call) { + // a leaf is named by its path, not by the field the ITEM reads from + if (isLeafRef(call, inputFields, fields)) { + used.add(leafName(call, inputFields)); + return call; + } + return super.visitCall(call); + } + }); + if (node instanceof Sort sort) { + sort.getCollation() + .getFieldCollations() + .forEach( + key -> { + if (isLeafColumn(key.getFieldIndex(), inputFields, fields, leafColumns)) { + used.add(displayName(node, inputFields, key.getFieldIndex(), fields)); + } + }); + } + if (node instanceof Aggregate aggregate) { + List positions = new ArrayList<>(aggregate.getGroupSet().asList()); + aggregate.getAggCallList().forEach(call -> positions.addAll(call.getArgList())); + positions.forEach( + position -> { + if (isLeafColumn(position, inputFields, fields, leafColumns)) { + used.add(displayName(node, inputFields, position, fields)); + } + }); + } + if (!used.isEmpty()) { + throw unsupported( + "Cannot %s by %s", nameOf(node), String.join(", ", new LinkedHashSet<>(used))); + } + } + + private static boolean isLeafColumn( + int index, List inputFields, Set fields, Set leaves) { + return leaves.contains(index) || isFlatObjectColumn(inputFields, index, fields); + } + + /** + * The output column indices of {@code node}'s inputs that came off a flat_object field: a leaf a + * projection read, or the field itself passed through. A projection renames what it produces, so + * a command one level up cannot recognize either by name. + */ + private static Set leafColumns(RelNode node, Set fields) { + Set leaves = new HashSet<>(); + int offset = 0; + for (RelNode input : node.getInputs()) { + if (input instanceof Project project) { + List below = inputFields(project); + List projects = project.getProjects(); + for (int i = 0; i < projects.size(); i++) { + RexNode expression = projects.get(i); + boolean fromField = + isLeafRef(expression, below, fields) + || (expression instanceof RexInputRef ref + && isFlatObjectColumn(below, ref.getIndex(), fields)); + if (fromField) { + leaves.add(offset + i); + } + } + } + offset += input.getRowType().getFieldCount(); + } + return leaves; + } + + /** + * Whether the expression only reads a leaf or the whole field. A cast of a leaf counts: a leaf is + * text, so converting it is the parse a caller would do anyway, the value comes from {@code + * _source} either way, and it is how a query asks for a leaf as a number. + */ + private static boolean isRead( + RexNode node, List inputFields, Set fields, Set leaves) { + if (node instanceof RexInputRef) { + return true; + } + RexNode read = node; + while (read instanceof RexCall cast + && (cast.getKind() == SqlKind.CAST || cast.getKind() == SqlKind.SAFE_CAST)) { + read = cast.getOperands().get(0); + } + return isLeafRef(read, inputFields, fields) + || (read instanceof RexInputRef ref && leaves.contains(ref.getIndex())); + } + + /** {@code ITEM(, '')}. */ + private static boolean isLeafRef( + RexNode node, List inputFields, Set fields) { + return node instanceof RexCall call + && call.getKind() == SqlKind.ITEM + && call.getOperands().get(0) instanceof RexInputRef ref + && isFlatObjectColumn(inputFields, ref.getIndex(), fields) + && call.getOperands().get(1) instanceof RexLiteral; + } + + private static boolean isFlatObjectColumn( + List inputFields, int index, Set fields) { + return index >= 0 + && index < inputFields.size() + && fields.contains(StringUtils.unquoteIdentifier(inputFields.get(index).getName())); + } + + /** The leaf references inside an expression, named as the query writes them. */ + private static List leavesIn( + RexNode node, List inputFields, Set fields, Set leaves) { + Set found = new LinkedHashSet<>(); + node.accept( + new RexVisitorImpl(true) { + @Override + public Void visitInputRef(RexInputRef ref) { + if (isLeafColumn(ref.getIndex(), inputFields, fields, leaves)) { + found.add(name(inputFields, ref.getIndex())); + } + return null; + } + + @Override + public Void visitCall(RexCall call) { + if (isLeafRef(call, inputFields, fields)) { + found.add(leafName(call, inputFields)); + return null; + } + return super.visitCall(call); + } + }); + return List.copyOf(found); + } + + private static List inputFields(RelNode node) { + List fields = new ArrayList<>(); + for (RelNode input : node.getInputs()) { + fields.addAll(input.getRowType().getFieldList()); + } + return fields; + } + + private static String name(List inputFields, int index) { + return StringUtils.unquoteIdentifier(inputFields.get(index).getName()); + } + + private static String leafName(RexCall item, List inputFields) { + return name(inputFields, ((RexInputRef) item.getOperands().get(0)).getIndex()) + + "." + + RexLiteral.stringValue((RexLiteral) item.getOperands().get(1)); + } + + /** + * The name to show for an input column. A command that sorts or groups by a leaf projects it into + * a column of its own first, and that column has a generated name ({@code $f2}); name the leaf, + * or the field, that projection reads instead, which is what the query says. + */ + private static String displayName( + RelNode node, List inputFields, int index, Set fields) { + if (!node.getInputs().isEmpty() && node.getInput(0) instanceof Project project) { + List below = inputFields(project); + RexNode expression = project.getProjects().get(index); + if (isLeafRef(expression, below, fields)) { + return leafName((RexCall) expression, below); + } + if (expression instanceof RexInputRef ref + && isFlatObjectColumn(below, ref.getIndex(), fields)) { + return name(below, ref.getIndex()); + } + } + return name(inputFields, index); + } + + /** The command as a query author names it, for the message. */ + private static String nameOf(RelNode node) { + return switch (node) { + case Sort ignored -> "sort"; + case Aggregate ignored -> "stats"; + case Filter ignored -> "filter"; + default -> node.getRelTypeName().toLowerCase(Locale.ROOT); + }; + } + + private static ErrorReport unsupported(String message, Object... args) { + return ErrorReport.wrap(new IllegalArgumentException(StringUtils.format(message, args))) + .code(ErrorCode.UNSUPPORTED_OPERATION) + .stage(QueryProcessingStage.ANALYZING) + .location("while checking what a flat_object field can answer") + .suggestion(SUGGESTION) + .build(); + } +} diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/OpenSearchTypeFactory.java b/core/src/main/java/org/opensearch/sql/calcite/utils/OpenSearchTypeFactory.java index 99fef793f5d..81ce881f0ce 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/utils/OpenSearchTypeFactory.java +++ b/core/src/main/java/org/opensearch/sql/calcite/utils/OpenSearchTypeFactory.java @@ -206,6 +206,15 @@ public static RelDataType convertExprTypeToRelDataType(ExprType fieldType, boole return TYPE_FACTORY.createSqlType(SqlTypeName.GEOMETRY, nullable); } else if (fieldType.legacyTypeName().equalsIgnoreCase("text")) { return TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR, nullable); + } else if (fieldType.legacyTypeName().equalsIgnoreCase("flat_object")) { + // Keys are the dotted leaf paths, values are text: a flat_object gives its leaves no type + // of their own, and the index holds each as a keyword term. This is what spath already + // does for a JSON string -- every value reads as text, and a path holding several values + // reads as their JSON. + return TYPE_FACTORY.createMapType( + TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR), + TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR, true), + nullable); } else if (fieldType.legacyTypeName().equalsIgnoreCase("ip")) { return TYPE_FACTORY.createUDT(ExprUDT.EXPR_IP, nullable); } else if (fieldType.getOriginalPath().isPresent()) { diff --git a/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java b/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java new file mode 100644 index 00000000000..1ec545f12fe --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java @@ -0,0 +1,196 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.calcite; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.TYPE_FACTORY; + +import com.google.common.collect.ImmutableList; +import java.math.BigDecimal; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import org.apache.calcite.plan.RelOptCluster; +import org.apache.calcite.plan.volcano.VolcanoPlanner; +import org.apache.calcite.rel.RelCollations; +import org.apache.calcite.rel.RelFieldCollation; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.core.AggregateCall; +import org.apache.calcite.rel.logical.LogicalAggregate; +import org.apache.calcite.rel.logical.LogicalFilter; +import org.apache.calcite.rel.logical.LogicalProject; +import org.apache.calcite.rel.logical.LogicalSort; +import org.apache.calcite.rel.logical.LogicalValues; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rex.RexBuilder; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.sql.fun.SqlStdOperatorTable; +import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.calcite.util.ImmutableBitSet; +import org.junit.jupiter.api.Test; +import org.opensearch.sql.common.error.ErrorCode; +import org.opensearch.sql.common.error.ErrorReport; + +/** + * What a query may do with a flat_object field: read a leaf, or the whole field. Everything else is + * rejected while the plan is built. + * + *

The plans here are built as the commands build them: the field is a column of type {@code + * MAP}, and a leaf of one is {@code ITEM(, '')}. + */ +class FlatObjectScopeValidatorTest { + + private final RexBuilder rexBuilder = new RexBuilder(TYPE_FACTORY); + private final RelOptCluster cluster = RelOptCluster.create(new VolcanoPlanner(), rexBuilder); + + /** A row of two columns: `service`, a keyword, and `attributes`, a flat_object field. */ + private final RelDataType rowType = + TYPE_FACTORY + .builder() + .add("service", TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR)) + .add( + "attributes", + TYPE_FACTORY.createMapType( + TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR), + TYPE_FACTORY.createTypeWithNullability( + TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR), true), + true)) + .build(); + + private RelNode scan() { + return LogicalValues.createEmpty(cluster, rowType); + } + + private RexNode field(int index) { + return rexBuilder.makeInputRef(rowType.getFieldList().get(index).getType(), index); + } + + /** {@code attributes.}, as the resolver builds it. */ + private RexNode leaf(String path) { + return rexBuilder.makeCall(SqlStdOperatorTable.ITEM, field(1), rexBuilder.makeLiteral(path)); + } + + private RelNode project(RexNode... expressions) { + List list = List.of(expressions); + List names = new ArrayList<>(); + for (int i = 0; i < list.size(); i++) { + names.add("c" + i); + } + return LogicalProject.create(scan(), ImmutableList.of(), list, names); + } + + /** The field is found by its mapping type in production; the check itself takes the names. */ + private static final Set FIELDS = Set.of("attributes"); + + private void allowed(RelNode plan) { + assertDoesNotThrow(() -> FlatObjectScopeValidator.validate(plan, FIELDS)); + } + + /** The message says what this query did; the rule and the doc link ride on the report. */ + private void rejected(RelNode plan, String what) { + ErrorReport ex = + assertThrows(ErrorReport.class, () -> FlatObjectScopeValidator.validate(plan, FIELDS)); + assertTrue(ex.getMessage().startsWith(what), ex.getMessage()); + assertEquals(ErrorCode.UNSUPPORTED_OPERATION, ex.getCode()); + assertTrue(ex.getSuggestion().contains("can only be read"), ex.getSuggestion()); + assertTrue( + ex.getSuggestion().contains("supported-field-types/flat-object/"), ex.getSuggestion()); + } + + // ---- reading + + @Test + void readingAFieldALeafOrAnotherColumn() { + allowed(project(field(1))); + allowed(project(leaf("namespace"))); + allowed(project(field(0), leaf("duration_ms"))); + } + + // A leaf is text, so converting it is the parse a caller would do anyway, and the value comes + // from _source either way -- this is how a query asks for a leaf as a number. + @Test + void castingALeafIsReading() { + RexNode asDouble = + rexBuilder.makeCast( + TYPE_FACTORY.createSqlType(SqlTypeName.DOUBLE), leaf("duration_ms"), true, true); + allowed(project(asDouble)); + } + + @Test + void aPlanThatDoesNotTouchTheFieldIsUntouched() { + allowed(project(field(0))); + allowed(LogicalSort.create(scan(), RelCollations.of(new RelFieldCollation(0)), null, null)); + } + + // ---- everything else + + @Test + void computingAValueFromALeafIsRejected() { + RexNode plusOne = + rexBuilder.makeCall( + SqlStdOperatorTable.PLUS, + rexBuilder.makeCast( + TYPE_FACTORY.createSqlType(SqlTypeName.DOUBLE), leaf("d"), true, true), + rexBuilder.makeExactLiteral(BigDecimal.ONE)); + rejected(project(plusOne), "Cannot compute a value from attributes.d"); + } + + @Test + void filteringByALeafIsRejected() { + RexNode equals = + rexBuilder.makeCall( + SqlStdOperatorTable.EQUALS, leaf("namespace"), rexBuilder.makeLiteral("prod")); + rejected(LogicalFilter.create(scan(), equals), "Cannot filter by attributes.namespace"); + } + + // A command that sorts or groups by a leaf projects it into a column of its own first, so the + // check has to follow that column back to the leaf it was read from. + @Test + void sortingByALeafIsRejected() { + RelNode projected = project(field(0), leaf("duration_ms")); + rejected( + LogicalSort.create(projected, RelCollations.of(new RelFieldCollation(1)), null, null), + "Cannot sort by attributes.duration_ms"); + rejected( + LogicalSort.create( + project(field(1)), RelCollations.of(new RelFieldCollation(0)), null, null), + "Cannot sort by attributes"); + } + + @Test + void groupingByALeafIsRejected() { + RelNode projected = project(field(0), leaf("status_code")); + rejected( + LogicalAggregate.create( + projected, ImmutableList.of(), ImmutableBitSet.of(1), null, ImmutableList.of()), + "Cannot stats by attributes.status_code"); + } + + @Test + void aggregatingALeafIsRejected() { + RelNode projected = project(field(0), leaf("duration_ms")); + AggregateCall count = + AggregateCall.create( + SqlStdOperatorTable.COUNT, + false, + false, + false, + ImmutableList.of(), + ImmutableList.of(1), + -1, + null, + RelCollations.EMPTY, + TYPE_FACTORY.createSqlType(SqlTypeName.BIGINT), + "c"); + rejected( + LogicalAggregate.create( + projected, ImmutableList.of(), ImmutableBitSet.of(), null, ImmutableList.of(count)), + "Cannot stats by attributes.duration_ms"); + } +} diff --git a/docs/user/general/datatypes.rst b/docs/user/general/datatypes.rst index adefd59397b..fcd052133ef 100644 --- a/docs/user/general/datatypes.rst +++ b/docs/user/general/datatypes.rst @@ -103,6 +103,8 @@ The table below list the mapping between OpenSearch Data Type, OpenSearch SQL Da +-----------------+---------------------+-----------+ | nested | array | STRUCT | +-----------------+---------------------+-----------+ +| flat_object | struct | STRUCT | ++-----------------+---------------------+-----------+ Notes: * Not all the OpenSearch SQL Type has correspond OpenSearch Type. e.g. data and time. To use function which required such data type, user should explicitly convert the data type. diff --git a/docs/user/ppl/general/datatypes.md b/docs/user/ppl/general/datatypes.md index b2f38418ea8..1cf6b7140f7 100644 --- a/docs/user/ppl/general/datatypes.md +++ b/docs/user/ppl/general/datatypes.md @@ -113,6 +113,25 @@ A string is a sequence of characters enclosed in either single or double quotes. ## Query Struct Data Types In PPL, the Struct Data Types corresponding to the [Object field type in OpenSearch](https://opensearch.org/docs/latest/field-types/supported-field-types/object/). The "." is used as the path selector when access the inner attribute of the struct data. + +A [flat_object field](https://docs.opensearch.org/latest/mappings/supported-field-types/flat-object/) is also presented as a struct, keyed by the dotted path of each leaf. A flat_object declares no sub-fields, so a leaf has no type of its own: every value reads as text, and a path holding more than one value reads as their JSON. This is the rule `spath` already applies to a JSON string, and `typeof` on a leaf reports `string`. + +Both a nested object and a literal dotted key reach the same leaf: `{"a": {"b": 1}}` and `{"a.b": 1}` each give the entry `a.b` with the value `"1"`. A path receives more than one value when it comes from an array, from several elements of an array of objects, or from both spellings of the same path appearing in one document; it then reads as a JSON array of those texts, in the order the document wrote them. + +What a query can do with a flat_object leaf is what the field type itself can do: PPL inherits the field type's [limitations](https://docs.opensearch.org/latest/mappings/supported-field-types/flat-object/#limitations). The index files every leaf value as a keyword term with the path folded in and gives the leaf no mapping, so a leaf can be read, but nothing can be computed from it without opening every record -- the cost this field type exists to avoid. + +Supported on a leaf: + +- reading it, or the whole field; +- casting it, which is how a query asks for a leaf as a number: `eval ms = cast(attributes.duration_ms as double)`. + +Not supported on a leaf. These are rejected when the query is planned, with an error that names the field and links to the field type's documentation: + +- filtering by it; +- aggregating it, or grouping by it; +- sorting by it; +- computing a value from it, such as a function or an arithmetic expression; +- expanding it. ### Example: People There are three fields in test index `people`: 1) deep nested object field `city`; 2) object field of array value `account`; 3) nested field `projects` diff --git a/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/1604.yml b/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/1604.yml new file mode 100644 index 00000000000..31cb720cffd --- /dev/null +++ b/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/1604.yml @@ -0,0 +1,160 @@ +# Issue: https://github.com/opensearch-project/sql/issues/1604 +# https://github.com/opensearch-project/sql/issues/3067 +# A flat_object field used to be dropped by OpenSearchDataType.parseMapping ("unknown type, skip +# it"), so it was absent from the schema and any reference failed as "field not found". +# +# The field is now a struct keyed by the dotted path of each leaf. A leaf has no type of its own in +# the index, so every value reads as text and a path holding several values reads as their JSON -- +# the same rule spath applies to a JSON string. +setup: + - do: + indices.create: + index: otel + body: + settings: + number_of_shards: 1 + number_of_replicas: 0 + mappings: + properties: + service: + type: keyword + attributes: + type: flat_object + + - do: + bulk: + refresh: true + body: + - '{"index": {"_index": "otel", "_id": "1"}}' + - '{"service": "checkout", "attributes": {"http.response.status_code": 500, "duration_ms": 12.5, "ok": true}}' + - '{"index": {"_index": "otel", "_id": "2"}}' + - '{"service": "payments", "attributes": {"http": {"response": {"status_code": 503}}, "duration_ms": "4"}}' + - '{"index": {"_index": "otel", "_id": "3"}}' + - '{"service": "tags", "attributes": {"tags": ["a", "b"], "mixed": [1, "x"], "nested": [{"k": 1}, {"k": 2}]}}' + - '{"index": {"_index": "otel", "_id": "4"}}' + - '{"service": "collide", "attributes": {"a": {"b": 1}, "a.b": 2}}' + + - do: + query.settings: + body: + transient: + plugins.calcite.enabled: true + plugins.calcite.fallback.allowed: false + +--- +teardown: + - do: + indices.delete: + index: otel + ignore_unavailable: true + - do: + query.settings: + body: + transient: + plugins.calcite.enabled: null + plugins.calcite.fallback.allowed: null + +--- +"describe lists the field as flat_object": + - skip: + features: [headers] + - do: + headers: {Content-Type: 'application/json'} + ppl: + body: + query: 'describe otel | fields COLUMN_NAME, TYPE_NAME' + - match: {"datarows": [["attributes", "flat_object"], ["service", "string"]]} + +--- +"the whole field reads as a map of dotted leaf path to text": + - skip: + features: [headers] + - do: + headers: {Content-Type: 'application/json'} + ppl: + body: + query: 'source=otel | where service = ''checkout'' | fields attributes' + # every value is text, and the dotted path is the key -- both spellings of a path reach the same one + - match: {"datarows": [[{"http.response.status_code": "500", "duration_ms": "12.5", "ok": "true"}]]} + +--- +"a leaf reads as text, under either spelling of its path": + - skip: + features: [headers] + - do: + headers: {Content-Type: 'application/json'} + ppl: + body: + query: 'source=otel | fields service, attributes.http.response.status_code, attributes.duration_ms | sort service' + - match: {"datarows": [["checkout", "500", "12.5"], ["collide", null, null], ["payments", "503", "4"], ["tags", null, null]]} + +--- +"a path holding several values reads as their JSON": + - skip: + features: [headers] + - do: + headers: {Content-Type: 'application/json'} + ppl: + body: + query: 'source=otel | where service = ''tags'' | fields attributes.tags, attributes.mixed, attributes.nested.k' + - match: {"datarows": [["[\"a\",\"b\"]", "[\"1\",\"x\"]", "[\"1\",\"2\"]"]]} + # both spellings of a path in one document: neither value is dropped + - do: + headers: {Content-Type: 'application/json'} + ppl: + body: + query: 'source=otel | where service = ''collide'' | fields attributes.a.b' + - match: {"datarows": [["[\"1\",\"2\"]"]]} + +--- +"a leaf can be cast, and typeof reports text": + - skip: + features: [headers] + - do: + headers: {Content-Type: 'application/json'} + ppl: + body: + query: 'source=otel | where service = ''payments'' | eval n = cast(attributes.duration_ms as double), t = typeof(attributes.duration_ms) | fields n, t' + - match: {"datarows": [[4.0, "STRING"]]} + +--- +"anything other than reading a leaf is rejected when the query is planned": + - skip: + features: [headers] + - do: + catch: bad_request + headers: {Content-Type: 'application/json'} + ppl: + body: + query: 'source=otel | sort attributes.duration_ms | fields service' + - match: {"$body": "/Cannot\\s+sort\\s+by\\s+attributes.duration_ms.*/"} + - match: {"$body": "/.*can\\s+only\\s+be\\s+read.*/"} + - match: {"$body": "/.*docs.opensearch.org\\/latest\\/mappings\\/supported-field-types\\/flat-object.*/"} + - do: + catch: bad_request + headers: {Content-Type: 'application/json'} + ppl: + body: + query: 'source=otel | stats count() by attributes.duration_ms' + - match: {"$body": "/Cannot\\s+stats\\s+by\\s+attributes.duration_ms.*/"} + - do: + catch: bad_request + headers: {Content-Type: 'application/json'} + ppl: + body: + query: 'source=otel | where attributes.duration_ms = ''4'' | fields service' + - match: {"$body": "/Cannot\\s+filter\\s+by\\s+attributes.duration_ms.*/"} + - do: + catch: bad_request + headers: {Content-Type: 'application/json'} + ppl: + body: + query: 'source=otel | eval d = attributes.duration_ms + 1 | fields d' + - match: {"$body": "/Cannot\\s+compute\\s+a\\s+value\\s+from\\s+attributes.duration_ms.*/"} + - do: + catch: bad_request + headers: {Content-Type: 'application/json'} + ppl: + body: + query: 'source=otel | expand attributes.tags | fields service' + - match: {"$body": "/Cannot\\s+expand\\s+.attributes.tags.*/"} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java index 98bb69b7fdf..f3fbbe8d2c6 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java @@ -39,6 +39,7 @@ public enum MappingType { DateNanos("date_nanos", ExprCoreType.TIMESTAMP), Object("object", ExprCoreType.STRUCT), Nested("nested", ExprCoreType.ARRAY), + FlatObject("flat_object", ExprCoreType.UNKNOWN), Byte("byte", ExprCoreType.BYTE), Short("short", ExprCoreType.SHORT), Integer("integer", ExprCoreType.INTEGER), @@ -170,6 +171,8 @@ public static OpenSearchDataType of(MappingType mappingType, Map return OpenSearchGeoPointType.of(); case Binary: return OpenSearchBinaryType.of(); + case FlatObject: + return OpenSearchFlatObjectType.of(); case Date: case DateNanos: // Default date formatter is used when "" is passed as the second parameter diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchFlatObjectType.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchFlatObjectType.java new file mode 100644 index 00000000000..ad718fd79cd --- /dev/null +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchFlatObjectType.java @@ -0,0 +1,43 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.opensearch.data.type; + +import static org.opensearch.sql.data.type.ExprCoreType.UNKNOWN; + +import lombok.EqualsAndHashCode; + +/** + * The type of a flat_object field. See doc + * + *

A flat_object declares no sub-fields in the index mapping: its keys exist only inside the + * documents, and every leaf is indexed as a single keyword term with no numeric type. The engine + * therefore reads its values from {@code _source} and presents the field as a map keyed by the + * dotted leaf path, so that a nested object and a literal dotted key resolve to the same entry. + * + *

Like the other mapping types with no direct core-type counterpart (text, geo_point, binary), + * it carries {@code UNKNOWN} as its core type so that {@link #getExprType()} returns this instance + * and the Calcite type factory can map it by name. + */ +@EqualsAndHashCode(callSuper = false) +public class OpenSearchFlatObjectType extends OpenSearchDataType { + + private static final OpenSearchFlatObjectType instance = new OpenSearchFlatObjectType(); + + private OpenSearchFlatObjectType() { + super(MappingType.FlatObject); + exprCoreType = UNKNOWN; + } + + public static OpenSearchFlatObjectType of() { + return OpenSearchFlatObjectType.instance; + } + + @Override + protected OpenSearchDataType cloneEmpty() { + return instance; + } +} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/Content.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/Content.java index d2fa68dac06..fe4ea935454 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/Content.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/Content.java @@ -53,6 +53,9 @@ public interface Content { /** Is array value. */ boolean isArray(); + /** Is object value: a set of named members, as opposed to a scalar or an array. */ + boolean isObject(); + /** Get integer value. */ Integer intValue(); diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/ObjectContent.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/ObjectContent.java index 6818e75ec07..69dd3cd7075 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/ObjectContent.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/ObjectContent.java @@ -144,6 +144,11 @@ public boolean isArray() { return value instanceof ArrayNode || value instanceof List; } + @Override + public boolean isObject() { + return value instanceof Map; + } + @Override public boolean isString() { return value instanceof String; diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java index 587e133a10f..092b6ba7a69 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java @@ -140,6 +140,11 @@ public boolean isArray() { return value().isArray(); } + @Override + public boolean isObject() { + return value().isObject(); + } + @Override public Object objectValue() { return value(); diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/FlatObjectValues.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/FlatObjectValues.java new file mode 100644 index 00000000000..b7ea7b52e2c --- /dev/null +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/FlatObjectValues.java @@ -0,0 +1,164 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.opensearch.data.value; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import lombok.experimental.UtilityClass; +import org.opensearch.sql.data.model.ExprNullValue; +import org.opensearch.sql.data.model.ExprStringValue; +import org.opensearch.sql.data.model.ExprTupleValue; +import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.opensearch.data.utils.Content; + +/** + * Flattens a flat_object value into the shape the engine presents it as: a single-level map from + * dotted leaf path to the text of the value written there. + * + *

A flat_object declares no sub-fields, so its shape is known only from the document, and the + * index files one term per leaf with the path folded in. This mirrors that: every way of writing a + * path reaches the same entry ({@code {"a": {"b": 1}}} and {@code {"a.b": 1}} both give {@code + * a.b}), a path written more than once holds every value, and each value is the text the index + * holds for it -- which is also the only form the field can be looked up by. + */ +@UtilityClass +public class FlatObjectValues { + + /** + * Depth guard. A flat_object exists so that documents can escape the mapping depth limit, so + * nothing upstream bounds this recursion. Levels beyond the cap are kept as one leaf rather than + * dropped. + */ + static final int MAX_DEPTH = 20; + + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + + /** + * Flatten a parsed flat_object value. + * + * @param content the field's value as parsed from {@code _source} + * @return a tuple keyed by dotted leaf path, or null if the value is neither an object nor an + * array (a scalar where the mapping promised an object, e.g. a wildcard over conflicting + * mappings) + */ + public static ExprValue flatten(Content content) { + if (content == null || content.isNull()) { + return ExprNullValue.of(); + } + LinkedHashMap leaves = new LinkedHashMap<>(); + if (content.isObject()) { + flattenInto(content, "", leaves, 0); + } else if (content.isArray()) { + // OpenSearch accepts an array at the top level and folds every element's paths into the same + // terms, so the elements merge into one map here too. + content + .array() + .forEachRemaining( + element -> { + if (element.isObject()) { + flattenInto(element, "", leaves, 0); + } + }); + } else { + return ExprNullValue.of(); + } + return ExprTupleValue.fromExprValueMap(leaves); + } + + private static void flattenInto( + Content content, String prefix, LinkedHashMap out, int depth) { + content + .map() + .forEachRemaining( + entry -> { + String key = prefix.isEmpty() ? entry.getKey() : prefix + "." + entry.getKey(); + fold(entry.getValue(), key, out, depth); + }); + } + + /** + * Fold one value into the map under {@code key}, the way the index folds it into terms: an object + * contributes its own leaves under the dotted path, and an array contributes each element under + * the same path, whatever the element is. So {@code {"a": [1, 2]}}, {@code [{"a": 1}, {"a": 2}]} + * and {@code {"a": {"b": 1}, "a.b": 2}} all reach the same shape the index holds. + */ + private static void fold( + Content value, String key, LinkedHashMap out, int depth) { + if (value.isObject() && depth < MAX_DEPTH) { + flattenInto(value, key, out, depth + 1); + return; + } + if (value.isArray()) { + value.array().forEachRemaining(element -> fold(element, key, out, depth)); + return; + } + put(out, key, text(value)); + } + + /** + * A leaf as the text the index holds for it. The index has no type for a leaf -- every value is a + * keyword term -- so the engine presents the same text, and a query reads and compares exactly + * what a lookup on the field would match. + */ + private static ExprValue text(Content value) { + ExprValue parsed = OpenSearchExprValueFactory.parseContent(value); + return parsed.isNull() + ? ExprNullValue.of() + : new ExprStringValue(String.valueOf(parsed.value())); + } + + /** + * A path written once holds its value. A path written more than once -- both spellings of a + * dotted path in one document, or one path across the elements of an array -- holds all of them, + * since the index files a term for each and answers a lookup for any of them, and they read as + * their JSON, the way {@code spath} reads an array out of a JSON string. + */ + private static void put(LinkedHashMap out, String key, ExprValue value) { + ExprValue existing = out.get(key); + if (existing == null) { + out.put(key, value); + return; + } + List values = new ArrayList<>(); + collect(existing, values); + collect(value, values); + out.put(key, new ExprStringValue(asJsonArray(values))); + } + + /** The values a key already holds, unwrapped from the JSON an earlier merge wrote. */ + private static void collect(ExprValue value, List into) { + if (value.isNull()) { + return; + } + String text = value.stringValue(); + if (text.startsWith("[") && text.endsWith("]")) { + into.addAll(fromJsonArray(text)); + } else { + into.add(text); + } + } + + private static String asJsonArray(List values) { + try { + return OBJECT_MAPPER.writeValueAsString(values); + } catch (JsonProcessingException e) { + throw new IllegalStateException("Cannot render a flat_object leaf as JSON", e); + } + } + + private static List fromJsonArray(String json) { + try { + return OBJECT_MAPPER.readValue(json, new TypeReference>() {}); + } catch (JsonProcessingException e) { + // not JSON this class wrote: a single value that happens to look like an array + return List.of(json); + } + } +} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index aeda2da0ce6..be75bc5caa9 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -61,6 +61,7 @@ import org.opensearch.sql.opensearch.data.type.OpenSearchBinaryType; import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; import org.opensearch.sql.opensearch.data.type.OpenSearchDateType; +import org.opensearch.sql.opensearch.data.type.OpenSearchFlatObjectType; import org.opensearch.sql.opensearch.data.type.OpenSearchTextType; import org.opensearch.sql.opensearch.data.utils.Content; import org.opensearch.sql.opensearch.data.utils.ObjectContent; @@ -217,6 +218,11 @@ private ExprValue parse( } catch (Exception e) { return ExprNullValue.of(); } + } else if (type.equals(OpenSearchFlatObjectType.of())) { + // Before the isArray() branch below: OpenSearch accepts an array at the top level of a + // flat_object and folds its elements into the same terms, so flatten handles it rather than + // parseArray, which would leave the elements unflattened. + return FlatObjectValues.flatten(content); } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.Nested)) || content.isArray()) { return parseArray(content, field, type, supportArrays); @@ -260,7 +266,7 @@ private static String stringOf(Content content) { } } - private ExprValue parseContent(Content content) { + static ExprValue parseContent(Content content) { if (content.isNumber()) { if (content.isInt()) { return new ExprIntegerValue(content.intValue()); diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java index a62661ee601..312852e2df4 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java @@ -40,8 +40,10 @@ import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.opensearch.data.type.OpenSearchBinaryType; +import org.opensearch.sql.opensearch.data.type.OpenSearchFlatObjectType; import org.opensearch.sql.opensearch.data.type.OpenSearchTextType; import org.opensearch.sql.opensearch.storage.script.CalciteScriptEngine.Source; +import org.opensearch.sql.opensearch.storage.script.CalciteScriptEngine.UnsupportedScriptException; /** * This help standardizes the RexNode expression, the process including: @@ -114,6 +116,13 @@ public RexNode visitInputRef(RexInputRef inputRef, ScriptParameterHelper helper) || exprType instanceof OpenSearchBinaryType ? null : OpenSearchTextType.toKeywordSubField(field.getName(), exprType); + if (exprType instanceof OpenSearchFlatObjectType) { + // A flat_object has no usable doc values (each is the folded path=value term), so a script + // could only read it from _source, which means opening every record -- and the field type + // does not support Painless for retrieving subfield values either. + throw new UnsupportedScriptException( + "A flat_object field cannot be read by a pushed-down script: " + field.getName()); + } int newIndex = helper.sources.size(); if (docFieldName != null) { helper.digests.add(docFieldName); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java index 247b7a754e0..1f69e86be93 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java @@ -568,4 +568,16 @@ public void test_parseMapping_on_AliasType() { new OpenSearchAliasType("col1", OpenSearchDataType.of(MappingType.Invalid))), OpenSearchDataType.parseMapping(indexMapping2)); } + + @Test + // A flat_object declares no `properties`, so it must survive parseMapping as a leaf of its own + // type -- an unknown mapping type used to be dropped, which is why the field was invisible. + public void of_flat_object() { + Map parsed = + OpenSearchDataType.parseMapping(Map.of("attributes", Map.of("type", "flat_object"))); + assertAll( + () -> assertEquals(1, parsed.size()), + () -> assertEquals(OpenSearchFlatObjectType.of(), parsed.get("attributes")), + () -> assertEquals(1, OpenSearchDataType.traverseAndFlatten(parsed).size())); + } } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchFlatObjectTypeTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchFlatObjectTypeTest.java new file mode 100644 index 00000000000..edbd137a62f --- /dev/null +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchFlatObjectTypeTest.java @@ -0,0 +1,66 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.opensearch.data.type; + +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.opensearch.sql.data.type.ExprCoreType.UNKNOWN; + +import java.util.Map; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.sql.type.SqlTypeName; +import org.junit.jupiter.api.Test; +import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory; + +class OpenSearchFlatObjectTypeTest { + + @Test + void of_isASingletonThatSurvivesCloning() { + OpenSearchFlatObjectType type = OpenSearchFlatObjectType.of(); + assertAll( + () -> assertSame(type, OpenSearchFlatObjectType.of()), + () -> assertSame(type, OpenSearchDataType.of(OpenSearchDataType.MappingType.FlatObject)), + () -> assertSame(type, type.cloneEmpty()), + () -> assertSame(type, type.cloneDeep())); + } + + // Like text, geo_point and binary: no core-type counterpart, so the type stands for itself in + // the type environment and is mapped to Calcite by name. + @Test + void hasNoCoreTypeAndResolvesToItself() { + OpenSearchFlatObjectType type = OpenSearchFlatObjectType.of(); + assertAll( + () -> assertEquals(UNKNOWN, type.getExprCoreType()), + () -> assertSame(type, type.getExprType()), + () -> assertEquals("FLAT_OBJECT", type.legacyTypeName()), + () -> assertEquals(OpenSearchDataType.MappingType.FlatObject, type.getMappingType()), + () -> assertTrue(type.getProperties().isEmpty())); + } + + // The field is presented to the planner as a map from dotted leaf path to text: a leaf has no + // type of its own, and the index holds each as a keyword term. A map of text to text is also what + // tells this field apart from an ordinary object, which is a map of text to ANY. + @Test + void mapsToCalciteMapOfVarcharToVarchar() { + RelDataType relType = + OpenSearchTypeFactory.convertExprTypeToRelDataType(OpenSearchFlatObjectType.of()); + assertAll( + () -> assertEquals(SqlTypeName.MAP, relType.getSqlTypeName()), + () -> assertEquals(SqlTypeName.VARCHAR, relType.getKeyType().getSqlTypeName()), + () -> assertEquals(SqlTypeName.VARCHAR, relType.getValueType().getSqlTypeName()), + () -> assertTrue(relType.getValueType().isNullable())); + } + + // The parsed mapping keeps the singleton, so a schema built from it uses the map type above. + @Test + void parsedMappingUsesTheSingleton() { + Map parsed = + OpenSearchDataType.parseMapping(Map.of("attributes", Map.of("type", "flat_object"))); + assertSame(OpenSearchFlatObjectType.of(), parsed.get("attributes")); + } +} diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/FlatObjectValuesTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/FlatObjectValuesTest.java new file mode 100644 index 00000000000..565e2f99617 --- /dev/null +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/FlatObjectValuesTest.java @@ -0,0 +1,117 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.opensearch.data.value; + +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.opensearch.sql.data.model.ExprValueUtils.nullValue; +import static org.opensearch.sql.data.model.ExprValueUtils.stringValue; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.opensearch.data.utils.ObjectContent; + +class FlatObjectValuesTest { + + private static ExprValue flatten(Object source) { + return FlatObjectValues.flatten(new ObjectContent(source)); + } + + // A leaf has no type of its own in the index, so every value reads as its text -- the same rule + // spath applies to a JSON string. + @Test + void everyLeafReadsAsText() { + Map source = new LinkedHashMap<>(); + source.put("n", 12.5); + source.put("i", 503); + source.put("s", "4"); + source.put("b", true); + source.put("z", null); + ExprValue value = flatten(source); + assertAll( + () -> assertEquals(stringValue("12.5"), value.tupleValue().get("n")), + () -> assertEquals(stringValue("503"), value.tupleValue().get("i")), + () -> assertEquals(stringValue("4"), value.tupleValue().get("s")), + () -> assertEquals(stringValue("true"), value.tupleValue().get("b")), + () -> assertEquals(nullValue(), value.tupleValue().get("z"))); + } + + // Both spellings of a path reach the same leaf, which is what the index holds: one term with the + // path folded in, whichever way the document wrote it. + @Test + void bothSpellingsOfAPathReachTheSameLeaf() { + assertAll( + () -> + assertEquals( + stringValue("1"), flatten(Map.of("a", Map.of("b", 1))).tupleValue().get("a.b")), + () -> assertEquals(stringValue("1"), flatten(Map.of("a.b", 1)).tupleValue().get("a.b"))); + } + + // The index files a term per value, so a path that receives several holds them all, as the JSON + // of those values -- again what spath returns for an array inside a JSON string. + @Test + void aPathWithSeveralValuesReadsAsJson() { + LinkedHashMap collide = new LinkedHashMap<>(); + collide.put("a", Map.of("b", 1)); + collide.put("a.b", 2); + LinkedHashMap collideOtherOrder = new LinkedHashMap<>(); + collideOtherOrder.put("a.b", 2); + collideOtherOrder.put("a", Map.of("b", 1)); + assertAll( + // an array leaf + () -> + assertEquals( + stringValue("[\"a\",\"b\"]"), + flatten(Map.of("tags", List.of("a", "b"))).tupleValue().get("tags")), + // elements of different types still read as text + () -> + assertEquals( + stringValue("[\"1\",\"x\"]"), + flatten(Map.of("mixed", List.of(1, "x"))).tupleValue().get("mixed")), + // an object inside an array folds into the dotted path, one term per element + () -> + assertEquals( + stringValue("[\"1\",\"2\"]"), + flatten(Map.of("nested", List.of(Map.of("k", 1), Map.of("k", 2)))) + .tupleValue() + .get("nested.k")), + // an array at the top level, which OpenSearch also accepts + () -> + assertEquals( + stringValue("[\"1\",\"2\"]"), + flatten(List.of(Map.of("p", 1), Map.of("p", 2))).tupleValue().get("p")), + // both spellings present in one document: neither value may be dropped. The order follows + // the document, as _source order does, so the two spellings read in the order written + () -> assertEquals(stringValue("[\"1\",\"2\"]"), flatten(collide).tupleValue().get("a.b")), + () -> + assertEquals( + stringValue("[\"2\",\"1\"]"), flatten(collideOtherOrder).tupleValue().get("a.b"))); + } + + @Test + void aScalarWhereAnObjectWasPromisedIsNull() { + assertEquals(nullValue(), flatten(5)); + } + + // A flat_object exists so documents can escape the mapping depth limit, so nothing upstream + // bounds the recursion: levels past the cap are kept as one leaf rather than dropped. + @Test + void depthIsCapped() { + Map deepest = Map.of("leaf", "v"); + Map source = deepest; + for (int i = 0; i < 25; i++) { + source = Map.of("l" + i, source); + } + ExprValue value = flatten(source); + assertEquals(1, value.tupleValue().size()); + String key = value.tupleValue().keySet().iterator().next(); + // MAX_DEPTH levels are descended, so the key is that many prefixes plus the leaf's own name + assertEquals(FlatObjectValues.MAX_DEPTH + 1, key.split("\\.").length); + } +} From 985741a55e6f3b5a942fcf81286913fd480c6e9a Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Wed, 30 Sep 2026 12:58:38 -0700 Subject: [PATCH 2/8] Read the flat_object field names from the mapping where it is in hand The scope check walked the finished plan for a table to ask which columns were flat_object. Record them instead as each relation is bound, which is where the mapping is already read, and hand the names to the check. This also covers every relation a query scans rather than the first one found, so the right side of a join is no longer missed. Signed-off-by: Jialiang Liang --- .../sql/calcite/CalcitePlanContext.java | 10 ++++ .../sql/calcite/CalciteRelNodeVisitor.java | 25 +++++++++- .../sql/calcite/FlatObjectScopeValidator.java | 50 +++++-------------- 3 files changed, 47 insertions(+), 38 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java b/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java index 0fbefa4a17a..a8afac905e2 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java @@ -10,9 +10,11 @@ import java.sql.Connection; import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.Stack; import java.util.function.BiFunction; import lombok.Getter; @@ -96,6 +98,14 @@ public class CalcitePlanContext { */ @Getter @Setter private boolean includeMetadata = false; + /** + * Dotted names of the {@code flat_object} fields on the relations this query scans, recorded when + * each relation is bound -- the one point where the index mapping is in hand. {@link + * FlatObjectScopeValidator} runs over the finished plan, by which time a scan may sit behind + * rewrites that no longer carry the table. + */ + @Getter private final Set flatObjectFields = new LinkedHashSet<>(); + private final Stack correlVar = new Stack<>(); private final Stack> windowPartitions = new Stack<>(); 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 dbbe07b1c5e..86de9f1bc5d 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java @@ -240,7 +240,7 @@ public RelNode analyze(UnresolvedPlan unresolved, CalcitePlanContext context) { RelNode plan = unresolved.accept(this, context); // Here rather than in QueryService.analyze, so the unified query pipeline -- which calls this // method directly -- is held to the same rules. - FlatObjectScopeValidator.validate(plan); + FlatObjectScopeValidator.validate(plan, context.getFlatObjectFields()); return plan; } @@ -272,6 +272,7 @@ public RelNode visitRelation(Relation node, CalcitePlanContext context) { } context.relBuilder.scan(node.getTableQualifiedName().getParts()); RelNode scan = context.relBuilder.peek(); + recordFlatObjectFields(scan, context); // Eagerly push down highlight config to the scan (highlight is a scan hint, not an operator) if (context.getHighlightConfig() != null && scan instanceof HighlightPushDown) { @@ -289,6 +290,28 @@ public RelNode visitRelation(Relation node, CalcitePlanContext context) { return context.relBuilder.peek(); } + /** + * Notes which of a relation's fields are {@code flat_object}, for {@link + * FlatObjectScopeValidator}. Their leaves have no mapping of their own, so the check cannot read + * the type off the plan -- and here is where the index mapping is in hand. + */ + private static void recordFlatObjectFields(RelNode scan, CalcitePlanContext context) { + RelOptTable relOptTable = scan.getTable(); + AbstractOpenSearchTable osTable = + relOptTable == null ? null : relOptTable.unwrap(AbstractOpenSearchTable.class); + if (osTable == null) { + return; + } + osTable + .getFieldTypes() + .forEach( + (name, type) -> { + if ("flat_object".equalsIgnoreCase(type.legacyTypeName())) { + context.getFlatObjectFields().add(name); + } + }); + } + // This is a tool method to add an existed RelOptTable to builder stack, not used for now private RelBuilder scan(RelOptTable tableSchema, CalcitePlanContext context) { final RelNode scan = diff --git a/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java b/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java index 17799e79fc1..824a6b65285 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java +++ b/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java @@ -11,7 +11,6 @@ import java.util.List; import java.util.Locale; import java.util.Set; -import org.apache.calcite.plan.RelOptTable; import org.apache.calcite.rel.RelNode; import org.apache.calcite.rel.RelVisitor; import org.apache.calcite.rel.core.Aggregate; @@ -27,8 +26,6 @@ import org.apache.calcite.rex.RexVisitorImpl; import org.apache.calcite.sql.SqlKind; import org.checkerframework.checker.nullness.qual.Nullable; -import org.opensearch.sql.calcite.plan.AbstractOpenSearchTable; -import org.opensearch.sql.calcite.utils.PlanUtils; import org.opensearch.sql.common.error.ErrorCode; import org.opensearch.sql.common.error.ErrorReport; import org.opensearch.sql.common.error.QueryProcessingStage; @@ -43,11 +40,11 @@ * have to open every record, which is the cost this field type exists to avoid. Such a query is * rejected here, while the plan is built, rather than answered slowly. * - *

A leaf is {@code ITEM(, '')}. The columns are found by their mapping - * type rather than by the type they take in the plan, because that type -- a map of text to text -- - * is also what {@code spath} produces for an extracted document. A column that a projection derived - * from a leaf is tracked too, so that a command which sorts or groups by a leaf, projecting it into - * a column of its own first, is caught. + *

A leaf is {@code ITEM(, '')}. Which columns those are is not read + * off the plan -- the type they take there, a map of text to text, is also what {@code spath} + * produces for an extracted document -- but comes from the mapping, recorded as each relation was + * bound. A column that a projection derived from a leaf is tracked too, so that a command which + * sorts or groups by a leaf, projecting it into a column of its own first, is caught. */ public final class FlatObjectScopeValidator { @@ -61,16 +58,15 @@ public final class FlatObjectScopeValidator { private FlatObjectScopeValidator() {} - /** Throws if the plan does anything with a flat_object field other than read it. */ - public static void validate(RelNode plan) { - Set fields = flatObjectFields(plan); - if (!fields.isEmpty()) { - validate(plan, fields); + /** + * Throws if the plan does anything with one of the given flat_object fields other than read it. + * The names come from {@link CalcitePlanContext#getFlatObjectFields()}, filled in as each + * relation was bound. + */ + public static void validate(RelNode plan, Set fields) { + if (fields.isEmpty()) { + return; } - } - - /** The check itself, over a known set of flat_object column names. */ - static void validate(RelNode plan, Set fields) { new RelVisitor() { @Override public void visit(RelNode node, int ordinal, @Nullable RelNode parent) { @@ -80,26 +76,6 @@ public void visit(RelNode node, int ordinal, @Nullable RelNode parent) { }.go(plan); } - /** The plan's flat_object columns, by mapping type; empty when the query touches none. */ - private static Set flatObjectFields(RelNode plan) { - RelOptTable table = PlanUtils.findTable(plan); - AbstractOpenSearchTable osTable = - table == null ? null : table.unwrap(AbstractOpenSearchTable.class); - if (osTable == null) { - return Set.of(); - } - Set fields = new HashSet<>(); - osTable - .getFieldTypes() - .forEach( - (name, type) -> { - if ("flat_object".equalsIgnoreCase(type.legacyTypeName())) { - fields.add(name); - } - }); - return fields; - } - private static void check(RelNode node, Set fields) { List inputFields = inputFields(node); Set leafColumns = leafColumns(node, fields); From 9023b72d103d8dc9cbd2470f7163dfbbfcce45af Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Wed, 30 Sep 2026 13:13:13 -0700 Subject: [PATCH 3/8] Follow a leaf through a cast everywhere, not just where it is allowed Reading a leaf as a number is allowed, so the check strips a cast before deciding that. Two other places ask the same question -- which column a read went into, and what to call it -- and neither stripped the cast. A cast leaf therefore reached pushdown untracked, where a script over a flat_object cannot be built, and the failure surfaced as HTTP 500, code UNKNOWN java.sql.SQLException: ... A flat_object field cannot be read by a pushed-down script: attributes for a query such as source=otel | eval d = cast(attributes.duration_ms as double) | sort d | fields service which now reports "Cannot sort by attributes.duration_ms" like the rest. The three callers share one stripCasts so they cannot drift again. Signed-off-by: Jialiang Liang --- .../sql/calcite/FlatObjectScopeValidator.java | 24 +++++++++++++------ .../calcite/FlatObjectScopeValidatorTest.java | 14 +++++++++++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java b/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java index 824a6b65285..207c6c5d7dc 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java +++ b/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java @@ -158,7 +158,7 @@ private static Set leafColumns(RelNode node, Set fields) { List below = inputFields(project); List projects = project.getProjects(); for (int i = 0; i < projects.size(); i++) { - RexNode expression = projects.get(i); + RexNode expression = stripCasts(projects.get(i)); boolean fromField = isLeafRef(expression, below, fields) || (expression instanceof RexInputRef ref @@ -178,16 +178,26 @@ private static Set leafColumns(RelNode node, Set fields) { * text, so converting it is the parse a caller would do anyway, the value comes from {@code * _source} either way, and it is how a query asks for a leaf as a number. */ - private static boolean isRead( - RexNode node, List inputFields, Set fields, Set leaves) { - if (node instanceof RexInputRef) { - return true; - } + /** + * Unwraps the casts a read may wear. {@code isRead} and {@link #leafColumns} have to agree on + * what counts as reading a leaf: a column one of them lets through while the other does not track + * is a column that reaches pushdown, where a script over a flat_object cannot be built. + */ + private static RexNode stripCasts(RexNode node) { RexNode read = node; while (read instanceof RexCall cast && (cast.getKind() == SqlKind.CAST || cast.getKind() == SqlKind.SAFE_CAST)) { read = cast.getOperands().get(0); } + return read; + } + + private static boolean isRead( + RexNode node, List inputFields, Set fields, Set leaves) { + if (node instanceof RexInputRef) { + return true; + } + RexNode read = stripCasts(node); return isLeafRef(read, inputFields, fields) || (read instanceof RexInputRef ref && leaves.contains(ref.getIndex())); } @@ -262,7 +272,7 @@ private static String displayName( RelNode node, List inputFields, int index, Set fields) { if (!node.getInputs().isEmpty() && node.getInput(0) instanceof Project project) { List below = inputFields(project); - RexNode expression = project.getProjects().get(index); + RexNode expression = stripCasts(project.getProjects().get(index)); if (isLeafRef(expression, below, fields)) { return leafName((RexCall) expression, below); } diff --git a/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java b/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java index 1ec545f12fe..199f21c7533 100644 --- a/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java +++ b/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java @@ -163,6 +163,20 @@ void sortingByALeafIsRejected() { "Cannot sort by attributes"); } + // Reading a leaf as a number is allowed, so a cast is stripped when deciding that -- and it has to + // be stripped just the same when noting which column the read went into. Otherwise the column + // sorts by a leaf unchecked and the failure surfaces from pushdown, as a 500. + @Test + void sortingByACastLeafIsRejected() { + RexNode asDouble = + rexBuilder.makeCast( + TYPE_FACTORY.createSqlType(SqlTypeName.DOUBLE), leaf("duration_ms"), true, true); + rejected( + LogicalSort.create( + project(field(0), asDouble), RelCollations.of(new RelFieldCollation(1)), null, null), + "Cannot sort by attributes.duration_ms"); + } + @Test void groupingByALeafIsRejected() { RelNode projected = project(field(0), leaf("status_code")); From 99e53161450bfd3e99661055d50ccc1931fe0bfe Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Wed, 30 Sep 2026 13:24:39 -0700 Subject: [PATCH 4/8] Keep the scope check to its package and say why it is a plan pass A reviewer's first question is why this is not thrown from where a leaf reference is built, the way an unsupported command is rejected from its visit method. Field resolution is not a safe place to reject from: MapPathPreMaterializer, which every plan node passes through, wraps it in catch (RuntimeException | AssertionError) and logs at debug, so the rejection would be dropped for rename, fillnull, replace, rare, top, lookup and join and the query would fail in pushdown instead. Noted on the class, and the class is package-private now -- only the visitor and its own test use it. Signed-off-by: Jialiang Liang --- .../sql/calcite/FlatObjectScopeValidator.java | 12 ++++++++++-- .../sql/calcite/FlatObjectScopeValidatorTest.java | 3 ++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java b/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java index 207c6c5d7dc..aa48564be1c 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java +++ b/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java @@ -40,13 +40,21 @@ * have to open every record, which is the cost this field type exists to avoid. Such a query is * rejected here, while the plan is built, rather than answered slowly. * + *

This runs over the finished plan rather than throwing from {@link QualifiedNameResolver} where + * a leaf reference is built, the way an unsupported command is rejected from {@code visitX}. Field + * resolution is not a safe place to reject from: {@code MapPathPreMaterializer}, which every plan + * node passes through, wraps it in {@code catch (RuntimeException | AssertionError)} and logs at + * debug, so a rejection raised there is dropped for {@code rename}, {@code fillnull}, {@code + * replace}, {@code rare}, {@code top}, {@code lookup} and {@code join}, and the query goes on to + * fail in pushdown instead. The finished plan is downstream of that. + * *

A leaf is {@code ITEM(, '')}. Which columns those are is not read * off the plan -- the type they take there, a map of text to text, is also what {@code spath} * produces for an extracted document -- but comes from the mapping, recorded as each relation was * bound. A column that a projection derived from a leaf is tracked too, so that a command which * sorts or groups by a leaf, projecting it into a column of its own first, is caught. */ -public final class FlatObjectScopeValidator { +final class FlatObjectScopeValidator { private static final String DOC_URL = "https://docs.opensearch.org/latest/mappings/supported-field-types/flat-object/"; @@ -63,7 +71,7 @@ private FlatObjectScopeValidator() {} * The names come from {@link CalcitePlanContext#getFlatObjectFields()}, filled in as each * relation was bound. */ - public static void validate(RelNode plan, Set fields) { + static void validate(RelNode plan, Set fields) { if (fields.isEmpty()) { return; } diff --git a/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java b/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java index 199f21c7533..5f6e3f11146 100644 --- a/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java +++ b/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java @@ -163,7 +163,8 @@ void sortingByALeafIsRejected() { "Cannot sort by attributes"); } - // Reading a leaf as a number is allowed, so a cast is stripped when deciding that -- and it has to + // Reading a leaf as a number is allowed, so a cast is stripped when deciding that -- and it has + // to // be stripped just the same when noting which column the read went into. Otherwise the column // sorts by a leaf unchecked and the failure surfaces from pushdown, as a 500. @Test From ba8088202fe3df875c520942c922c84320e9f6b5 Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Wed, 30 Sep 2026 13:45:40 -0700 Subject: [PATCH 5/8] Correct why a script over a flat_object is refused The comment claimed the field type does not support Painless for retrieving subfield values. That is a limitation of doc-value access; this engine's script reads _source, and it does retrieve flat_object leaves correctly -- a sort over one returns the right order. The reason to refuse is the cost of that read, not an inability, and the types above are read from _source on purpose, so the comment now says which of the two this is. Signed-off-by: Jialiang Liang --- .../sql/opensearch/storage/serde/RexStandardizer.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java index 312852e2df4..64bcecba6e5 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java @@ -117,9 +117,10 @@ public RexNode visitInputRef(RexInputRef inputRef, ScriptParameterHelper helper) ? null : OpenSearchTextType.toKeywordSubField(field.getName(), exprType); if (exprType instanceof OpenSearchFlatObjectType) { - // A flat_object has no usable doc values (each is the folded path=value term), so a script - // could only read it from _source, which means opening every record -- and the field type - // does not support Painless for retrieving subfield values either. + // Each leaf is indexed as one folded path=value term, so a flat_object has no doc values a + // script could read: it would have to open _source for every record it is asked about, the + // cost the field type exists to avoid. A script over one is refused rather than built, unlike + // the no-doc-values types above, which are read from _source that way on purpose. throw new UnsupportedScriptException( "A flat_object field cannot be read by a pushed-down script: " + field.getName()); } From e2b7cf68ae14490a0e98115645698f5b0e727b80 Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Wed, 30 Sep 2026 14:12:44 -0700 Subject: [PATCH 6/8] Check what a flat_object allows while the plan is analyzed, as a list of allowed uses The check walked the finished plan and rejected every node that was not a read. Nothing else in this package works that way -- MapPathPreMaterializer, SchemaUnifier, ForeachPlanner and QualifiedNameResolver are all helpers the visitor calls while building -- and a plan pass pays for it: it has to trace a column back through a projection to find the leaf it came from, and rebuild "attributes.duration_ms" out of a generated $f2, because Calcite renames columns and rebuilds expressions. The rule is now a list of the uses a flat_object allows, checked where a field reference resolves. Every strategy in QualifiedNameResolver converges on one point, so a command not on the list is refused without an entry of its own -- including one added later -- and the name in the message is the one the query wrote. Reading a field or a leaf, optionally cast, and typeof are the list; a column a read went into is remembered so a later command using it is refused too, named by the leaf rather than by the alias. Two things had to be arranged around this. The materializer resolves fields inside catch (RuntimeException | AssertionError), so it runs with the read permission open rather than having a refusal swallowed there and the field quietly ignored. And expand already refuses anything that is not a plain column, in its own words, so its resolution is a read and that check decides. Behaviour is unchanged over 22 queries covering every allowed and refused form, with one message corrected: dedup said "Cannot filter by", the name of what it lowers to, and now says "Cannot dedup by". Signed-off-by: Jialiang Liang --- .../sql/calcite/CalcitePlanContext.java | 14 + .../sql/calcite/CalciteRelNodeVisitor.java | 71 ++-- .../sql/calcite/FlatObjectScopeValidator.java | 313 ------------------ .../sql/calcite/QualifiedNameResolver.java | 4 + .../sql/calcite/utils/FlatObjectScope.java | 230 +++++++++++++ .../calcite/FlatObjectScopeValidatorTest.java | 211 ------------ .../calcite/utils/FlatObjectScopeTest.java | 207 ++++++++++++ 7 files changed, 502 insertions(+), 548 deletions(-) delete mode 100644 core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java create mode 100644 core/src/main/java/org/opensearch/sql/calcite/utils/FlatObjectScope.java delete mode 100644 core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java create mode 100644 core/src/test/java/org/opensearch/sql/calcite/utils/FlatObjectScopeTest.java diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java b/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java index a8afac905e2..cc57e2ea1b7 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java @@ -10,6 +10,7 @@ import java.sql.Connection; import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -106,6 +107,19 @@ public class CalcitePlanContext { */ @Getter private final Set flatObjectFields = new LinkedHashSet<>(); + /** + * Columns a command read a {@code flat_object} field or one of its leaves into, by the name the + * command gave them. A later command using such a column is refused the same way, since pushdown + * fuses the projection back into whatever reads it. + */ + @Getter private final Map flatObjectLeafColumns = new LinkedHashMap<>(); + + /** Whether the expression being resolved is one of the reads a flat_object field allows. */ + @Getter @Setter private boolean flatObjectReadAllowed = false; + + /** What the command being analyzed would do with a flat_object field, for the refusal message. */ + @Getter @Setter private String flatObjectUse = "use"; + private final Stack correlVar = new Stack<>(); private final Stack> windowPartitions = new Stack<>(); 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 86de9f1bc5d..2ad8c3a7b28 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java @@ -181,6 +181,7 @@ import org.opensearch.sql.calcite.plan.rel.LogicalSystemLimit; import org.opensearch.sql.calcite.plan.rel.LogicalSystemLimit.SystemLimitType; import org.opensearch.sql.calcite.utils.BinUtils; +import org.opensearch.sql.calcite.utils.FlatObjectScope; import org.opensearch.sql.calcite.utils.JoinAndLookupUtils; import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory; import org.opensearch.sql.calcite.utils.PPLHintUtils; @@ -240,7 +241,6 @@ public RelNode analyze(UnresolvedPlan unresolved, CalcitePlanContext context) { RelNode plan = unresolved.accept(this, context); // Here rather than in QueryService.analyze, so the unified query pipeline -- which calls this // method directly -- is held to the same rules. - FlatObjectScopeValidator.validate(plan, context.getFlatObjectFields()); return plan; } @@ -248,7 +248,11 @@ public RelNode analyze(UnresolvedPlan unresolved, CalcitePlanContext context) { public RelNode visitChildren(Node node, CalcitePlanContext context) { RelNode result = super.visitChildren(node, context); if (node instanceof UnresolvedPlan plan) { - mapPathMaterializer.materializePaths(plan, context); + // Materializing a dotted path is a read, so it is allowed -- and it has to be, because the + // materializer wraps field resolution in catch (RuntimeException | AssertionError), which + // would swallow a refusal raised there and leave the command quietly ignoring the field. + // What the command then does with the materialized column is checked where it resolves it. + FlatObjectScope.runAsRead(context, () -> mapPathMaterializer.materializePaths(plan, context)); } return result; } @@ -369,7 +373,9 @@ public RelNode visitFilter(Filter node, CalcitePlanContext context) { context.relBuilder.variable(v::set); context.pushCorrelVar(v.get()); } - RexNode condition = rexVisitor.analyze(node.getCondition(), context); + RexNode condition = + FlatObjectScope.as( + "filter by", context, () -> rexVisitor.analyze(node.getCondition(), context)); if (containsSubqueryExpression) { context.relBuilder.filter(ImmutableList.of(v.get().id), condition); context.popCorrelVar(); @@ -612,7 +618,9 @@ private List expandProjectFields( } matchingFields.forEach(f -> expandedFields.add(context.relBuilder.field(f))); } else if (addedFields.add(fieldName)) { - RexNode resolved = rexVisitor.analyze(field, context); + RexNode resolved = + FlatObjectScope.allowRead( + field, fieldName, context, () -> 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. @@ -801,6 +809,7 @@ public RelNode visitRename(Rename node, CalcitePlanContext context) { String newName = WildcardRenameUtils.applyWildcardTransformation( sourcePattern, targetPattern, fieldName); + FlatObjectScope.trackRename(fieldName, newName, context); if (newNames.contains(newName) && !newName.equals(fieldName)) { removeFieldIfExists(newName, newNames, context); } @@ -829,23 +838,27 @@ private void removeFieldIfExists( public RelNode visitSort(org.opensearch.sql.ast.tree.Sort node, CalcitePlanContext context) { visitChildren(node, context); List sortList = - node.getSortList().stream() - .map( - expr -> { - RexNode sortField = rexVisitor.analyze(expr, context); - SortOption sortOption = analyzeSortOption(expr.getFieldArgs()); - // Default is ASC - if (sortOption.getSortOrder() == DESC) { - sortField = context.relBuilder.desc(sortField); - } - if (sortOption.getNullOrder() == NULL_LAST) { - sortField = context.relBuilder.nullsLast(sortField); - } else { - sortField = context.relBuilder.nullsFirst(sortField); - } - return sortField; - }) - .collect(Collectors.toList()); + FlatObjectScope.as( + "sort by", + context, + () -> + node.getSortList().stream() + .map( + expr -> { + RexNode sortField = rexVisitor.analyze(expr, context); + SortOption sortOption = analyzeSortOption(expr.getFieldArgs()); + // Default is ASC + if (sortOption.getSortOrder() == DESC) { + sortField = context.relBuilder.desc(sortField); + } + if (sortOption.getNullOrder() == NULL_LAST) { + sortField = context.relBuilder.nullsLast(sortField); + } else { + sortField = context.relBuilder.nullsFirst(sortField); + } + return sortField; + }) + .collect(Collectors.toList())); context.relBuilder.sort(sortList); // Apply count parameter as limit if (node.getCount() != 0) { @@ -1292,7 +1305,12 @@ public RelNode visitEval(Eval node, CalcitePlanContext context) { context.relBuilder.variable(v::set); context.pushCorrelVar(v.get()); } - RexNode eval = rexVisitor.analyze(expr, context); + RexNode eval = + FlatObjectScope.allowRead( + expr, + expr.getVar().getField().toString(), + context, + () -> rexVisitor.analyze(expr, context)); if (containsSubqueryExpression) { // RelBuilder.projectPlus doesn't have a parameter with variablesSet: // projectPlus(Iterable variablesSet, RexNode... nodes) @@ -1802,7 +1820,8 @@ public RelNode visitAggregation(Aggregation node, CalcitePlanContext context) { if (!bucketNullable) { nonNullGroupMask.set(0, nGroup); } - visitAggregation(node, context, nonNullGroupMask, true, false); + FlatObjectScope.runAs( + "stats by", context, () -> visitAggregation(node, context, nonNullGroupMask, true, false)); return context.relBuilder.peek(); } @@ -2273,6 +2292,7 @@ public RelNode visitLookup(Lookup node, CalcitePlanContext context) { @Override public RelNode visitDedupe(Dedupe node, CalcitePlanContext context) { + context.setFlatObjectUse("dedup by"); visitChildren(node, context); List options = node.getOptions(); Integer allowedDuplication = (Integer) options.get(0).getValue().getValue(); @@ -4484,7 +4504,10 @@ public RelNode visitExpand(Expand expand, CalcitePlanContext context) { // 2. Get the field to expand and an optional alias. Field arrayField = expand.getField(); - RexNode resolved = rexVisitor.analyze(arrayField, context); + // Resolve as a read: the check below already refuses a flat_object leaf, by the same rule it + // uses for an object leaf or an expression, and says so in expand's own words. + RexNode resolved = + FlatObjectScope.allowReadOf(context, () -> rexVisitor.analyze(arrayField, context)); if (!(resolved instanceof RexInputRef arrayFieldRex)) { // Anything that is not a column of the input -- a leaf of an object or a flat_object field, // an expression -- has no column to correlate the expansion with. Raised the way the diff --git a/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java b/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java deleted file mode 100644 index aa48564be1c..00000000000 --- a/core/src/main/java/org/opensearch/sql/calcite/FlatObjectScopeValidator.java +++ /dev/null @@ -1,313 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.calcite; - -import java.util.ArrayList; -import java.util.HashSet; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Locale; -import java.util.Set; -import org.apache.calcite.rel.RelNode; -import org.apache.calcite.rel.RelVisitor; -import org.apache.calcite.rel.core.Aggregate; -import org.apache.calcite.rel.core.Filter; -import org.apache.calcite.rel.core.Project; -import org.apache.calcite.rel.core.Sort; -import org.apache.calcite.rel.type.RelDataTypeField; -import org.apache.calcite.rex.RexCall; -import org.apache.calcite.rex.RexInputRef; -import org.apache.calcite.rex.RexLiteral; -import org.apache.calcite.rex.RexNode; -import org.apache.calcite.rex.RexShuttle; -import org.apache.calcite.rex.RexVisitorImpl; -import org.apache.calcite.sql.SqlKind; -import org.checkerframework.checker.nullness.qual.Nullable; -import org.opensearch.sql.common.error.ErrorCode; -import org.opensearch.sql.common.error.ErrorReport; -import org.opensearch.sql.common.error.QueryProcessingStage; -import org.opensearch.sql.common.utils.StringUtils; - -/** - * Keeps a query to what a {@code flat_object} field can answer. - * - *

A flat_object gives its leaves no mapping of their own: the index files every leaf value as a - * keyword term with the path folded in, and the engine reads the field from {@code _source}. So a - * leaf can be read, and that is all PPL does with it today -- anything computed from a leaf would - * have to open every record, which is the cost this field type exists to avoid. Such a query is - * rejected here, while the plan is built, rather than answered slowly. - * - *

This runs over the finished plan rather than throwing from {@link QualifiedNameResolver} where - * a leaf reference is built, the way an unsupported command is rejected from {@code visitX}. Field - * resolution is not a safe place to reject from: {@code MapPathPreMaterializer}, which every plan - * node passes through, wraps it in {@code catch (RuntimeException | AssertionError)} and logs at - * debug, so a rejection raised there is dropped for {@code rename}, {@code fillnull}, {@code - * replace}, {@code rare}, {@code top}, {@code lookup} and {@code join}, and the query goes on to - * fail in pushdown instead. The finished plan is downstream of that. - * - *

A leaf is {@code ITEM(, '')}. Which columns those are is not read - * off the plan -- the type they take there, a map of text to text, is also what {@code spath} - * produces for an extracted document -- but comes from the mapping, recorded as each relation was - * bound. A column that a projection derived from a leaf is tracked too, so that a command which - * sorts or groups by a leaf, projecting it into a column of its own first, is caught. - */ -final class FlatObjectScopeValidator { - - private static final String DOC_URL = - "https://docs.opensearch.org/latest/mappings/supported-field-types/flat-object/"; - - private static final String SUGGESTION = - "A flat_object field can only be read. Its leaves have no mapping of their own, so nothing" - + " else can be answered without opening every record. See " - + DOC_URL; - - private FlatObjectScopeValidator() {} - - /** - * Throws if the plan does anything with one of the given flat_object fields other than read it. - * The names come from {@link CalcitePlanContext#getFlatObjectFields()}, filled in as each - * relation was bound. - */ - static void validate(RelNode plan, Set fields) { - if (fields.isEmpty()) { - return; - } - new RelVisitor() { - @Override - public void visit(RelNode node, int ordinal, @Nullable RelNode parent) { - super.visit(node, ordinal, parent); - check(node, fields); - } - }.go(plan); - } - - private static void check(RelNode node, Set fields) { - List inputFields = inputFields(node); - Set leafColumns = leafColumns(node, fields); - if (node instanceof Project project) { - // Reading a leaf, or the whole field, is the one thing allowed. - for (RexNode expression : project.getProjects()) { - if (isRead(expression, inputFields, fields, leafColumns)) { - continue; - } - List leaves = leavesIn(expression, inputFields, fields, leafColumns); - if (!leaves.isEmpty()) { - throw unsupported("Cannot compute a value from %s", String.join(", ", leaves)); - } - } - return; - } - // Every other node: a leaf may pass through by position, but nothing the node does may use one. - List used = new ArrayList<>(); - node.accept( - new RexShuttle() { - @Override - public RexNode visitInputRef(RexInputRef ref) { - if (isLeafColumn(ref.getIndex(), inputFields, fields, leafColumns)) { - used.add(displayName(node, inputFields, ref.getIndex(), fields)); - } - return ref; - } - - @Override - public RexNode visitCall(RexCall call) { - // a leaf is named by its path, not by the field the ITEM reads from - if (isLeafRef(call, inputFields, fields)) { - used.add(leafName(call, inputFields)); - return call; - } - return super.visitCall(call); - } - }); - if (node instanceof Sort sort) { - sort.getCollation() - .getFieldCollations() - .forEach( - key -> { - if (isLeafColumn(key.getFieldIndex(), inputFields, fields, leafColumns)) { - used.add(displayName(node, inputFields, key.getFieldIndex(), fields)); - } - }); - } - if (node instanceof Aggregate aggregate) { - List positions = new ArrayList<>(aggregate.getGroupSet().asList()); - aggregate.getAggCallList().forEach(call -> positions.addAll(call.getArgList())); - positions.forEach( - position -> { - if (isLeafColumn(position, inputFields, fields, leafColumns)) { - used.add(displayName(node, inputFields, position, fields)); - } - }); - } - if (!used.isEmpty()) { - throw unsupported( - "Cannot %s by %s", nameOf(node), String.join(", ", new LinkedHashSet<>(used))); - } - } - - private static boolean isLeafColumn( - int index, List inputFields, Set fields, Set leaves) { - return leaves.contains(index) || isFlatObjectColumn(inputFields, index, fields); - } - - /** - * The output column indices of {@code node}'s inputs that came off a flat_object field: a leaf a - * projection read, or the field itself passed through. A projection renames what it produces, so - * a command one level up cannot recognize either by name. - */ - private static Set leafColumns(RelNode node, Set fields) { - Set leaves = new HashSet<>(); - int offset = 0; - for (RelNode input : node.getInputs()) { - if (input instanceof Project project) { - List below = inputFields(project); - List projects = project.getProjects(); - for (int i = 0; i < projects.size(); i++) { - RexNode expression = stripCasts(projects.get(i)); - boolean fromField = - isLeafRef(expression, below, fields) - || (expression instanceof RexInputRef ref - && isFlatObjectColumn(below, ref.getIndex(), fields)); - if (fromField) { - leaves.add(offset + i); - } - } - } - offset += input.getRowType().getFieldCount(); - } - return leaves; - } - - /** - * Whether the expression only reads a leaf or the whole field. A cast of a leaf counts: a leaf is - * text, so converting it is the parse a caller would do anyway, the value comes from {@code - * _source} either way, and it is how a query asks for a leaf as a number. - */ - /** - * Unwraps the casts a read may wear. {@code isRead} and {@link #leafColumns} have to agree on - * what counts as reading a leaf: a column one of them lets through while the other does not track - * is a column that reaches pushdown, where a script over a flat_object cannot be built. - */ - private static RexNode stripCasts(RexNode node) { - RexNode read = node; - while (read instanceof RexCall cast - && (cast.getKind() == SqlKind.CAST || cast.getKind() == SqlKind.SAFE_CAST)) { - read = cast.getOperands().get(0); - } - return read; - } - - private static boolean isRead( - RexNode node, List inputFields, Set fields, Set leaves) { - if (node instanceof RexInputRef) { - return true; - } - RexNode read = stripCasts(node); - return isLeafRef(read, inputFields, fields) - || (read instanceof RexInputRef ref && leaves.contains(ref.getIndex())); - } - - /** {@code ITEM(, '')}. */ - private static boolean isLeafRef( - RexNode node, List inputFields, Set fields) { - return node instanceof RexCall call - && call.getKind() == SqlKind.ITEM - && call.getOperands().get(0) instanceof RexInputRef ref - && isFlatObjectColumn(inputFields, ref.getIndex(), fields) - && call.getOperands().get(1) instanceof RexLiteral; - } - - private static boolean isFlatObjectColumn( - List inputFields, int index, Set fields) { - return index >= 0 - && index < inputFields.size() - && fields.contains(StringUtils.unquoteIdentifier(inputFields.get(index).getName())); - } - - /** The leaf references inside an expression, named as the query writes them. */ - private static List leavesIn( - RexNode node, List inputFields, Set fields, Set leaves) { - Set found = new LinkedHashSet<>(); - node.accept( - new RexVisitorImpl(true) { - @Override - public Void visitInputRef(RexInputRef ref) { - if (isLeafColumn(ref.getIndex(), inputFields, fields, leaves)) { - found.add(name(inputFields, ref.getIndex())); - } - return null; - } - - @Override - public Void visitCall(RexCall call) { - if (isLeafRef(call, inputFields, fields)) { - found.add(leafName(call, inputFields)); - return null; - } - return super.visitCall(call); - } - }); - return List.copyOf(found); - } - - private static List inputFields(RelNode node) { - List fields = new ArrayList<>(); - for (RelNode input : node.getInputs()) { - fields.addAll(input.getRowType().getFieldList()); - } - return fields; - } - - private static String name(List inputFields, int index) { - return StringUtils.unquoteIdentifier(inputFields.get(index).getName()); - } - - private static String leafName(RexCall item, List inputFields) { - return name(inputFields, ((RexInputRef) item.getOperands().get(0)).getIndex()) - + "." - + RexLiteral.stringValue((RexLiteral) item.getOperands().get(1)); - } - - /** - * The name to show for an input column. A command that sorts or groups by a leaf projects it into - * a column of its own first, and that column has a generated name ({@code $f2}); name the leaf, - * or the field, that projection reads instead, which is what the query says. - */ - private static String displayName( - RelNode node, List inputFields, int index, Set fields) { - if (!node.getInputs().isEmpty() && node.getInput(0) instanceof Project project) { - List below = inputFields(project); - RexNode expression = stripCasts(project.getProjects().get(index)); - if (isLeafRef(expression, below, fields)) { - return leafName((RexCall) expression, below); - } - if (expression instanceof RexInputRef ref - && isFlatObjectColumn(below, ref.getIndex(), fields)) { - return name(below, ref.getIndex()); - } - } - return name(inputFields, index); - } - - /** The command as a query author names it, for the message. */ - private static String nameOf(RelNode node) { - return switch (node) { - case Sort ignored -> "sort"; - case Aggregate ignored -> "stats"; - case Filter ignored -> "filter"; - default -> node.getRelTypeName().toLowerCase(Locale.ROOT); - }; - } - - private static ErrorReport unsupported(String message, Object... args) { - return ErrorReport.wrap(new IllegalArgumentException(StringUtils.format(message, args))) - .code(ErrorCode.UNSUPPORTED_OPERATION) - .stage(QueryProcessingStage.ANALYZING) - .location("while checking what a flat_object field can answer") - .suggestion(SUGGESTION) - .build(); - } -} 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 c75c0829b5f..1250daee259 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/QualifiedNameResolver.java +++ b/core/src/main/java/org/opensearch/sql/calcite/QualifiedNameResolver.java @@ -17,6 +17,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.sql.ast.expression.QualifiedName; +import org.opensearch.sql.calcite.utils.FlatObjectScope; import org.opensearch.sql.common.error.ErrorCode; import org.opensearch.sql.common.error.ErrorReport; import org.opensearch.sql.common.utils.StringUtils; @@ -82,6 +83,9 @@ private static RexNode resolveInNonJoinCondition( .or(() -> resolveRenamedField(nameNode, context)); if (fieldRef.isPresent()) { + // Every way of resolving a field converges here, so this is where a flat_object used outside + // what the field type answers is refused -- see FlatObjectScope. + FlatObjectScope.check(nameNode.toString(), context); // If we're in a lambda context and this is not a lambda variable, // we need to capture it as an external variable if (context.isInLambdaContext()) { diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/FlatObjectScope.java b/core/src/main/java/org/opensearch/sql/calcite/utils/FlatObjectScope.java new file mode 100644 index 00000000000..0c788d646d9 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/calcite/utils/FlatObjectScope.java @@ -0,0 +1,230 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.calcite.utils; + +import java.util.Set; +import org.opensearch.sql.ast.expression.Alias; +import org.opensearch.sql.ast.expression.Cast; +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.QualifiedName; +import org.opensearch.sql.ast.expression.UnresolvedExpression; +import org.opensearch.sql.calcite.CalcitePlanContext; +import org.opensearch.sql.common.error.ErrorCode; +import org.opensearch.sql.common.error.ErrorReport; +import org.opensearch.sql.common.error.QueryProcessingStage; +import org.opensearch.sql.common.utils.StringUtils; + +/** + * What a query may do with a {@code flat_object} field: a list of the allowed uses, everything else + * refused. + * + *

A flat_object gives its leaves no mapping of their own. The index files every leaf value as + * one folded {@code path=value} keyword term, so the queries the field type answers are the ones + * that match a term -- the list under Supported + * queries. Anything else would have to open {@code _source} for every record it is asked about, + * the cost the field type exists to avoid, so it is refused while the plan is built rather than + * answered slowly. + * + *

Today the list is: read a leaf, or read the whole field. A read may wear a cast, which is how + * a query asks for a leaf as a number. + * + *

The refusal is raised from {@code QualifiedNameResolver}, where every reference to a field + * passes, so a command that is not on the list needs no entry here to be refused -- including one + * added later. {@code CalciteRelNodeVisitor} opens the permission for the reads above and closes it + * again, and names the command it was doing for the message. + */ +public final class FlatObjectScope { + + private static final String DOC_URL = + "https://docs.opensearch.org/latest/mappings/supported-field-types/flat-object/"; + + private static final String SUGGESTION = + "A flat_object field can only be read. Its leaves have no mapping of their own, so nothing" + + " else can be answered without opening every record. See " + + DOC_URL; + + private FlatObjectScope() {} + + /** + * Whether {@code name} is a flat_object field or a leaf under one, or a column a read of either + * went into. The mapping types are recorded as each relation is bound; the columns as each read + * is allowed. + */ + public static boolean isFlatObject(String name, CalcitePlanContext context) { + return context.getFlatObjectFields().contains(name) + || context.getFlatObjectLeafColumns().containsKey(name); + } + + /** Whether any part of {@code parts} up to {@code length} names a flat_object field. */ + public static boolean isFlatObjectField(String fieldName, CalcitePlanContext context) { + return context.getFlatObjectFields().contains(fieldName); + } + + /** + * Refuses the use unless the command being analyzed opened the permission. {@code name} is the + * field or leaf as the query spelled it. + */ + public static void check(String name, CalcitePlanContext context) { + if (context.isFlatObjectReadAllowed() || !rootedInFlatObject(name, context)) { + return; + } + throw refuse("Cannot %s %s", context.getFlatObjectUse(), originOf(name, context)); + } + + /** + * Runs {@code read} with the permission open, and notes the column a read of a flat_object went + * into so a later command that uses that column is refused too. {@code target} is the name the + * command gives the result, or null when it does not name one. + */ + public static T allowRead( + UnresolvedExpression expression, String target, CalcitePlanContext context, Reader read) { + boolean isRead = nameOf(expression) != null; + boolean previousAllowed = context.isFlatObjectReadAllowed(); + String previousUse = context.getFlatObjectUse(); + context.setFlatObjectReadAllowed(isRead); + if (!isRead) { + context.setFlatObjectUse("compute a value from"); + } + try { + T resolved = read.get(); + if (isRead && target != null && namesFlatObject(expression, context)) { + context.getFlatObjectLeafColumns().put(target, nameOf(expression)); + } + return resolved; + } finally { + context.setFlatObjectReadAllowed(previousAllowed); + context.setFlatObjectUse(previousUse); + } + } + + /** Runs {@code read} while naming what the command would be doing, for the refusal message. */ + public static T as(String use, CalcitePlanContext context, Reader read) { + String previous = context.getFlatObjectUse(); + context.setFlatObjectUse(use); + try { + return read.get(); + } finally { + context.setFlatObjectUse(previous); + } + } + + /** + * Runs {@code read} with the permission open, for a step that is a read by construction rather + * than by the shape of an expression. + */ + public static void runAsRead(CalcitePlanContext context, Runnable read) { + boolean previous = context.isFlatObjectReadAllowed(); + context.setFlatObjectReadAllowed(true); + try { + read.run(); + } finally { + context.setFlatObjectReadAllowed(previous); + } + } + + /** {@link #runAsRead} for a resolution that returns a value. */ + public static T allowReadOf(CalcitePlanContext context, Reader read) { + boolean previous = context.isFlatObjectReadAllowed(); + context.setFlatObjectReadAllowed(true); + try { + return read.get(); + } finally { + context.setFlatObjectReadAllowed(previous); + } + } + + /** + * Notes that {@code target} now holds what {@code source} held, so a use of it is refused too. + */ + public static void trackRename(String source, String target, CalcitePlanContext context) { + if (rootedInFlatObject(source, context)) { + context.getFlatObjectLeafColumns().put(target, originOf(source, context)); + } + } + + /** {@link #as} for a resolution that returns nothing. */ + public static void runAs(String use, CalcitePlanContext context, Runnable read) { + as( + use, + context, + () -> { + read.run(); + return null; + }); + } + + /** The field a name refers to: a column read from a leaf is named by that leaf, not by itself. */ + private static String originOf(String name, CalcitePlanContext context) { + return context.getFlatObjectLeafColumns().getOrDefault(name, name); + } + + /** Whether {@code name} is a flat_object field, a leaf under one, or a column read from one. */ + private static boolean rootedInFlatObject(String name, CalcitePlanContext context) { + if (isFlatObject(name, context)) { + return true; + } + Set fields = context.getFlatObjectFields(); + for (String field : fields) { + if (name.startsWith(field + ".")) { + return true; + } + } + return false; + } + + /** Whether the expression is nothing but a field reference, optionally cast. */ + private static boolean isBareRead(UnresolvedExpression expression) { + return nameOf(expression) != null; + } + + /** Whether the expression is a bare read of a flat_object field or a leaf under one. */ + private static boolean namesFlatObject( + UnresolvedExpression expression, CalcitePlanContext context) { + String name = nameOf(expression); + return name != null && rootedInFlatObject(name, context); + } + + /** The field a bare read names, with the aliases and casts around it removed; null otherwise. */ + private static String nameOf(UnresolvedExpression expression) { + UnresolvedExpression read = expression; + while (true) { + switch (read) { + case Alias alias -> read = alias.getDelegated(); + case Let let -> read = let.getExpression(); + case Cast cast -> read = cast.getExpression(); + case Field field -> read = field.getField(); + // typeof asks what a leaf is, not what it holds, so it reads nothing + case Function function + when "typeof".equalsIgnoreCase(function.getFuncName()) + && function.getFuncArgs().size() == 1 -> + read = function.getFuncArgs().get(0); + case QualifiedName name -> { + return name.toString(); + } + default -> { + return null; + } + } + } + } + + private static ErrorReport refuse(String message, Object... args) { + return ErrorReport.wrap(new IllegalArgumentException(StringUtils.format(message, args))) + .code(ErrorCode.UNSUPPORTED_OPERATION) + .stage(QueryProcessingStage.ANALYZING) + .location("while checking what a flat_object field can answer") + .suggestion(SUGGESTION) + .build(); + } + + /** What {@link #allowRead} and {@link #as} wrap: the resolution the command was about to do. */ + public interface Reader { + T get(); + } +} diff --git a/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java b/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java deleted file mode 100644 index 5f6e3f11146..00000000000 --- a/core/src/test/java/org/opensearch/sql/calcite/FlatObjectScopeValidatorTest.java +++ /dev/null @@ -1,211 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.calcite; - -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.TYPE_FACTORY; - -import com.google.common.collect.ImmutableList; -import java.math.BigDecimal; -import java.util.ArrayList; -import java.util.List; -import java.util.Set; -import org.apache.calcite.plan.RelOptCluster; -import org.apache.calcite.plan.volcano.VolcanoPlanner; -import org.apache.calcite.rel.RelCollations; -import org.apache.calcite.rel.RelFieldCollation; -import org.apache.calcite.rel.RelNode; -import org.apache.calcite.rel.core.AggregateCall; -import org.apache.calcite.rel.logical.LogicalAggregate; -import org.apache.calcite.rel.logical.LogicalFilter; -import org.apache.calcite.rel.logical.LogicalProject; -import org.apache.calcite.rel.logical.LogicalSort; -import org.apache.calcite.rel.logical.LogicalValues; -import org.apache.calcite.rel.type.RelDataType; -import org.apache.calcite.rex.RexBuilder; -import org.apache.calcite.rex.RexNode; -import org.apache.calcite.sql.fun.SqlStdOperatorTable; -import org.apache.calcite.sql.type.SqlTypeName; -import org.apache.calcite.util.ImmutableBitSet; -import org.junit.jupiter.api.Test; -import org.opensearch.sql.common.error.ErrorCode; -import org.opensearch.sql.common.error.ErrorReport; - -/** - * What a query may do with a flat_object field: read a leaf, or the whole field. Everything else is - * rejected while the plan is built. - * - *

The plans here are built as the commands build them: the field is a column of type {@code - * MAP}, and a leaf of one is {@code ITEM(, '')}. - */ -class FlatObjectScopeValidatorTest { - - private final RexBuilder rexBuilder = new RexBuilder(TYPE_FACTORY); - private final RelOptCluster cluster = RelOptCluster.create(new VolcanoPlanner(), rexBuilder); - - /** A row of two columns: `service`, a keyword, and `attributes`, a flat_object field. */ - private final RelDataType rowType = - TYPE_FACTORY - .builder() - .add("service", TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR)) - .add( - "attributes", - TYPE_FACTORY.createMapType( - TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR), - TYPE_FACTORY.createTypeWithNullability( - TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR), true), - true)) - .build(); - - private RelNode scan() { - return LogicalValues.createEmpty(cluster, rowType); - } - - private RexNode field(int index) { - return rexBuilder.makeInputRef(rowType.getFieldList().get(index).getType(), index); - } - - /** {@code attributes.}, as the resolver builds it. */ - private RexNode leaf(String path) { - return rexBuilder.makeCall(SqlStdOperatorTable.ITEM, field(1), rexBuilder.makeLiteral(path)); - } - - private RelNode project(RexNode... expressions) { - List list = List.of(expressions); - List names = new ArrayList<>(); - for (int i = 0; i < list.size(); i++) { - names.add("c" + i); - } - return LogicalProject.create(scan(), ImmutableList.of(), list, names); - } - - /** The field is found by its mapping type in production; the check itself takes the names. */ - private static final Set FIELDS = Set.of("attributes"); - - private void allowed(RelNode plan) { - assertDoesNotThrow(() -> FlatObjectScopeValidator.validate(plan, FIELDS)); - } - - /** The message says what this query did; the rule and the doc link ride on the report. */ - private void rejected(RelNode plan, String what) { - ErrorReport ex = - assertThrows(ErrorReport.class, () -> FlatObjectScopeValidator.validate(plan, FIELDS)); - assertTrue(ex.getMessage().startsWith(what), ex.getMessage()); - assertEquals(ErrorCode.UNSUPPORTED_OPERATION, ex.getCode()); - assertTrue(ex.getSuggestion().contains("can only be read"), ex.getSuggestion()); - assertTrue( - ex.getSuggestion().contains("supported-field-types/flat-object/"), ex.getSuggestion()); - } - - // ---- reading - - @Test - void readingAFieldALeafOrAnotherColumn() { - allowed(project(field(1))); - allowed(project(leaf("namespace"))); - allowed(project(field(0), leaf("duration_ms"))); - } - - // A leaf is text, so converting it is the parse a caller would do anyway, and the value comes - // from _source either way -- this is how a query asks for a leaf as a number. - @Test - void castingALeafIsReading() { - RexNode asDouble = - rexBuilder.makeCast( - TYPE_FACTORY.createSqlType(SqlTypeName.DOUBLE), leaf("duration_ms"), true, true); - allowed(project(asDouble)); - } - - @Test - void aPlanThatDoesNotTouchTheFieldIsUntouched() { - allowed(project(field(0))); - allowed(LogicalSort.create(scan(), RelCollations.of(new RelFieldCollation(0)), null, null)); - } - - // ---- everything else - - @Test - void computingAValueFromALeafIsRejected() { - RexNode plusOne = - rexBuilder.makeCall( - SqlStdOperatorTable.PLUS, - rexBuilder.makeCast( - TYPE_FACTORY.createSqlType(SqlTypeName.DOUBLE), leaf("d"), true, true), - rexBuilder.makeExactLiteral(BigDecimal.ONE)); - rejected(project(plusOne), "Cannot compute a value from attributes.d"); - } - - @Test - void filteringByALeafIsRejected() { - RexNode equals = - rexBuilder.makeCall( - SqlStdOperatorTable.EQUALS, leaf("namespace"), rexBuilder.makeLiteral("prod")); - rejected(LogicalFilter.create(scan(), equals), "Cannot filter by attributes.namespace"); - } - - // A command that sorts or groups by a leaf projects it into a column of its own first, so the - // check has to follow that column back to the leaf it was read from. - @Test - void sortingByALeafIsRejected() { - RelNode projected = project(field(0), leaf("duration_ms")); - rejected( - LogicalSort.create(projected, RelCollations.of(new RelFieldCollation(1)), null, null), - "Cannot sort by attributes.duration_ms"); - rejected( - LogicalSort.create( - project(field(1)), RelCollations.of(new RelFieldCollation(0)), null, null), - "Cannot sort by attributes"); - } - - // Reading a leaf as a number is allowed, so a cast is stripped when deciding that -- and it has - // to - // be stripped just the same when noting which column the read went into. Otherwise the column - // sorts by a leaf unchecked and the failure surfaces from pushdown, as a 500. - @Test - void sortingByACastLeafIsRejected() { - RexNode asDouble = - rexBuilder.makeCast( - TYPE_FACTORY.createSqlType(SqlTypeName.DOUBLE), leaf("duration_ms"), true, true); - rejected( - LogicalSort.create( - project(field(0), asDouble), RelCollations.of(new RelFieldCollation(1)), null, null), - "Cannot sort by attributes.duration_ms"); - } - - @Test - void groupingByALeafIsRejected() { - RelNode projected = project(field(0), leaf("status_code")); - rejected( - LogicalAggregate.create( - projected, ImmutableList.of(), ImmutableBitSet.of(1), null, ImmutableList.of()), - "Cannot stats by attributes.status_code"); - } - - @Test - void aggregatingALeafIsRejected() { - RelNode projected = project(field(0), leaf("duration_ms")); - AggregateCall count = - AggregateCall.create( - SqlStdOperatorTable.COUNT, - false, - false, - false, - ImmutableList.of(), - ImmutableList.of(1), - -1, - null, - RelCollations.EMPTY, - TYPE_FACTORY.createSqlType(SqlTypeName.BIGINT), - "c"); - rejected( - LogicalAggregate.create( - projected, ImmutableList.of(), ImmutableBitSet.of(), null, ImmutableList.of(count)), - "Cannot stats by attributes.duration_ms"); - } -} diff --git a/core/src/test/java/org/opensearch/sql/calcite/utils/FlatObjectScopeTest.java b/core/src/test/java/org/opensearch/sql/calcite/utils/FlatObjectScopeTest.java new file mode 100644 index 00000000000..d15b5ef5d64 --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/calcite/utils/FlatObjectScopeTest.java @@ -0,0 +1,207 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.calcite.utils; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.opensearch.sql.ast.dsl.AstDSL.field; +import static org.opensearch.sql.ast.dsl.AstDSL.function; +import static org.opensearch.sql.ast.dsl.AstDSL.intLiteral; +import static org.opensearch.sql.ast.dsl.AstDSL.let; + +import java.sql.Connection; +import org.apache.calcite.rex.RexBuilder; +import org.apache.calcite.tools.FrameworkConfig; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.junit.jupiter.MockitoExtension; +import org.opensearch.sql.ast.expression.Cast; +import org.opensearch.sql.ast.expression.UnresolvedExpression; +import org.opensearch.sql.calcite.CalcitePlanContext; +import org.opensearch.sql.calcite.SysLimit; +import org.opensearch.sql.calcite.utils.CalciteToolsHelper.OpenSearchRelBuilder; +import org.opensearch.sql.common.error.ErrorCode; +import org.opensearch.sql.common.error.ErrorReport; +import org.opensearch.sql.executor.QueryType; + +/** The uses a {@code flat_object} field allows, and the refusal for everything else. */ +@ExtendWith(MockitoExtension.class) +class FlatObjectScopeTest { + + @Mock private OpenSearchRelBuilder relBuilder; + @Mock private MockedStatic mockedToolsHelper; + + private CalcitePlanContext context; + + @BeforeEach + void setUp() { + mockedToolsHelper + .when(() -> CalciteToolsHelper.connect(any(), any())) + .thenReturn(mock(Connection.class)); + mockedToolsHelper + .when(() -> CalciteToolsHelper.create(any(), any(), any())) + .thenReturn(relBuilder); + when(relBuilder.getRexBuilder()).thenReturn(new RexBuilder(OpenSearchTypeFactory.TYPE_FACTORY)); + context = + CalcitePlanContext.create(mock(FrameworkConfig.class), SysLimit.DEFAULT, QueryType.PPL); + context.getFlatObjectFields().add("attributes"); + } + + /** What a command does when it resolves a name, with no read permission open. */ + private ErrorReport refused(String name) { + return assertThrows(ErrorReport.class, () -> FlatObjectScope.check(name, context)); + } + + // ---- what is allowed + + @Test + void aFieldOrALeafMayBeRead() { + FlatObjectScope.allowRead( + field("attributes.duration_ms"), + "attributes.duration_ms", + context, + () -> { + FlatObjectScope.check("attributes.duration_ms", context); + return null; + }); + FlatObjectScope.allowRead( + field("attributes"), + "attributes", + context, + () -> { + FlatObjectScope.check("attributes", context); + return null; + }); + } + + /** A read may wear a cast: that is how a query asks for a leaf as a number. */ + @Test + void aReadMayBeCast() { + UnresolvedExpression asDouble = + let( + field("d"), + new Cast( + field("attributes.duration_ms"), + org.opensearch.sql.ast.dsl.AstDSL.stringLiteral("double"))); + assertDoesNotThrow( + () -> + FlatObjectScope.allowRead( + asDouble, + "d", + context, + () -> { + FlatObjectScope.check("attributes.duration_ms", context); + return null; + })); + } + + /** typeof asks what a leaf is, not what it holds, so it reads nothing from the record. */ + @Test + void typeofIsAllowed() { + UnresolvedExpression typeOfLeaf = + let(field("t"), function("typeof", field("attributes.duration_ms"))); + assertDoesNotThrow( + () -> + FlatObjectScope.allowRead( + typeOfLeaf, + "t", + context, + () -> { + FlatObjectScope.check("attributes.duration_ms", context); + return null; + })); + } + + @Test + void aNameOutsideTheFieldIsNotTouched() { + assertDoesNotThrow(() -> FlatObjectScope.check("service", context)); + assertDoesNotThrow(() -> FlatObjectScope.check("attributesX", context)); + } + + // ---- what is refused + + @Test + void aValueComputedFromALeafIsRefused() { + UnresolvedExpression plusOne = + let(field("x"), function("+", field("attributes.duration_ms"), intLiteral(1))); + ErrorReport error = + assertThrows( + ErrorReport.class, + () -> + FlatObjectScope.allowRead( + plusOne, + "x", + context, + () -> { + FlatObjectScope.check("attributes.duration_ms", context); + return null; + })); + assertTrue( + error.getMessage().contains("Cannot compute a value from attributes.duration_ms"), + error.getMessage()); + } + + @Test + void theCommandNamesWhatItWasDoing() { + FlatObjectScope.runAs( + "sort by", + context, + () -> + assertTrue( + refused("attributes.duration_ms") + .getMessage() + .contains("Cannot sort by attributes.duration_ms"))); + } + + /** + * A command that reads a leaf into a column of its own does not escape the rule: pushdown fuses + * the projection back into whatever reads that column. + */ + @Test + void aColumnReadFromALeafIsRefusedToo() { + FlatObjectScope.allowRead( + let(field("d"), field("attributes.duration_ms")), "d", context, () -> null); + FlatObjectScope.runAs( + "sort by", + context, + () -> + assertTrue( + refused("d").getMessage().contains("Cannot sort by attributes.duration_ms"), + "the refusal names the leaf, not the column the query gave it")); + } + + @Test + void aRenameCarriesTheColumnForward() { + FlatObjectScope.allowRead( + let(field("d"), field("attributes.duration_ms")), "d", context, () -> null); + FlatObjectScope.trackRename("d", "e", context); + FlatObjectScope.runAs( + "stats by", + context, + () -> + assertTrue( + refused("e").getMessage().contains("Cannot stats by attributes.duration_ms"), + "a renamed column is still named by the leaf it was read from")); + } + + @Test + void theRefusalCarriesTheRuleAndTheDocumentation() { + ErrorReport error = refused("attributes.duration_ms"); + assertEquals(ErrorCode.UNSUPPORTED_OPERATION, error.getCode()); + assertTrue(error.getSuggestion().contains("can only be read"), error.getSuggestion()); + assertTrue( + error.getSuggestion().contains("supported-field-types/flat-object/"), + error.getSuggestion()); + } +} From c80763b86b15a4d6c90241e86505f0806a48984e Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Wed, 30 Sep 2026 14:33:41 -0700 Subject: [PATCH 7/8] Shorten the comments and drop what nothing calls The class javadoc repeated in three paragraphs what one says, several method comments restated their signature, and isFlatObjectField carried a javadoc describing parameters it does not have -- left from an earlier version of the check. It and isBareRead had no callers, and rootedInFlatObject folds into isFlatObject. 230 lines to 172; behaviour unchanged over the 22 queries. Signed-off-by: Jialiang Liang --- .../sql/calcite/CalcitePlanContext.java | 15 +- .../sql/calcite/CalciteRelNodeVisitor.java | 12 +- .../sql/calcite/QualifiedNameResolver.java | 4 +- .../sql/calcite/utils/FlatObjectScope.java | 160 ++++++------------ 4 files changed, 65 insertions(+), 126 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java b/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java index cc57e2ea1b7..0a25094e5b8 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java @@ -100,24 +100,21 @@ public class CalcitePlanContext { @Getter @Setter private boolean includeMetadata = false; /** - * Dotted names of the {@code flat_object} fields on the relations this query scans, recorded when - * each relation is bound -- the one point where the index mapping is in hand. {@link - * FlatObjectScopeValidator} runs over the finished plan, by which time a scan may sit behind - * rewrites that no longer carry the table. + * The {@code flat_object} fields on the relations this query scans, by dotted name, recorded as + * each relation is bound -- the one point where the index mapping is in hand. */ @Getter private final Set flatObjectFields = new LinkedHashSet<>(); /** - * Columns a command read a {@code flat_object} field or one of its leaves into, by the name the - * command gave them. A later command using such a column is refused the same way, since pushdown - * fuses the projection back into whatever reads it. + * Columns holding a {@code flat_object} read, mapped to the field they read. A later command + * using one is refused too: pushdown fuses the projection back into whatever reads it. */ @Getter private final Map flatObjectLeafColumns = new LinkedHashMap<>(); - /** Whether the expression being resolved is one of the reads a flat_object field allows. */ + /** Whether the expression being resolved is one of the reads a flat_object allows. */ @Getter @Setter private boolean flatObjectReadAllowed = false; - /** What the command being analyzed would do with a flat_object field, for the refusal message. */ + /** What the command would do with a flat_object, for the refusal message. */ @Getter @Setter private String flatObjectUse = "use"; private final Stack correlVar = new Stack<>(); 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 2ad8c3a7b28..0d152d7c5ed 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java @@ -248,10 +248,10 @@ public RelNode analyze(UnresolvedPlan unresolved, CalcitePlanContext context) { public RelNode visitChildren(Node node, CalcitePlanContext context) { RelNode result = super.visitChildren(node, context); if (node instanceof UnresolvedPlan plan) { - // Materializing a dotted path is a read, so it is allowed -- and it has to be, because the - // materializer wraps field resolution in catch (RuntimeException | AssertionError), which - // would swallow a refusal raised there and leave the command quietly ignoring the field. - // What the command then does with the materialized column is checked where it resolves it. + // Materializing a dotted path is a read. It has to be resolved as one: the materializer + // catches RuntimeException, so a refusal raised inside would be swallowed and the field + // quietly ignored. What the command then does with the column is checked where it resolves + // it. FlatObjectScope.runAsRead(context, () -> mapPathMaterializer.materializePaths(plan, context)); } return result; @@ -4504,8 +4504,8 @@ public RelNode visitExpand(Expand expand, CalcitePlanContext context) { // 2. Get the field to expand and an optional alias. Field arrayField = expand.getField(); - // Resolve as a read: the check below already refuses a flat_object leaf, by the same rule it - // uses for an object leaf or an expression, and says so in expand's own words. + // Resolve as a read: the check below already refuses a flat_object leaf, by the rule it uses + // for an object leaf or an expression, and in expand's own words. RexNode resolved = FlatObjectScope.allowReadOf(context, () -> rexVisitor.analyze(arrayField, context)); if (!(resolved instanceof RexInputRef arrayFieldRex)) { 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 1250daee259..699ede0d7ab 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/QualifiedNameResolver.java +++ b/core/src/main/java/org/opensearch/sql/calcite/QualifiedNameResolver.java @@ -83,8 +83,8 @@ private static RexNode resolveInNonJoinCondition( .or(() -> resolveRenamedField(nameNode, context)); if (fieldRef.isPresent()) { - // Every way of resolving a field converges here, so this is where a flat_object used outside - // what the field type answers is refused -- see FlatObjectScope. + // Every way of resolving a field converges here, so a flat_object used for anything the + // field type cannot answer is refused here. See FlatObjectScope. FlatObjectScope.check(nameNode.toString(), context); // If we're in a lambda context and this is not a lambda variable, // we need to capture it as an external variable diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/FlatObjectScope.java b/core/src/main/java/org/opensearch/sql/calcite/utils/FlatObjectScope.java index 0c788d646d9..e6b925a2885 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/utils/FlatObjectScope.java +++ b/core/src/main/java/org/opensearch/sql/calcite/utils/FlatObjectScope.java @@ -5,7 +5,6 @@ package org.opensearch.sql.calcite.utils; -import java.util.Set; import org.opensearch.sql.ast.expression.Alias; import org.opensearch.sql.ast.expression.Cast; import org.opensearch.sql.ast.expression.Field; @@ -20,81 +19,49 @@ import org.opensearch.sql.common.utils.StringUtils; /** - * What a query may do with a {@code flat_object} field: a list of the allowed uses, everything else - * refused. + * The uses a {@code flat_object} field allows: read a leaf, read the whole field, or ask its type. + * A read may wear a cast, which is how a query asks for a leaf as a number. Everything else is + * refused, because the index files each leaf as one folded {@code path=value} term and answering + * anything more would mean opening {@code _source} for every record. * - *

A flat_object gives its leaves no mapping of their own. The index files every leaf value as - * one folded {@code path=value} keyword term, so the queries the field type answers are the ones - * that match a term -- the list under Supported - * queries. Anything else would have to open {@code _source} for every record it is asked about, - * the cost the field type exists to avoid, so it is refused while the plan is built rather than - * answered slowly. - * - *

Today the list is: read a leaf, or read the whole field. A read may wear a cast, which is how - * a query asks for a leaf as a number. - * - *

The refusal is raised from {@code QualifiedNameResolver}, where every reference to a field - * passes, so a command that is not on the list needs no entry here to be refused -- including one - * added later. {@code CalciteRelNodeVisitor} opens the permission for the reads above and closes it - * again, and names the command it was doing for the message. + *

{@link #check} runs where a field reference resolves, so a command not on the list is refused + * without an entry of its own. {@code CalciteRelNodeVisitor} opens the permission for the reads and + * names what it was doing for the message. */ public final class FlatObjectScope { - private static final String DOC_URL = - "https://docs.opensearch.org/latest/mappings/supported-field-types/flat-object/"; - private static final String SUGGESTION = "A flat_object field can only be read. Its leaves have no mapping of their own, so nothing" - + " else can be answered without opening every record. See " - + DOC_URL; + + " else can be answered without opening every record. See" + + " https://docs.opensearch.org/latest/mappings/supported-field-types/flat-object/"; private FlatObjectScope() {} - /** - * Whether {@code name} is a flat_object field or a leaf under one, or a column a read of either - * went into. The mapping types are recorded as each relation is bound; the columns as each read - * is allowed. - */ - public static boolean isFlatObject(String name, CalcitePlanContext context) { - return context.getFlatObjectFields().contains(name) - || context.getFlatObjectLeafColumns().containsKey(name); - } - - /** Whether any part of {@code parts} up to {@code length} names a flat_object field. */ - public static boolean isFlatObjectField(String fieldName, CalcitePlanContext context) { - return context.getFlatObjectFields().contains(fieldName); - } - - /** - * Refuses the use unless the command being analyzed opened the permission. {@code name} is the - * field or leaf as the query spelled it. - */ + /** Refuses {@code name} unless the command opened the permission. */ public static void check(String name, CalcitePlanContext context) { - if (context.isFlatObjectReadAllowed() || !rootedInFlatObject(name, context)) { + if (context.isFlatObjectReadAllowed() || !isFlatObject(name, context)) { return; } throw refuse("Cannot %s %s", context.getFlatObjectUse(), originOf(name, context)); } /** - * Runs {@code read} with the permission open, and notes the column a read of a flat_object went - * into so a later command that uses that column is refused too. {@code target} is the name the - * command gives the result, or null when it does not name one. + * Resolves {@code expression}, permitted when it is one of the allowed reads. {@code target} is + * the column the command puts it in, remembered so a later use of that column is refused too. */ public static T allowRead( UnresolvedExpression expression, String target, CalcitePlanContext context, Reader read) { - boolean isRead = nameOf(expression) != null; + String name = nameOf(expression); boolean previousAllowed = context.isFlatObjectReadAllowed(); String previousUse = context.getFlatObjectUse(); - context.setFlatObjectReadAllowed(isRead); - if (!isRead) { + context.setFlatObjectReadAllowed(name != null); + if (name == null) { context.setFlatObjectUse("compute a value from"); } try { T resolved = read.get(); - if (isRead && target != null && namesFlatObject(expression, context)) { - context.getFlatObjectLeafColumns().put(target, nameOf(expression)); + if (target != null && name != null && isFlatObject(name, context)) { + context.getFlatObjectLeafColumns().put(target, originOf(name, context)); } return resolved; } finally { @@ -103,52 +70,39 @@ public static T allowRead( } } - /** Runs {@code read} while naming what the command would be doing, for the refusal message. */ - public static T as(String use, CalcitePlanContext context, Reader read) { - String previous = context.getFlatObjectUse(); - context.setFlatObjectUse(use); + /** Resolves {@code read} as a read, for a step that is one by construction. */ + public static T allowReadOf(CalcitePlanContext context, Reader read) { + boolean previous = context.isFlatObjectReadAllowed(); + context.setFlatObjectReadAllowed(true); try { return read.get(); } finally { - context.setFlatObjectUse(previous); + context.setFlatObjectReadAllowed(previous); } } - /** - * Runs {@code read} with the permission open, for a step that is a read by construction rather - * than by the shape of an expression. - */ + /** {@link #allowReadOf} for a step that returns nothing. */ public static void runAsRead(CalcitePlanContext context, Runnable read) { - boolean previous = context.isFlatObjectReadAllowed(); - context.setFlatObjectReadAllowed(true); - try { - read.run(); - } finally { - context.setFlatObjectReadAllowed(previous); - } + allowReadOf( + context, + () -> { + read.run(); + return null; + }); } - /** {@link #runAsRead} for a resolution that returns a value. */ - public static T allowReadOf(CalcitePlanContext context, Reader read) { - boolean previous = context.isFlatObjectReadAllowed(); - context.setFlatObjectReadAllowed(true); + /** Resolves {@code read} while naming what the command is doing, for the message. */ + public static T as(String use, CalcitePlanContext context, Reader read) { + String previous = context.getFlatObjectUse(); + context.setFlatObjectUse(use); try { return read.get(); } finally { - context.setFlatObjectReadAllowed(previous); - } - } - - /** - * Notes that {@code target} now holds what {@code source} held, so a use of it is refused too. - */ - public static void trackRename(String source, String target, CalcitePlanContext context) { - if (rootedInFlatObject(source, context)) { - context.getFlatObjectLeafColumns().put(target, originOf(source, context)); + context.setFlatObjectUse(previous); } } - /** {@link #as} for a resolution that returns nothing. */ + /** {@link #as} for a step that returns nothing. */ public static void runAs(String use, CalcitePlanContext context, Runnable read) { as( use, @@ -159,38 +113,26 @@ public static void runAs(String use, CalcitePlanContext context, Runnable read) }); } - /** The field a name refers to: a column read from a leaf is named by that leaf, not by itself. */ - private static String originOf(String name, CalcitePlanContext context) { - return context.getFlatObjectLeafColumns().getOrDefault(name, name); - } - - /** Whether {@code name} is a flat_object field, a leaf under one, or a column read from one. */ - private static boolean rootedInFlatObject(String name, CalcitePlanContext context) { - if (isFlatObject(name, context)) { - return true; - } - Set fields = context.getFlatObjectFields(); - for (String field : fields) { - if (name.startsWith(field + ".")) { - return true; - } + /** Carries the fact that a column holds a leaf over to the name it is renamed to. */ + public static void trackRename(String source, String target, CalcitePlanContext context) { + if (isFlatObject(source, context)) { + context.getFlatObjectLeafColumns().put(target, originOf(source, context)); } - return false; } - /** Whether the expression is nothing but a field reference, optionally cast. */ - private static boolean isBareRead(UnresolvedExpression expression) { - return nameOf(expression) != null; + /** Whether the name is a flat_object field, a leaf under one, or a column read from either. */ + private static boolean isFlatObject(String name, CalcitePlanContext context) { + return context.getFlatObjectFields().contains(name) + || context.getFlatObjectLeafColumns().containsKey(name) + || context.getFlatObjectFields().stream().anyMatch(field -> name.startsWith(field + ".")); } - /** Whether the expression is a bare read of a flat_object field or a leaf under one. */ - private static boolean namesFlatObject( - UnresolvedExpression expression, CalcitePlanContext context) { - String name = nameOf(expression); - return name != null && rootedInFlatObject(name, context); + /** A column read from a leaf is named by that leaf; any other name is its own. */ + private static String originOf(String name, CalcitePlanContext context) { + return context.getFlatObjectLeafColumns().getOrDefault(name, name); } - /** The field a bare read names, with the aliases and casts around it removed; null otherwise. */ + /** The field an allowed read names, or null when the expression is not one. */ private static String nameOf(UnresolvedExpression expression) { UnresolvedExpression read = expression; while (true) { @@ -199,7 +141,7 @@ private static String nameOf(UnresolvedExpression expression) { case Let let -> read = let.getExpression(); case Cast cast -> read = cast.getExpression(); case Field field -> read = field.getField(); - // typeof asks what a leaf is, not what it holds, so it reads nothing + // typeof asks what a leaf is, not what it holds case Function function when "typeof".equalsIgnoreCase(function.getFuncName()) && function.getFuncArgs().size() == 1 -> @@ -223,7 +165,7 @@ private static ErrorReport refuse(String message, Object... args) { .build(); } - /** What {@link #allowRead} and {@link #as} wrap: the resolution the command was about to do. */ + /** The resolution a command was about to do. */ public interface Reader { T get(); } From f47b0bb2cd16c9162bbdc6b6326c77aff2b4ec86 Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Wed, 30 Sep 2026 15:47:27 -0700 Subject: [PATCH 8/8] Let a flat_object leaf behave like the text column it is A leaf was readable and nothing more: a check in the analysis stage refused every other use, and RexStandardizer refused to build a script over one. Both are gone. PPL already has a field of exactly this shape -- one spath extracts from a JSON string -- and puts no restriction on it: the value is text, a path holding several values is their JSON, and filter, sort, stats and arithmetic all work. Measured on a spath field, `stats avg` over "12.5" and "4" gives 8.25 and `where tags = 'x'` over ["x","y"] matches nothing. A flat_object leaf now gives the same answers for the same shapes, so there is one rule to learn rather than two, and no list of refused commands to maintain. What the refusal was protecting is not specific to this field type either. A predicate on any multi-valued field answers differently depending on whether it reached the index -- `where k = 'a'` on a plain keyword field holding ["a","b"] returns nothing once a `head` blocks pushdown -- so it belongs to the comparison, not here. Kept: the expand check, which reports a leaf the way the mvexpand check does rather than as the ClassCastException the cast used to throw. Signed-off-by: Jialiang Liang --- .../sql/calcite/CalcitePlanContext.java | 21 -- .../sql/calcite/CalciteRelNodeVisitor.java | 98 ++------- .../sql/calcite/QualifiedNameResolver.java | 4 - .../sql/calcite/utils/FlatObjectScope.java | 172 --------------- .../calcite/utils/FlatObjectScopeTest.java | 207 ------------------ docs/user/ppl/general/datatypes.md | 15 +- .../rest-api-spec/test/issues/1604.yml | 33 +-- .../storage/serde/RexStandardizer.java | 13 +- 8 files changed, 48 insertions(+), 515 deletions(-) delete mode 100644 core/src/main/java/org/opensearch/sql/calcite/utils/FlatObjectScope.java delete mode 100644 core/src/test/java/org/opensearch/sql/calcite/utils/FlatObjectScopeTest.java diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java b/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java index 0a25094e5b8..0fbefa4a17a 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java @@ -10,12 +10,9 @@ import java.sql.Connection; import java.util.ArrayList; import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.Stack; import java.util.function.BiFunction; import lombok.Getter; @@ -99,24 +96,6 @@ public class CalcitePlanContext { */ @Getter @Setter private boolean includeMetadata = false; - /** - * The {@code flat_object} fields on the relations this query scans, by dotted name, recorded as - * each relation is bound -- the one point where the index mapping is in hand. - */ - @Getter private final Set flatObjectFields = new LinkedHashSet<>(); - - /** - * Columns holding a {@code flat_object} read, mapped to the field they read. A later command - * using one is refused too: pushdown fuses the projection back into whatever reads it. - */ - @Getter private final Map flatObjectLeafColumns = new LinkedHashMap<>(); - - /** Whether the expression being resolved is one of the reads a flat_object allows. */ - @Getter @Setter private boolean flatObjectReadAllowed = false; - - /** What the command would do with a flat_object, for the refusal message. */ - @Getter @Setter private String flatObjectUse = "use"; - private final Stack correlVar = new Stack<>(); private final Stack> windowPartitions = new Stack<>(); 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 0d152d7c5ed..2cad91d7018 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java @@ -181,7 +181,6 @@ import org.opensearch.sql.calcite.plan.rel.LogicalSystemLimit; import org.opensearch.sql.calcite.plan.rel.LogicalSystemLimit.SystemLimitType; import org.opensearch.sql.calcite.utils.BinUtils; -import org.opensearch.sql.calcite.utils.FlatObjectScope; import org.opensearch.sql.calcite.utils.JoinAndLookupUtils; import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory; import org.opensearch.sql.calcite.utils.PPLHintUtils; @@ -238,21 +237,14 @@ public CalciteRelNodeVisitor(DataSourceService dataSourceService) { } public RelNode analyze(UnresolvedPlan unresolved, CalcitePlanContext context) { - RelNode plan = unresolved.accept(this, context); - // Here rather than in QueryService.analyze, so the unified query pipeline -- which calls this - // method directly -- is held to the same rules. - return plan; + return unresolved.accept(this, context); } @Override public RelNode visitChildren(Node node, CalcitePlanContext context) { RelNode result = super.visitChildren(node, context); if (node instanceof UnresolvedPlan plan) { - // Materializing a dotted path is a read. It has to be resolved as one: the materializer - // catches RuntimeException, so a refusal raised inside would be swallowed and the field - // quietly ignored. What the command then does with the column is checked where it resolves - // it. - FlatObjectScope.runAsRead(context, () -> mapPathMaterializer.materializePaths(plan, context)); + mapPathMaterializer.materializePaths(plan, context); } return result; } @@ -276,7 +268,6 @@ public RelNode visitRelation(Relation node, CalcitePlanContext context) { } context.relBuilder.scan(node.getTableQualifiedName().getParts()); RelNode scan = context.relBuilder.peek(); - recordFlatObjectFields(scan, context); // Eagerly push down highlight config to the scan (highlight is a scan hint, not an operator) if (context.getHighlightConfig() != null && scan instanceof HighlightPushDown) { @@ -294,28 +285,6 @@ public RelNode visitRelation(Relation node, CalcitePlanContext context) { return context.relBuilder.peek(); } - /** - * Notes which of a relation's fields are {@code flat_object}, for {@link - * FlatObjectScopeValidator}. Their leaves have no mapping of their own, so the check cannot read - * the type off the plan -- and here is where the index mapping is in hand. - */ - private static void recordFlatObjectFields(RelNode scan, CalcitePlanContext context) { - RelOptTable relOptTable = scan.getTable(); - AbstractOpenSearchTable osTable = - relOptTable == null ? null : relOptTable.unwrap(AbstractOpenSearchTable.class); - if (osTable == null) { - return; - } - osTable - .getFieldTypes() - .forEach( - (name, type) -> { - if ("flat_object".equalsIgnoreCase(type.legacyTypeName())) { - context.getFlatObjectFields().add(name); - } - }); - } - // This is a tool method to add an existed RelOptTable to builder stack, not used for now private RelBuilder scan(RelOptTable tableSchema, CalcitePlanContext context) { final RelNode scan = @@ -373,9 +342,7 @@ public RelNode visitFilter(Filter node, CalcitePlanContext context) { context.relBuilder.variable(v::set); context.pushCorrelVar(v.get()); } - RexNode condition = - FlatObjectScope.as( - "filter by", context, () -> rexVisitor.analyze(node.getCondition(), context)); + RexNode condition = rexVisitor.analyze(node.getCondition(), context); if (containsSubqueryExpression) { context.relBuilder.filter(ImmutableList.of(v.get().id), condition); context.popCorrelVar(); @@ -618,9 +585,7 @@ private List expandProjectFields( } matchingFields.forEach(f -> expandedFields.add(context.relBuilder.field(f))); } else if (addedFields.add(fieldName)) { - RexNode resolved = - FlatObjectScope.allowRead( - field, fieldName, context, () -> rexVisitor.analyze(field, context)); + 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. @@ -809,7 +774,6 @@ public RelNode visitRename(Rename node, CalcitePlanContext context) { String newName = WildcardRenameUtils.applyWildcardTransformation( sourcePattern, targetPattern, fieldName); - FlatObjectScope.trackRename(fieldName, newName, context); if (newNames.contains(newName) && !newName.equals(fieldName)) { removeFieldIfExists(newName, newNames, context); } @@ -838,27 +802,23 @@ private void removeFieldIfExists( public RelNode visitSort(org.opensearch.sql.ast.tree.Sort node, CalcitePlanContext context) { visitChildren(node, context); List sortList = - FlatObjectScope.as( - "sort by", - context, - () -> - node.getSortList().stream() - .map( - expr -> { - RexNode sortField = rexVisitor.analyze(expr, context); - SortOption sortOption = analyzeSortOption(expr.getFieldArgs()); - // Default is ASC - if (sortOption.getSortOrder() == DESC) { - sortField = context.relBuilder.desc(sortField); - } - if (sortOption.getNullOrder() == NULL_LAST) { - sortField = context.relBuilder.nullsLast(sortField); - } else { - sortField = context.relBuilder.nullsFirst(sortField); - } - return sortField; - }) - .collect(Collectors.toList())); + node.getSortList().stream() + .map( + expr -> { + RexNode sortField = rexVisitor.analyze(expr, context); + SortOption sortOption = analyzeSortOption(expr.getFieldArgs()); + // Default is ASC + if (sortOption.getSortOrder() == DESC) { + sortField = context.relBuilder.desc(sortField); + } + if (sortOption.getNullOrder() == NULL_LAST) { + sortField = context.relBuilder.nullsLast(sortField); + } else { + sortField = context.relBuilder.nullsFirst(sortField); + } + return sortField; + }) + .collect(Collectors.toList()); context.relBuilder.sort(sortList); // Apply count parameter as limit if (node.getCount() != 0) { @@ -1305,12 +1265,7 @@ public RelNode visitEval(Eval node, CalcitePlanContext context) { context.relBuilder.variable(v::set); context.pushCorrelVar(v.get()); } - RexNode eval = - FlatObjectScope.allowRead( - expr, - expr.getVar().getField().toString(), - context, - () -> rexVisitor.analyze(expr, context)); + RexNode eval = rexVisitor.analyze(expr, context); if (containsSubqueryExpression) { // RelBuilder.projectPlus doesn't have a parameter with variablesSet: // projectPlus(Iterable variablesSet, RexNode... nodes) @@ -1820,8 +1775,7 @@ public RelNode visitAggregation(Aggregation node, CalcitePlanContext context) { if (!bucketNullable) { nonNullGroupMask.set(0, nGroup); } - FlatObjectScope.runAs( - "stats by", context, () -> visitAggregation(node, context, nonNullGroupMask, true, false)); + visitAggregation(node, context, nonNullGroupMask, true, false); return context.relBuilder.peek(); } @@ -2292,7 +2246,6 @@ public RelNode visitLookup(Lookup node, CalcitePlanContext context) { @Override public RelNode visitDedupe(Dedupe node, CalcitePlanContext context) { - context.setFlatObjectUse("dedup by"); visitChildren(node, context); List options = node.getOptions(); Integer allowedDuplication = (Integer) options.get(0).getValue().getValue(); @@ -4504,10 +4457,7 @@ public RelNode visitExpand(Expand expand, CalcitePlanContext context) { // 2. Get the field to expand and an optional alias. Field arrayField = expand.getField(); - // Resolve as a read: the check below already refuses a flat_object leaf, by the rule it uses - // for an object leaf or an expression, and in expand's own words. - RexNode resolved = - FlatObjectScope.allowReadOf(context, () -> rexVisitor.analyze(arrayField, context)); + RexNode resolved = rexVisitor.analyze(arrayField, context); if (!(resolved instanceof RexInputRef arrayFieldRex)) { // Anything that is not a column of the input -- a leaf of an object or a flat_object field, // an expression -- has no column to correlate the expansion with. Raised the way the 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 699ede0d7ab..c75c0829b5f 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/QualifiedNameResolver.java +++ b/core/src/main/java/org/opensearch/sql/calcite/QualifiedNameResolver.java @@ -17,7 +17,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.sql.ast.expression.QualifiedName; -import org.opensearch.sql.calcite.utils.FlatObjectScope; import org.opensearch.sql.common.error.ErrorCode; import org.opensearch.sql.common.error.ErrorReport; import org.opensearch.sql.common.utils.StringUtils; @@ -83,9 +82,6 @@ private static RexNode resolveInNonJoinCondition( .or(() -> resolveRenamedField(nameNode, context)); if (fieldRef.isPresent()) { - // Every way of resolving a field converges here, so a flat_object used for anything the - // field type cannot answer is refused here. See FlatObjectScope. - FlatObjectScope.check(nameNode.toString(), context); // If we're in a lambda context and this is not a lambda variable, // we need to capture it as an external variable if (context.isInLambdaContext()) { diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/FlatObjectScope.java b/core/src/main/java/org/opensearch/sql/calcite/utils/FlatObjectScope.java deleted file mode 100644 index e6b925a2885..00000000000 --- a/core/src/main/java/org/opensearch/sql/calcite/utils/FlatObjectScope.java +++ /dev/null @@ -1,172 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.calcite.utils; - -import org.opensearch.sql.ast.expression.Alias; -import org.opensearch.sql.ast.expression.Cast; -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.QualifiedName; -import org.opensearch.sql.ast.expression.UnresolvedExpression; -import org.opensearch.sql.calcite.CalcitePlanContext; -import org.opensearch.sql.common.error.ErrorCode; -import org.opensearch.sql.common.error.ErrorReport; -import org.opensearch.sql.common.error.QueryProcessingStage; -import org.opensearch.sql.common.utils.StringUtils; - -/** - * The uses a {@code flat_object} field allows: read a leaf, read the whole field, or ask its type. - * A read may wear a cast, which is how a query asks for a leaf as a number. Everything else is - * refused, because the index files each leaf as one folded {@code path=value} term and answering - * anything more would mean opening {@code _source} for every record. - * - *

{@link #check} runs where a field reference resolves, so a command not on the list is refused - * without an entry of its own. {@code CalciteRelNodeVisitor} opens the permission for the reads and - * names what it was doing for the message. - */ -public final class FlatObjectScope { - - private static final String SUGGESTION = - "A flat_object field can only be read. Its leaves have no mapping of their own, so nothing" - + " else can be answered without opening every record. See" - + " https://docs.opensearch.org/latest/mappings/supported-field-types/flat-object/"; - - private FlatObjectScope() {} - - /** Refuses {@code name} unless the command opened the permission. */ - public static void check(String name, CalcitePlanContext context) { - if (context.isFlatObjectReadAllowed() || !isFlatObject(name, context)) { - return; - } - throw refuse("Cannot %s %s", context.getFlatObjectUse(), originOf(name, context)); - } - - /** - * Resolves {@code expression}, permitted when it is one of the allowed reads. {@code target} is - * the column the command puts it in, remembered so a later use of that column is refused too. - */ - public static T allowRead( - UnresolvedExpression expression, String target, CalcitePlanContext context, Reader read) { - String name = nameOf(expression); - boolean previousAllowed = context.isFlatObjectReadAllowed(); - String previousUse = context.getFlatObjectUse(); - context.setFlatObjectReadAllowed(name != null); - if (name == null) { - context.setFlatObjectUse("compute a value from"); - } - try { - T resolved = read.get(); - if (target != null && name != null && isFlatObject(name, context)) { - context.getFlatObjectLeafColumns().put(target, originOf(name, context)); - } - return resolved; - } finally { - context.setFlatObjectReadAllowed(previousAllowed); - context.setFlatObjectUse(previousUse); - } - } - - /** Resolves {@code read} as a read, for a step that is one by construction. */ - public static T allowReadOf(CalcitePlanContext context, Reader read) { - boolean previous = context.isFlatObjectReadAllowed(); - context.setFlatObjectReadAllowed(true); - try { - return read.get(); - } finally { - context.setFlatObjectReadAllowed(previous); - } - } - - /** {@link #allowReadOf} for a step that returns nothing. */ - public static void runAsRead(CalcitePlanContext context, Runnable read) { - allowReadOf( - context, - () -> { - read.run(); - return null; - }); - } - - /** Resolves {@code read} while naming what the command is doing, for the message. */ - public static T as(String use, CalcitePlanContext context, Reader read) { - String previous = context.getFlatObjectUse(); - context.setFlatObjectUse(use); - try { - return read.get(); - } finally { - context.setFlatObjectUse(previous); - } - } - - /** {@link #as} for a step that returns nothing. */ - public static void runAs(String use, CalcitePlanContext context, Runnable read) { - as( - use, - context, - () -> { - read.run(); - return null; - }); - } - - /** Carries the fact that a column holds a leaf over to the name it is renamed to. */ - public static void trackRename(String source, String target, CalcitePlanContext context) { - if (isFlatObject(source, context)) { - context.getFlatObjectLeafColumns().put(target, originOf(source, context)); - } - } - - /** Whether the name is a flat_object field, a leaf under one, or a column read from either. */ - private static boolean isFlatObject(String name, CalcitePlanContext context) { - return context.getFlatObjectFields().contains(name) - || context.getFlatObjectLeafColumns().containsKey(name) - || context.getFlatObjectFields().stream().anyMatch(field -> name.startsWith(field + ".")); - } - - /** A column read from a leaf is named by that leaf; any other name is its own. */ - private static String originOf(String name, CalcitePlanContext context) { - return context.getFlatObjectLeafColumns().getOrDefault(name, name); - } - - /** The field an allowed read names, or null when the expression is not one. */ - private static String nameOf(UnresolvedExpression expression) { - UnresolvedExpression read = expression; - while (true) { - switch (read) { - case Alias alias -> read = alias.getDelegated(); - case Let let -> read = let.getExpression(); - case Cast cast -> read = cast.getExpression(); - case Field field -> read = field.getField(); - // typeof asks what a leaf is, not what it holds - case Function function - when "typeof".equalsIgnoreCase(function.getFuncName()) - && function.getFuncArgs().size() == 1 -> - read = function.getFuncArgs().get(0); - case QualifiedName name -> { - return name.toString(); - } - default -> { - return null; - } - } - } - } - - private static ErrorReport refuse(String message, Object... args) { - return ErrorReport.wrap(new IllegalArgumentException(StringUtils.format(message, args))) - .code(ErrorCode.UNSUPPORTED_OPERATION) - .stage(QueryProcessingStage.ANALYZING) - .location("while checking what a flat_object field can answer") - .suggestion(SUGGESTION) - .build(); - } - - /** The resolution a command was about to do. */ - public interface Reader { - T get(); - } -} diff --git a/core/src/test/java/org/opensearch/sql/calcite/utils/FlatObjectScopeTest.java b/core/src/test/java/org/opensearch/sql/calcite/utils/FlatObjectScopeTest.java deleted file mode 100644 index d15b5ef5d64..00000000000 --- a/core/src/test/java/org/opensearch/sql/calcite/utils/FlatObjectScopeTest.java +++ /dev/null @@ -1,207 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.calcite.utils; - -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import static org.opensearch.sql.ast.dsl.AstDSL.field; -import static org.opensearch.sql.ast.dsl.AstDSL.function; -import static org.opensearch.sql.ast.dsl.AstDSL.intLiteral; -import static org.opensearch.sql.ast.dsl.AstDSL.let; - -import java.sql.Connection; -import org.apache.calcite.rex.RexBuilder; -import org.apache.calcite.tools.FrameworkConfig; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.MockedStatic; -import org.mockito.junit.jupiter.MockitoExtension; -import org.opensearch.sql.ast.expression.Cast; -import org.opensearch.sql.ast.expression.UnresolvedExpression; -import org.opensearch.sql.calcite.CalcitePlanContext; -import org.opensearch.sql.calcite.SysLimit; -import org.opensearch.sql.calcite.utils.CalciteToolsHelper.OpenSearchRelBuilder; -import org.opensearch.sql.common.error.ErrorCode; -import org.opensearch.sql.common.error.ErrorReport; -import org.opensearch.sql.executor.QueryType; - -/** The uses a {@code flat_object} field allows, and the refusal for everything else. */ -@ExtendWith(MockitoExtension.class) -class FlatObjectScopeTest { - - @Mock private OpenSearchRelBuilder relBuilder; - @Mock private MockedStatic mockedToolsHelper; - - private CalcitePlanContext context; - - @BeforeEach - void setUp() { - mockedToolsHelper - .when(() -> CalciteToolsHelper.connect(any(), any())) - .thenReturn(mock(Connection.class)); - mockedToolsHelper - .when(() -> CalciteToolsHelper.create(any(), any(), any())) - .thenReturn(relBuilder); - when(relBuilder.getRexBuilder()).thenReturn(new RexBuilder(OpenSearchTypeFactory.TYPE_FACTORY)); - context = - CalcitePlanContext.create(mock(FrameworkConfig.class), SysLimit.DEFAULT, QueryType.PPL); - context.getFlatObjectFields().add("attributes"); - } - - /** What a command does when it resolves a name, with no read permission open. */ - private ErrorReport refused(String name) { - return assertThrows(ErrorReport.class, () -> FlatObjectScope.check(name, context)); - } - - // ---- what is allowed - - @Test - void aFieldOrALeafMayBeRead() { - FlatObjectScope.allowRead( - field("attributes.duration_ms"), - "attributes.duration_ms", - context, - () -> { - FlatObjectScope.check("attributes.duration_ms", context); - return null; - }); - FlatObjectScope.allowRead( - field("attributes"), - "attributes", - context, - () -> { - FlatObjectScope.check("attributes", context); - return null; - }); - } - - /** A read may wear a cast: that is how a query asks for a leaf as a number. */ - @Test - void aReadMayBeCast() { - UnresolvedExpression asDouble = - let( - field("d"), - new Cast( - field("attributes.duration_ms"), - org.opensearch.sql.ast.dsl.AstDSL.stringLiteral("double"))); - assertDoesNotThrow( - () -> - FlatObjectScope.allowRead( - asDouble, - "d", - context, - () -> { - FlatObjectScope.check("attributes.duration_ms", context); - return null; - })); - } - - /** typeof asks what a leaf is, not what it holds, so it reads nothing from the record. */ - @Test - void typeofIsAllowed() { - UnresolvedExpression typeOfLeaf = - let(field("t"), function("typeof", field("attributes.duration_ms"))); - assertDoesNotThrow( - () -> - FlatObjectScope.allowRead( - typeOfLeaf, - "t", - context, - () -> { - FlatObjectScope.check("attributes.duration_ms", context); - return null; - })); - } - - @Test - void aNameOutsideTheFieldIsNotTouched() { - assertDoesNotThrow(() -> FlatObjectScope.check("service", context)); - assertDoesNotThrow(() -> FlatObjectScope.check("attributesX", context)); - } - - // ---- what is refused - - @Test - void aValueComputedFromALeafIsRefused() { - UnresolvedExpression plusOne = - let(field("x"), function("+", field("attributes.duration_ms"), intLiteral(1))); - ErrorReport error = - assertThrows( - ErrorReport.class, - () -> - FlatObjectScope.allowRead( - plusOne, - "x", - context, - () -> { - FlatObjectScope.check("attributes.duration_ms", context); - return null; - })); - assertTrue( - error.getMessage().contains("Cannot compute a value from attributes.duration_ms"), - error.getMessage()); - } - - @Test - void theCommandNamesWhatItWasDoing() { - FlatObjectScope.runAs( - "sort by", - context, - () -> - assertTrue( - refused("attributes.duration_ms") - .getMessage() - .contains("Cannot sort by attributes.duration_ms"))); - } - - /** - * A command that reads a leaf into a column of its own does not escape the rule: pushdown fuses - * the projection back into whatever reads that column. - */ - @Test - void aColumnReadFromALeafIsRefusedToo() { - FlatObjectScope.allowRead( - let(field("d"), field("attributes.duration_ms")), "d", context, () -> null); - FlatObjectScope.runAs( - "sort by", - context, - () -> - assertTrue( - refused("d").getMessage().contains("Cannot sort by attributes.duration_ms"), - "the refusal names the leaf, not the column the query gave it")); - } - - @Test - void aRenameCarriesTheColumnForward() { - FlatObjectScope.allowRead( - let(field("d"), field("attributes.duration_ms")), "d", context, () -> null); - FlatObjectScope.trackRename("d", "e", context); - FlatObjectScope.runAs( - "stats by", - context, - () -> - assertTrue( - refused("e").getMessage().contains("Cannot stats by attributes.duration_ms"), - "a renamed column is still named by the leaf it was read from")); - } - - @Test - void theRefusalCarriesTheRuleAndTheDocumentation() { - ErrorReport error = refused("attributes.duration_ms"); - assertEquals(ErrorCode.UNSUPPORTED_OPERATION, error.getCode()); - assertTrue(error.getSuggestion().contains("can only be read"), error.getSuggestion()); - assertTrue( - error.getSuggestion().contains("supported-field-types/flat-object/"), - error.getSuggestion()); - } -} diff --git a/docs/user/ppl/general/datatypes.md b/docs/user/ppl/general/datatypes.md index 1cf6b7140f7..28cf7be5656 100644 --- a/docs/user/ppl/general/datatypes.md +++ b/docs/user/ppl/general/datatypes.md @@ -118,20 +118,11 @@ A [flat_object field](https://docs.opensearch.org/latest/mappings/supported-fiel Both a nested object and a literal dotted key reach the same leaf: `{"a": {"b": 1}}` and `{"a.b": 1}` each give the entry `a.b` with the value `"1"`. A path receives more than one value when it comes from an array, from several elements of an array of objects, or from both spellings of the same path appearing in one document; it then reads as a JSON array of those texts, in the order the document wrote them. -What a query can do with a flat_object leaf is what the field type itself can do: PPL inherits the field type's [limitations](https://docs.opensearch.org/latest/mappings/supported-field-types/flat-object/#limitations). The index files every leaf value as a keyword term with the path folded in and gives the leaf no mapping, so a leaf can be read, but nothing can be computed from it without opening every record -- the cost this field type exists to avoid. +A leaf is text, so a query does with it what it does with any text column, the same as a field `spath` extracted from a JSON string: read it, cast it (`eval ms = cast(attributes.duration_ms as double)`), filter by it, sort by it, group by it. -Supported on a leaf: +A path holding more than one value reads as JSON, so it does not compare equal to any single one of those values. The index still matches each of them, so a filter answers differently depending on whether it reached the index — that applies to any field holding several values, not only to a flat_object leaf. -- reading it, or the whole field; -- casting it, which is how a query asks for a leaf as a number: `eval ms = cast(attributes.duration_ms as double)`. - -Not supported on a leaf. These are rejected when the query is planned, with an error that names the field and links to the field type's documentation: - -- filtering by it; -- aggregating it, or grouping by it; -- sorting by it; -- computing a value from it, such as a function or an arithmetic expression; -- expanding it. +`expand` takes a column of the index rather than a leaf; expand the field itself, or project the leaf first. ### Example: People There are three fields in test index `people`: 1) deep nested object field `city`; 2) object field of array value `account`; 3) nested field `projects` diff --git a/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/1604.yml b/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/1604.yml index 31cb720cffd..a961e3d5a99 100644 --- a/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/1604.yml +++ b/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/1604.yml @@ -118,39 +118,42 @@ teardown: - match: {"datarows": [[4.0, "STRING"]]} --- -"anything other than reading a leaf is rejected when the query is planned": +# A leaf is text, so PPL does with it what it does with any text column -- the same as a field +# spath extracted from a JSON string. +"a leaf can be filtered, sorted and aggregated like any text column": - skip: features: [headers] - do: - catch: bad_request headers: {Content-Type: 'application/json'} ppl: body: - query: 'source=otel | sort attributes.duration_ms | fields service' - - match: {"$body": "/Cannot\\s+sort\\s+by\\s+attributes.duration_ms.*/"} - - match: {"$body": "/.*can\\s+only\\s+be\\s+read.*/"} - - match: {"$body": "/.*docs.opensearch.org\\/latest\\/mappings\\/supported-field-types\\/flat-object.*/"} + query: 'source=otel | where attributes.duration_ms = ''4'' | fields service' + - match: {"datarows": [["payments"]]} - do: - catch: bad_request headers: {Content-Type: 'application/json'} ppl: body: - query: 'source=otel | stats count() by attributes.duration_ms' - - match: {"$body": "/Cannot\\s+stats\\s+by\\s+attributes.duration_ms.*/"} + query: 'source=otel | where attributes.duration_ms > 5 | fields service' + - match: {"datarows": [["checkout"]]} - do: - catch: bad_request headers: {Content-Type: 'application/json'} ppl: body: - query: 'source=otel | where attributes.duration_ms = ''4'' | fields service' - - match: {"$body": "/Cannot\\s+filter\\s+by\\s+attributes.duration_ms.*/"} + query: 'source=otel | stats avg(attributes.duration_ms)' + - match: {"datarows": [[8.25]]} - do: - catch: bad_request headers: {Content-Type: 'application/json'} ppl: body: - query: 'source=otel | eval d = attributes.duration_ms + 1 | fields d' - - match: {"$body": "/Cannot\\s+compute\\s+a\\s+value\\s+from\\s+attributes.duration_ms.*/"} + query: 'source=otel | where isnotnull(attributes.duration_ms) | sort attributes.duration_ms | fields attributes.duration_ms' + - match: {"datarows": [["12.5"], ["4"]]} + +--- +# expand needs a column of the input to correlate the expansion with. A leaf is not one, so it is +# reported the way the mvexpand check reports it rather than as a ClassCastException. +"expand does not take a leaf": + - skip: + features: [headers] - do: catch: bad_request headers: {Content-Type: 'application/json'} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java index 64bcecba6e5..919736e05f0 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java @@ -43,7 +43,6 @@ import org.opensearch.sql.opensearch.data.type.OpenSearchFlatObjectType; import org.opensearch.sql.opensearch.data.type.OpenSearchTextType; import org.opensearch.sql.opensearch.storage.script.CalciteScriptEngine.Source; -import org.opensearch.sql.opensearch.storage.script.CalciteScriptEngine.UnsupportedScriptException; /** * This help standardizes the RexNode expression, the process including: @@ -112,18 +111,12 @@ public RexNode visitInputRef(RexInputRef inputRef, ScriptParameterHelper helper) String docFieldName = exprType == ExprCoreType.STRUCT || exprType == ExprCoreType.ARRAY - // A binary field has no doc values, so it has to be read from _source too. + // A binary field has no doc values, so it has to be read from _source too, and + // neither does a flat_object: each leaf is one folded path=value term. || exprType instanceof OpenSearchBinaryType + || exprType instanceof OpenSearchFlatObjectType ? null : OpenSearchTextType.toKeywordSubField(field.getName(), exprType); - if (exprType instanceof OpenSearchFlatObjectType) { - // Each leaf is indexed as one folded path=value term, so a flat_object has no doc values a - // script could read: it would have to open _source for every record it is asked about, the - // cost the field type exists to avoid. A script over one is refused rather than built, unlike - // the no-doc-values types above, which are read from _source that way on purpose. - throw new UnsupportedScriptException( - "A flat_object field cannot be read by a pushed-down script: " + field.getName()); - } int newIndex = helper.sources.size(); if (docFieldName != null) { helper.digests.add(docFieldName);