ESQL: Views REST API #137818

craigtaverner · 2025-11-10T11:22:06Z

This is extracted from the Views Prototype at #134995 and is originally based on the "ESQL: View" prototype by Nik but with the following changes:

No ES|QL language integration, only REST API for adding/removing and managing Views in cluster state
ViewMetadata now inside ProjectMetadata
NodeFeature to ensure metadata only exists in clusters fully upgraded
Wide CRUD support (add, update, delete, list)
View name validation
View count, length and depth restrictions

The REST API looks like:

PUT _query/view/critical_service_errors_v {"query":"FROM ..."}
DELETE _query/view/critical_service_errors_v
GET _query/view/critical_service_errors_v
GET _query/view

Checklist:

Extract REST API from views prototype
Remove list interface and generalise GET command to cover list use cases (more in-line with other APIs)
Verify that snapshot-only really blocks registration of REST API
Consider switching to use feature flags (instead of snapshot-only)
Consider dropping one of NodeFeature or TransportVersion since they cover the same requirement
Implement batched tasks, probably using SequentialAckingBatchedTaskExecutor
Stop using ProjectResolver and instead use the ProjectID as is done in Enrich REST API

elasticsearchmachine · 2025-11-10T11:22:32Z

Pinging @elastic/es-analytical-engine (Team:Analytics)

elasticsearchmachine · 2025-11-10T11:22:32Z

Pinging @elastic/es-data-management (Team:Data Management)

elasticsearchmachine · 2025-11-10T11:22:55Z

Hi @craigtaverner, I've created a changelog YAML for you.

dakrone · 2025-11-10T15:37:14Z

Since these don't yet do anything, and will be part of multiple PRs before the feature is fully-fleshed out, I think it would be helpful for these to be behind a feature flag, so that they are not inadvertently released-and-then-changed.

craigtaverner · 2025-11-10T15:47:21Z

Since these don't yet do anything, and will be part of multiple PRs before the feature is fully-fleshed out, I think it would be helpful for these to be behind a feature flag, so that they are not inadvertently released-and-then-changed.

We've made this SNAPSHOT only, which is essentially the same as a feature flag, and is the approach normally taken in the ES|QL team for cases like this where we do not yet want the feature to be released. However, I will add more tests that explicitly assert that the feature is missing outside of SNAPSHOT builds.

dakrone · 2025-11-10T15:50:04Z

We've made this SNAPSHOT only, which is essentially the same as a feature flag, and is the approach normally taken in the ES|QL team for cases like this where we do not yet want the feature to be released. However, I will add more tests that explicitly assert that the feature is missing outside of SNAPSHOT builds.

As I read this, that is for the implementation itself, but does not prevent the REST APIs from being registered and available, even without the node feature. Unless I am misunderstanding it?

dakrone · 2025-11-10T15:52:11Z

(Another drive-by comment, but this is not a full review)

Why do we have both a "get" and "list" action/API for these? All of our other APIs that follow this model register a single get REST handler that takes an optional {name} parameter, returning all the <things> when the {name} is missing.

elasticsearchmachine · 2025-11-11T09:39:48Z

Hi @craigtaverner, I've created a changelog YAML for you.

This is extracted from the Views Prototype at elastic#134995 and is originally based on the "ESQL: View" prototype by Nik but with the following changes: * No ES|QL language integration, only REST API for adding/removing and managing Views in cluster state * ViewMetadata now inside ProjectMetadata * NodeFeature to ensure metadata only exists in clusters fully upgraded * Wide CRUD support (add, update, delete, list) * View name validation * View count, length and depth restrictions Co-authored-by: Nik Everett <[email protected]>

…n on missing view

…der list structures

nielsbauman

Thanks for working on this, @craigtaverner! I scanned most parts of the code briefly and left a few comments, some of which are not a big deal.

...vileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java

nielsbauman · 2025-11-12T17:28:55Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewMetadata.java

