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

Adding Nested Function Documentation #2

Merged

Conversation

forestmvey
Copy link

@forestmvey forestmvey commented May 18, 2023

Description

Added documentation section in SQL for nested function added to V2 engine in PR-1490 and PR-1657. The added features covers the nested function used in SQL.

PDF

Nested Function - OpenSearch documentation.pdf

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

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

This is documentation for entire plugin, not for V2. Mention that in V2 it is supported in SELECT clause only and reference this paragraph.
But you have to share examples and syntax for V1 too and for WHERE clause too.
Fortunately, this won't change with your implementation in V2.

Please also add entire web page (not MD) as PDF to the PR, so we can see how it looks like.

The `field_expression` parameter is required, and the `path_expression` parameter is optional. Dot notation is used to show nesting level for `field_expression` and `path_expression` parameters. For example `nestedObj.innerFieldName` denotes a field nested one level. If the user does not provide the `path_expression` parameter it will be generated dynamically. For example the field `user.office.cubicle` would dynamically generate the path `user.office`.

```sql
nested(field_expression | field_expression, path_expression)

Choose a reason for hiding this comment

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

Would it be worth putting the example before the description so you can see the layout of the expression before the explanation?

Copy link
Author

Choose a reason for hiding this comment

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

Doesn't tend to be the standard way that the examples are presented on the documentation. I'd like to conform with the other pages.


## Nested query

The nested function is used in the `SELECT` clause to unnest nested object type collections. The nested collection is flattened and a cartesian product is returned when querying against two nested objects.

Choose a reason for hiding this comment

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

When used in the SELECT clause?

Choose a reason for hiding this comment

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

This document doesn't mention anything about the WHERE clause. Maybe we can add that too?

matthewryanwells pushed a commit that referenced this pull request Jun 8, 2023
…t#2374)

* for new page how to analyze Jaeger trace data

Signed-off-by: alicejw <[email protected]>

* remove old image

Signed-off-by: alicejw <[email protected]>

* for new information and doc writer checklist

Signed-off-by: alicejw <[email protected]>

* for new information and doc writer checklist

Signed-off-by: alicejw <[email protected]>

* small rewrite

Signed-off-by: alicejw <[email protected]>

* new clean images from Dashboards URL directly

Signed-off-by: alicejw <[email protected]>

* for additional information

Signed-off-by: alicejw <[email protected]>

* remove blank lines

Signed-off-by: alicejw <[email protected]>

* for tech review feedback updates

Signed-off-by: alicejw <[email protected]>

* add requirements section

Signed-off-by: alicejw <[email protected]>

* for new procedure

Signed-off-by: alicejw <[email protected]>

* for tech review feedback updates

Signed-off-by: alicejw <[email protected]>

* continued updates

Signed-off-by: alicejw <[email protected]>

* for docker compose file instructions

Signed-off-by: alicejw <[email protected]>

* for docker usage instruction

Signed-off-by: alicejw <[email protected]>

* for step 2 view dashboards

Signed-off-by: alicejw <[email protected]>

* for additional link provided in tech review

Signed-off-by: alicejw <[email protected]>

* for link to index page to introduce the feature

Signed-off-by: alicejw <[email protected]>

* final checklist

Signed-off-by: alicejw <[email protected]>

* add warning not to use sample file in prod env

Signed-off-by: alicejw <[email protected]>

* updated docker file that is safe for prod env, remove warning note for previous file

Signed-off-by: alicejw <[email protected]>

* for small update to parent page

Signed-off-by: alicejw <[email protected]>

* for tech review

Signed-off-by: alicejw <[email protected]>

* typo fix for font

Signed-off-by: alicejw <[email protected]>

* for doc review #1 feedback updates

Signed-off-by: alicejw <[email protected]>

* for doc review feedback #2 updates

Signed-off-by: alicejw <[email protected]>

* for a couple minor changes

Signed-off-by: alicejw <[email protected]>

* spell out dashboard URI directly to trace analytics for accessibility. With URL obfuscated, this would not get spelled out in the vision-impaired app that scans through.it would simply read the title only

Signed-off-by: alicejw <[email protected]>

* need to add additional step from eng to generate sample data

Signed-off-by: alicejw <[email protected]>

* for additional step image of sample app

Signed-off-by: alicejw <[email protected]>

* rename step numbers

Signed-off-by: alicejw <[email protected]>

* minor fix heading levels

Signed-off-by: alicejw <[email protected]>

* updates recommended by the editorial reviewer

Signed-off-by: alicejw <[email protected]>

* clarify Spans window function

Signed-off-by: alicejw <[email protected]>

* clarified individual trace details section

Signed-off-by: alicejw <[email protected]>

Signed-off-by: alicejw <[email protected]>
@forestmvey forestmvey changed the title Adding Nested Function SELECT Clause Documentation Adding Nested Function Documentation Jun 13, 2023

## Nested In SELECT Clause

The nested function is used in the `SELECT` clause to unnest nested object type collections. The nested collection is flattened and a cartesian product is returned when querying against two or more nested collections. The nested function is supported in the V2 engine in the `SELECT` and `WHERE` clauses, see [query-processing-engines](https://opensearch.org/docs/latest/search-plugins/sql/limitation/#query-processing-engines) for more details.

Choose a reason for hiding this comment

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

I feel that phrase

The nested function is supported

is not suitable here. Maybe you need to move it outside of this paragraph, for example to a separate section on the very bottom called 'limitations'.

_search-plugins/sql/sql/nested.md Show resolved Hide resolved
The results contain documents that match the nested query:

| nested(message.info) |
:----------------------|
Copy link

@MitchellGale MitchellGale Jun 13, 2023

Choose a reason for hiding this comment

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

Is this the correct table format and not:

| nested(message.info) |
| -------------------- |
| letter2              |

@forestmvey forestmvey merged commit 987da7d into integ-sql-nested-select-clause Jun 13, 2023
@forestmvey forestmvey deleted the dev-sql-nested-select-clause branch June 13, 2023 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants