-
Notifications
You must be signed in to change notification settings - Fork 25.6k
ESQL: Views REST API #137818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
ESQL: Views REST API #137818
Changes from 7 commits
49d15e6
507bf1c
b0ae25c
73da7cd
21ca8f0
050d89c
f529f11
2694cef
d466dcd
fe44dbb
507c781
ef4e0bf
2065a22
3cd972d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| pr: 137818 | ||
| summary: ES|QL Views REST API | ||
| area: "ES|QL" | ||
| type: feature | ||
| issues: [] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| { | ||
| "esql.delete_view": { | ||
| "documentation": { | ||
| "url": "https://www.elastic.co/docs/api/doc/elasticsearch/operation/operation-esql-view-delete", | ||
| "description": "Delete a non-materialized VIEW for ESQL." | ||
| }, | ||
| "stability": "experimental", | ||
| "visibility": "public", | ||
| "headers": { | ||
| "accept": [ | ||
| "application/json" | ||
| ], | ||
| "content_type": [ | ||
| "application/json" | ||
| ] | ||
| }, | ||
| "url": { | ||
| "paths": [ | ||
| { | ||
| "path": "/_query/view/{name}", | ||
| "methods": [ | ||
| "DELETE" | ||
| ], | ||
| "parts": { | ||
| "name": { | ||
| "type": "string", | ||
| "description": "The name of the view to delete" | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| { | ||
| "esql.get_view": { | ||
| "documentation": { | ||
| "url": "https://www.elastic.co/docs/api/doc/elasticsearch/operation/operation-esql-view-get", | ||
| "description": "Get a non-materialized VIEW for ESQL." | ||
| }, | ||
| "stability": "experimental", | ||
| "visibility": "public", | ||
| "headers": { | ||
| "accept": [ | ||
| "application/json" | ||
| ], | ||
| "content_type": [ | ||
| "application/json" | ||
| ] | ||
| }, | ||
| "url": { | ||
| "paths": [ | ||
| { | ||
| "path": "/_query/view/{name}", | ||
| "methods": [ | ||
| "GET" | ||
| ], | ||
| "parts": { | ||
| "name": { | ||
| "type": "list", | ||
| "description": "A comma-separated list of view names" | ||
| } | ||
| } | ||
| }, | ||
| { | ||
| "path": "/_query/view", | ||
| "methods": [ | ||
| "GET" | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| { | ||
| "esql.put_view": { | ||
| "documentation": { | ||
| "url": "https://www.elastic.co/docs/api/doc/elasticsearch/operation/operation-esql-view-put", | ||
| "description": "Creates a non-materialized VIEW for ESQL." | ||
| }, | ||
| "stability": "experimental", | ||
| "visibility": "public", | ||
| "headers": { | ||
| "accept": [ | ||
| "application/json" | ||
| ], | ||
| "content_type": [ | ||
| "application/json" | ||
| ] | ||
| }, | ||
| "url": { | ||
| "paths": [ | ||
| { | ||
| "path": "/_query/view/{name}", | ||
| "methods": [ | ||
| "PUT" | ||
| ], | ||
| "parts": { | ||
| "name": { | ||
| "type": "string", | ||
| "description": "The name of the view to create or update" | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "body": { | ||
| "description": "Use the `query` element to define the ES|QL query to use as a non-materialized VIEW.", | ||
| "required": true | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| 9217000 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| esql_execution_metadata,9216000 | ||
| esql_views,9217000 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| package org.elasticsearch.xpack.esql.view; | ||
|
|
||
| import org.elasticsearch.action.ActionListener; | ||
| import org.elasticsearch.cluster.ClusterState; | ||
| import org.elasticsearch.cluster.ClusterStateUpdateTask; | ||
| import org.elasticsearch.cluster.metadata.ProjectMetadata; | ||
| import org.elasticsearch.cluster.project.ProjectResolver; | ||
| import org.elasticsearch.cluster.service.ClusterService; | ||
| import org.elasticsearch.core.SuppressForbidden; | ||
| import org.elasticsearch.features.FeatureService; | ||
| import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry; | ||
| import org.elasticsearch.xpack.esql.plugin.EsqlFeatures; | ||
|
|
||
| import java.util.Map; | ||
| import java.util.function.Function; | ||
|
|
||
| /** | ||
| * Implementation of {@link ViewService} that keeps the views in the cluster state. | ||
| */ | ||
| public class ClusterViewService extends ViewService { | ||
| private final ClusterService clusterService; | ||
| private final FeatureService featureService; | ||
| private final ProjectResolver projectResolver; | ||
|
|
||
| public ClusterViewService( | ||
| EsqlFunctionRegistry functionRegistry, | ||
| ClusterService clusterService, | ||
| FeatureService featureService, | ||
| ProjectResolver projectResolver, | ||
| ViewServiceConfig config | ||
| ) { | ||
| super(functionRegistry, config); | ||
| this.clusterService = clusterService; | ||
| this.featureService = featureService; | ||
| this.projectResolver = projectResolver; | ||
| } | ||
|
|
||
| @Override | ||
| protected ViewMetadata getMetadata() { | ||
| return getMetadata(clusterService.state()); | ||
| } | ||
|
|
||
| protected ViewMetadata getMetadata(ClusterState clusterState) { | ||
| return getProjectMetadata(clusterState).custom(ViewMetadata.TYPE, ViewMetadata.EMPTY); | ||
| } | ||
|
|
||
| protected ProjectMetadata getProjectMetadata(ClusterState clusterState) { | ||
| return projectResolver.getProjectMetadata(clusterService.state()); | ||
| } | ||
|
|
||
| @Override | ||
| protected void updateViewMetadata(ActionListener<Void> callback, Function<ViewMetadata, Map<String, View>> function) { | ||
| submitUnbatchedTask("update-esql-view-metadata", new ClusterStateUpdateTask() { | ||
| @Override | ||
| public ClusterState execute(ClusterState currentState) { | ||
| var project = getProjectMetadata(currentState); | ||
| var views = project.custom(ViewMetadata.TYPE, ViewMetadata.EMPTY); | ||
| Map<String, View> policies = function.apply(views); | ||
| var metadata = ProjectMetadata.builder(project).putCustom(ViewMetadata.TYPE, new ViewMetadata(policies)); | ||
| return ClusterState.builder(currentState).putProjectMetadata(metadata).build(); | ||
| } | ||
|
|
||
| @Override | ||
| public void clusterStateProcessed(ClusterState oldState, ClusterState newState) { | ||
| callback.onResponse(null); | ||
| } | ||
|
|
||
| @Override | ||
| public void onFailure(Exception e) { | ||
| callback.onFailure(e); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| @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); | ||
| } | ||
|
Comment on lines
+96
to
+99
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comes from the original prototype, and it would be good to fix this already now.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, the enrich policy api also uses
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah that code is definitely not recent either, hence the |
||
|
|
||
| @Override | ||
| protected void assertMasterNode() { | ||
| assert clusterService.localNode().isMasterNode(); | ||
| } | ||
|
|
||
| @Override | ||
| protected boolean viewsFeatureEnabled() { | ||
| return featureService.clusterHasFeature(clusterService.state(), EsqlFeatures.ESQL_VIEWS); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getProjectMetadatauses theProjectResolver, but theProjectResolveris 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
ProjectIdto 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:Let me know what you think and if you have any questions/concerns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.