+        PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> {
+            Map<String, View> patterns = new HashMap<>();
+            String fieldName = null;
+            for (XContentParser.Token token = p.nextToken(); token != XContentParser.Token.END_OBJECT; token = p.nextToken()) {
+                if (token == XContentParser.Token.FIELD_NAME) {
+                    fieldName = p.currentName();
+                } else if (token == XContentParser.Token.START_OBJECT) {
+                    patterns.put(fieldName, View.fromXContent(p));
+                } else {
+                    throw new ElasticsearchParseException("unexpected token [" + token + "]");
+                }
+            }
+            return patterns;
+        }, VIEWS);


Could we use AbstractObjectParser#declareNamedObjects instead here? That avoids using the old parser syntax, which is a lot harder to read (and debug). I think that would look something like this:

Suggested change

PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> {

Map<String, View> patterns = new HashMap<>();

String fieldName = null;

for (XContentParser.Token token = p.nextToken(); token != XContentParser.Token.END_OBJECT; token = p.nextToken()) {

if (token == XContentParser.Token.FIELD_NAME) {

fieldName = p.currentName();

} else if (token == XContentParser.Token.START_OBJECT) {

patterns.put(fieldName, View.fromXContent(p));

} else {

throw new ElasticsearchParseException("unexpected token [" + token + "]");

}

}

return patterns;

}, VIEWS);

PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> View.fromXContent(p), VIEWS);

but I didn't test that, so there's a chance I'm saying something that doesn't make sense :)

This is copied verbatim from EnrichMetadata, which has a very similar structure with the constructor taking a Map<String, EnrichPolicy>, and here we take Map<String, View>. The difference is handled on line 56 which decodes the View using View.fromXContent. You suggest that I skip the map, which I'm not sure makes sense to me.

That code in EnrichMetadata hasn't been modified since 2019, so not every aspect of that class serves as a great example for us here. I indeed now realize that declareObject returns a list and not a map, that's unfortunate... I've asked around if we don't have any utility for this, but we can leave it for now.

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/View.java

nielsbauman · 2025-11-12T17:33:36Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ClusterViewService.java

+    @SuppressForbidden(reason = "legacy usage of unbatched task") // TODO add support for batching here
+    private void submitUnbatchedTask(@SuppressWarnings("SameParameterValue") String source, ClusterStateUpdateTask task) {
+        clusterService.submitUnbatchedStateUpdateTask(source, task);
+    }


Not sure if you were still planning on doing this, but it would be good to use a proper task queue here. I would recommend using a SimpleBatchedAckListenerTaskExecutor, as that avoids you having to implement a boilerplate executor.

This comes from the original prototype, and it would be good to fix this already now.

Interesting, the enrich policy api also uses submitUnbatchedStateUpdateTask...

Yeah that code is definitely not recent either, hence the TODO in the code :)

nielsbauman · 2025-11-12T17:39:54Z

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewService.java

