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

Add docs for polygon functions #49512

Merged
merged 8 commits into from
May 24, 2023
Merged

Add docs for polygon functions #49512

merged 8 commits into from
May 24, 2023

Conversation

DanRoscigno
Copy link
Contributor

Add docs for PR #19257

closes #35341

Changelog category (leave one):

  • Documentation (changelog entry is not required)

@nikitamikhaylov can you help with the descriptions for these functions? I am having a problem with the spherical functions, I do not understand how they work.

@DanRoscigno DanRoscigno added pr-documentation Documentation PRs for the specific code PR can be tested Allows running workflows for external contributors labels May 4, 2023
@robot-ch-test-poll4
Copy link
Contributor

robot-ch-test-poll4 commented May 4, 2023

This is an automated comment for commit 9f349e2 with description of existing statuses. It's updated for the latest CI running
The full report is available here
The overall status of the commit is 🟡 pending

Check nameDescriptionStatus
CI runningA meta-check that indicates the running CI. Normally, it's in success or pending state. The failed status indicates some problems with the PR🟡 pending
Mergeable CheckChecks if all other necessary checks are successful🟢 success
Push to DockerhubThe check for building and pushing the CI related docker images to docker hub🟢 success

@DanRoscigno DanRoscigno marked this pull request as draft May 4, 2023 13:54
@DanRoscigno DanRoscigno marked this pull request as ready for review May 22, 2023 17:46
docs/en/sql-reference/functions/geo/polygon.md Outdated Show resolved Hide resolved
docs/en/sql-reference/functions/geo/polygon.md Outdated Show resolved Hide resolved
docs/en/sql-reference/functions/geo/polygon.md Outdated Show resolved Hide resolved
docs/en/sql-reference/functions/geo/polygon.md Outdated Show resolved Hide resolved
docs/en/sql-reference/functions/geo/polygon.md Outdated Show resolved Hide resolved
docs/en/sql-reference/functions/geo/polygon.md Outdated Show resolved Hide resolved
docs/en/sql-reference/functions/geo/polygon.md Outdated Show resolved Hide resolved
docs/en/sql-reference/functions/geo/polygon.md Outdated Show resolved Hide resolved
@nikitamikhaylov nikitamikhaylov self-assigned this May 23, 2023
@DanRoscigno
Copy link
Contributor Author

Thanks so much @nikitamikhaylov. I will go back to the tests and add the rest of the examples and publish this, then add the docs to the source so that they can be maintained there and we have the same info in system.functions and in the published docs.

@DanRoscigno DanRoscigno marked this pull request as draft May 23, 2023 19:37
@DanRoscigno
Copy link
Contributor Author

@nikitamikhaylov I need more help, please. Can you teach me how to read the code and determine the input params for polygonAreaSpherical()? It looks like several points and an int.

@DanRoscigno DanRoscigno marked this pull request as ready for review May 23, 2023 23:14
@nikitamikhaylov
Copy link
Member

Oh, there is no easy way to determine the input params. From this line

size_t getNumberOfArguments() const override
we understand that the input param count is 1. And from the name of the function we can say that the input param is Polygon, which in fact has type Array(Array(Tuple(Float64, Float64)))

@DanRoscigno
Copy link
Contributor Author

Oh, there is no easy way to determine the input params. From this line

size_t getNumberOfArguments() const override

we understand that the input param count is 1. And from the name of the function we can say that the input param is Polygon, which in fact has type Array(Array(Tuple(Float64, Float64)))

Thanks, I must have been losing my mind; when I was looking at the test, I could have sworn I saw a polygon and a single tuple as args, and I was trying to figure out why the extra tuple/point was needed. Thanks again for your help!

@nikitamikhaylov
Copy link
Member

nikitamikhaylov commented May 24, 2023

Some cheat sheet:

Point is Tuple(Float64, Float64)
Ring is Array(Tuple(Float64, Float64)) - Array of points in clockwise order (maybe counter-clockwise, I don't remember)
Polygon is Array(Array(Tuple(Float64, Float64))) - First array define an external border, others define holes inside.
MultiPolygon is Array(Array(Array(Tuple(Float64, Float64))))

@DanRoscigno DanRoscigno merged commit 9b014eb into ClickHouse:master May 24, 2023
3 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
can be tested Allows running workflows for external contributors pr-documentation Documentation PRs for the specific code PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

some geo functions are not documented
3 participants