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

Update docs to document breaking change in explain attributes #16031

Merged
merged 3 commits into from
Mar 6, 2024

Conversation

adarshsanjeev
Copy link
Contributor

Updates the docs to include a breaking change made regarding the attributes returned by an explain query. The return type of the field targetDataSource was changed to an object instead of a string containing the data source name.

This change is only present in Druid 29, and not in future versions, so it has been documented here. The change has been reverted in future versions here: #16004

### Breaking change

Druid 29 has a breaking change for EXPLAIN queries.
In the attribute field returned as part of the result for an explain query, the value of the key `targetDataSource` from a string to a JSON object.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add more details like it impacts only MSQ queries and give an example of the new json object.

Copy link
Contributor

Choose a reason for hiding this comment

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

This has been added on top of the release notes. Which kind of seem bad. This should be present in the upgrade section which is there near the end of the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to the upgrade section

docs/querying/sql-translation.md Outdated Show resolved Hide resolved
@@ -71,7 +71,7 @@ EXPLAIN PLAN statements return:
- a `RESOURCES` column that describes the resources used in the query
- an `ATTRIBUTES` column that describes the attributes of the query, including:
- `statementType`: the SQL statement type
- `targetDataSource`: the target datasource in an INSERT or REPLACE statement
- `targetDataSource`: a JSON object representing the target datasource in an INSERT or REPLACE statement.
- `partitionedBy`: the time-based partitioning granularity in an INSERT or REPLACE statement
Copy link
Contributor

Choose a reason for hiding this comment

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

For 29.0.0, could you also please update targetDataSource format in this doc to the JSON object in these example EXPLAIN PLAN outputs here and here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

Copy link
Contributor

@cryptoe cryptoe left a comment

Choose a reason for hiding this comment

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

Please have a look at the review comments. I feel those changes are important before we merge this.

@cryptoe cryptoe merged commit 86ee778 into apache:29.0.0 Mar 6, 2024
7 checks passed
Copy link
Contributor

@cryptoe cryptoe left a comment

Choose a reason for hiding this comment

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

Changes LGTM

@317brian
Copy link
Contributor

317brian commented Mar 6, 2024

@adarshsanjeev The relevant changes will also need to go into the 29.0.1 branch so that it gets included in those docs and goes out with that release

@adarshsanjeev
Copy link
Contributor Author

@317brian, the change should only be present in the 29.0, and was reverted in 29.0.1, so the documentation would not need to be updated, right?

@317brian
Copy link
Contributor

317brian commented Mar 7, 2024

Oh ok. It's not needed then. It was little unclear to me cause 29 can be read as all versions of 29 rather than 29.0.0 specifically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants