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 make_geohash function #1265

Merged
merged 12 commits into from
Sep 6, 2021
Merged

Add make_geohash function #1265

merged 12 commits into from
Sep 6, 2021

Conversation

eugenels
Copy link
Contributor

make_geohash(lon, lat, bits)

Copy link
Contributor

@marregui marregui left a comment

Choose a reason for hiding this comment

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

Would it make sense to check the args to see whether they are constants as in EqGeoHashGeoHashFunctionFactory? If so, then a constant function can be returned with the value calculated.
There might be a missing test for getGeoHashByte.

@ideoma
Copy link
Collaborator

ideoma commented Sep 6, 2021

[PR Coverage check]

😍 pass : 27 / 28 (96.43%)

file detail

path covered line new line coverage
🔵 io/questdb/cairo/GeoHashes.java 6 7 85.71%
🔵 io/questdb/griffin/engine/functions/rnd/RndGeoHashFunctionFactory.java 1 1 100.00%
🔵 io/questdb/griffin/engine/functions/geohash/GeoHashFromCoordinatesFunctionFactory.java 20 20 100.00%

@bluestreak01 bluestreak01 dismissed marregui’s stale review September 6, 2021 14:48

need to expedite the PR, can optimise later

@bluestreak01 bluestreak01 merged commit 0e83cc7 into master Sep 6, 2021
@bluestreak01 bluestreak01 deleted the gh_from_coord branch September 6, 2021 14:48
marregui pushed a commit that referenced this pull request Sep 6, 2021
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.

None yet

4 participants