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

[ES|QL] Generate docs for unregistered esql functions from annotations #108749

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

fang-xing-esql
Copy link
Contributor

This is a subtask of #104247 , covers operators that are unregistered functions.

@fang-xing-esql fang-xing-esql added >docs General docs changes :Analytics/ES|QL AKA ESQL ES|QL-ui Impacts ES|QL UI v8.15.0 labels May 17, 2024
Copy link

Documentation preview:

@fang-xing-esql fang-xing-esql marked this pull request as ready for review May 21, 2024 21:30
@elasticsearchmachine elasticsearchmachine added Team:Docs Meta label for docs team Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels May 21, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticsearchmachine
Copy link
Collaborator

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

@Override
public void testFactoryToString() {
assumeFalse("test case is invalid", false);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can pull out a superclass of AbstractFunctionTestCase without these methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed with Nik about this, skipping testFactoryToString is for InTest only, it is because the way In is implemented in ES|QL is different from the other functions, it doesn't implement EvaluatorMapper. Follow up PRs will be opened to rewrite In using EvaluatorMapper and clean up EvaMapper. This PR is mainly for doc generation through annotations, leave this test skipped for now, and when the In is rewritten with EvaluatorMapper, InTests will be modified.


@Override
public void testSimpleWithNulls() {
assumeFalse("test case is invalid", false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How many legitimate callers of this are there left? I imagine you've nearly replaced all users of this with anyNullIsNull and the like.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, after rewriting In with EvaluatorMapper, the InTests will be modified accordingly, this is for doc generation only.

@nik9000
Copy link
Member

nik9000 commented May 24, 2024

The docs don't quite look right:
image
It has both the "equality (==)" bit and the EQUALS bit. I'm not sure what's up there.

@nik9000
Copy link
Member

nik9000 commented May 24, 2024

This is heroic work, btw. Thanks so much for it.

@fang-xing-esql
Copy link
Contributor Author

The docs don't quite look right: image It has both the "equality (==)" bit and the EQUALS bit. I'm not sure what's up there.

I'll remove one of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL >docs General docs changes ES|QL-ui Impacts ES|QL UI Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Docs Meta label for docs team v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants