Skip to content
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

Warn or fail if runtime field doesn't have emit #72723

Open
nik9000 opened this issue May 4, 2021 · 7 comments
Open

Warn or fail if runtime field doesn't have emit #72723

nik9000 opened this issue May 4, 2021 · 7 comments
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement Team:Core/Infra Meta label for core/infra team

Comments

@nik9000
Copy link
Member

nik9000 commented May 4, 2021

At this point runtime fields require you to call emit to return values. As much as it'd be nice to auto-emit in some obvious cases, we don't now. So maybe we should fail to compile scripts that don't ever call emit. Or at return a warning or something. These scripts are almost certainly an accident. Possibly one from copying one of kibana's "scripted fields".

I'm imagining something like this being ok:

if (doc['a'].value < 10) {
  emit(doc['b'].value * 10);
}

It calls emit in an if. But something like this:

doc['b'].value * 10;

would fail. Or return a warning. Or something. Because its never going to result in a value at all. It can't because you never invoke the emit function in any code path.

@nik9000 nik9000 added >enhancement :Search/Search Search-related issues that do not fall into other categories needs:triage Requires assignment of a team area label labels May 4, 2021
@elasticmachine elasticmachine added the Team:Search Meta label for search team label May 4, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@javanna
Copy link
Member

javanna commented May 5, 2021

I tend to see the proposed solution as a way to work around the fact that we don't automatically emit values when we could. I would much rather prefer that we rather focus first on auto-emitting.

@nik9000
Copy link
Member Author

nik9000 commented May 5, 2021

Would you prefer that we leave things as they are for six mon

@nik9000 nik9000 closed this as completed May 5, 2021
@javanna javanna added team-discuss and removed needs:triage Requires assignment of a team area label labels May 10, 2021
@javanna javanna reopened this May 10, 2021
@javanna
Copy link
Member

javanna commented May 28, 2021

We have discussed this with the team, we said we are ok going ahead with it. I also opened #73536 to track the need for automatically emitting values. Like I commented there, failing when emit is forgotten, if we can detect it accurately, would be a good improvement, but we also want to raise the importance of automatically emitting values when possible.

@javanna javanna added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache and removed team-discuss labels Jun 3, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Jun 3, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@rjernst
Copy link
Member

rjernst commented Jun 3, 2021

Both warning, as well as auto emitting, would be difficult to do in painless in a script agnostic way. It's not impossible (similar in some ways to auto return, as far as detection), but it would require some new designs to designate what gets added to the tree, perhaps by generalizing auto return. Remember that emit is not a language feature, it is just a method exposed on runtime field script classes.

I've added this for discussion by the scripting team.

@stu-elastic
Copy link
Contributor

A straightforward way of doing this would be to track the methods called by a script and expose that to the caller. The caller (rt fields in this case) can then inspect the methods and fails if emit is not among them.

@javanna javanna removed :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team labels Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

No branches or pull requests

5 participants