+ * Simple implementation of {@link ClusterViewService} that keeps the views in memory.
+ * This is useful for testing.
+ */
+public class InMemoryViewService extends ViewService {


I'm probably missing something, but why do we need this in-memory version exactly?

And, follow-up question: wouldn't it make more sense to put that in a test package somewhere, rather than having it in production code, if it's only useful for testing?

It is only moderately useful here, but very useful in the actual ES|QL views were we have semi-unit tests that run the full ES|QL stack, but outside of Elasticsearch, and then this becomes the main mechanism for enabling that. I could indeed move it.

In this PR it serves primarily as unit tests for the ViewService, without touching cluster state (so only tests the ViewService API internally). That still has value, so I'm happy to keep it here (although I moved to the test package, as you suggested).

nielsbauman · 2025-11-12T17:47:27Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ClusterViewService.java

+        submitUnbatchedTask("update-esql-view-metadata", new ClusterStateUpdateTask() {
+            @Override
+            public ClusterState execute(ClusterState currentState) {
+                var project = getProjectMetadata(currentState);


getProjectMetadata uses the ProjectResolver, but the ProjectResolver is not meant to be used inside cluster state update tasks, as the thread context isn't copied over when running cluster state update tasks (which is what the project resolver relies on in multi-project mode). It's totally understandable that you would implement it like this; when the multi-project work gets picked up, we should put effort into making it clearer that the project resolver isn't supposed to work like this.

That means that you'll have to explicitly pass a ProjectId to this method. I would advise resolving the project ID in the transport actions, because:

I think having this getMetadata() method that relies on the project resolver can cause confusion and possible bugs, because it's not apparent at all that this method only works if the project ID is present in the thread context, which isn't always the case, such as here. I would advise doing something along the lines of:

protected ViewMetadata getMetadata(ProjectId projectId) { return getMetadata(clusterService.state().metadata().getProject(projectId)); } protected ViewMetadata getMetadata(ProjectMetadata projectMetadata) { return projectMetadata.custom(ViewMetadata.TYPE, ViewMetadata.EMPTY); }

Let me know what you think and if you have any questions/concerns.

I originally got this approach from the EnrichPolicyResolver, which uses ProjectResolver, but then also noticed that the EnrichPolicy CRUD used ProjectID instead, so it seems Enrich uses both approaches (but in different places, so probably correct). I can try switch to using ProjectID for CRUD as you suggest.

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/TransportGetViewAction.java

craigtaverner requested review from nielsbauman and nik9000 November 10, 2025 11:22

craigtaverner added >feature :Data Management/Indices APIs APIs to create and manage indices and templates Team:Data Management Meta label for data/management team Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL labels Nov 10, 2025

elasticsearchmachine added the v9.3.0 label Nov 10, 2025

craigtaverner force-pushed the views_rest_crud branch from 259d88e to 2b93fa5 Compare November 10, 2025 11:38

craigtaverner force-pushed the views_rest_crud branch 2 times, most recently from bb1ce21 to 89f766e Compare November 10, 2025 17:43

craigtaverner and others added 4 commits November 12, 2025 12:50

Update docs/changelog/137818.yaml

507bf1c

Refine changelog

b0ae25c

Expand get-views to cover use cases for listing views

73da7cd

craigtaverner force-pushed the views_rest_crud branch from f21d063 to 73da7cd Compare November 12, 2025 11:50

craigtaverner added 3 commits November 12, 2025 15:57

Fixed failing tests after changing behaviour of get to throw exceptio…

21ca8f0

…n on missing view

Fixed failing yaml-tests after changing behaviour of get to return ol…

050d89c

…der list structures

Remove list_views API (covered by get_view now)

f529f11

nielsbauman reviewed Nov 12, 2025

View reviewed changes

craigtaverner added 2 commits November 13, 2025 14:47

Simply fixes based on code review

2694cef

Merge remote-tracking branch 'origin/main' into views_rest_crud

d466dcd

craigtaverner added 5 commits November 13, 2025 14:58

Fix transport version numbers

fe44dbb

Update REST API to use ProjectID where possible

507c781

Fixed things that did not get backported correctly

ef4e0bf

Fixed named writable

2065a22

Missing project ID

3cd972d

ESQL: Views REST API #137818

Are you sure you want to change the base?

ESQL: Views REST API #137818

Conversation

craigtaverner commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Nov 10, 2025

Uh oh!

elasticsearchmachine commented Nov 10, 2025

Uh oh!

elasticsearchmachine commented Nov 10, 2025

Uh oh!

dakrone commented Nov 10, 2025

Uh oh!

craigtaverner commented Nov 10, 2025

Uh oh!

dakrone commented Nov 10, 2025

Uh oh!

dakrone commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Nov 11, 2025

Uh oh!

nielsbauman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

craigtaverner commented Nov 10, 2025 •

edited

Loading

dakrone commented Nov 10, 2025 •

edited

Loading