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

fix(bq|h3,quadbin): H3_POLYFILL and QUADBIN_POLYFILL functions not working with holes #542

Merged

Conversation

vdelacruzb
Copy link
Contributor

@vdelacruzb vdelacruzb commented Jan 23, 2025

Description

Shortcut

This error started to happen because of ST_BOUNDINGBOX in Bigquery not working properly with polygons containing holes anymore. I used the internal ST_ENVELOPE function instead of ST_EXTERIORRING even if it's a JS function because ST_EXTERIORRING is crashing for multypolygons.

Type of change

  • Fix

Acceptance

The CI was also failing but thanks to this fix it should be green now.

QUADBIN_POLYFILL

with geo as (
  select ST_GEOGFROMTEXT('POLYGON((-5.73685493950322 41.5045088398422, -5.73685483649725 41.4674462934938, -5.73685137730796 41.4674199761992, -5.73684113282628 41.4673946702735, -5.73682449674882 41.4673713482076, -5.73680210839533 41.4673509062528, -5.73677482813928 41.4673341299788, -5.73674370434402 41.4673216640854, -5.7367099330749 41.4673139876274, -5.73667481213596 41.467311395605, -5.73667475369684 41.4673113956121, -5.6805002195605 41.4673113956121, -5.68046509750052 41.4673139790913, -5.68043132290769 41.4673216473342, -5.68040019371373 41.4673341056561, -5.68037290619151 41.4673508752933, -5.68035050898356 41.4673713118012, -5.68033386280364 41.4673946298192, -5.68032360735973 41.4674199332516, -5.6803201367697 41.4674462497034, -5.68032013676009 41.4674462934938, -5.68032003375412 41.5045088398422, -5.68032349477917 41.5045351571475, -5.68033374500085 41.5045604631036, -5.68035039051581 41.504583785215, -5.68037279165088 41.5046042272236, -5.68040008754463 41.5046210035513, -5.68043122922954 41.5046334694902, -5.6804650199435 41.5046411459787, -5.6805001026058 41.5046437380047, -5.73667487065155 41.5046437380047, -5.73671001070321 41.5046411374224, -5.73674379808549 41.5046334527135, -5.73677493436131 41.5046209791989, -5.73680222297642 41.5046041962314, -5.73682461524303 41.504583748774, -5.73684125064053 41.5045604226142, -5.73685148988454 41.5045351141657, -5.73685493950322 41.5045088398422), (-5.71119165112396 41.4857090384495, -5.71118140558165 41.485734344792, -5.71116476647128 41.485757667531, -5.71114237321992 41.4857781103841, -5.71111508638603 41.4857948877408, -5.71108395458896 41.4858073548538, -5.71105017421083 41.4858150326162, -5.71101504341955 41.4858176259742, -5.70230660556656 41.4858176259742, -5.70227148366637 41.4858150339412, -5.70223770277232 41.4858073574527, -5.70220657013748 41.4857948915139, -5.70217928217616 41.4857781151862, -5.70215688755104 41.4857576731776, -5.70214024687338 41.4857343510662, -5.70212999963047 41.4857090451101, -5.70212653961117 41.4856827278048, -5.702126558357 41.4789365460862, -5.70212655835724 41.4789365392961, -5.70213001992792 41.478910222132, -5.70214026792586 41.4788849165928, -5.70215690851935 41.4788615951544, -5.70217930221508 41.478841154044, -5.70220658843427 41.478824378799, -5.70223771858442 41.4788119140796, -5.70227149635597 41.4788042388954, -5.70230662369501 41.4788016481976, -5.7110150252911 41.4788016481976, -5.71101503435423 41.4788016481975, -5.71105016151917 41.4788042402199, -5.71108393877502 41.4788119166779, -5.7111150680877 41.4788243825713, -5.71114235317981 41.4788411588452, -5.71116474550218 41.4788616008001, -5.71118138452879 41.478884922866, -5.71119163082657 41.4789102287916, -5.71119509062911 41.4789365460862, -5.71119510937494 41.4856827278048, -5.71119165112396 41.4857090384495))') as geom
)
select `cartodb-data-engineering-team`.vdelacruz_carto.QUADBIN_POLYFILL_MODE(geom, 18, 'intersects') AS quadbin
from geo
-- Works

H3_POLYFILL

with geo as (
  select ST_GEOGFROMTEXT('POLYGON((-5.73685493950322 41.5045088398422, -5.73685483649725 41.4674462934938, -5.73685137730796 41.4674199761992, -5.73684113282628 41.4673946702735, -5.73682449674882 41.4673713482076, -5.73680210839533 41.4673509062528, -5.73677482813928 41.4673341299788, -5.73674370434402 41.4673216640854, -5.7367099330749 41.4673139876274, -5.73667481213596 41.467311395605, -5.73667475369684 41.4673113956121, -5.6805002195605 41.4673113956121, -5.68046509750052 41.4673139790913, -5.68043132290769 41.4673216473342, -5.68040019371373 41.4673341056561, -5.68037290619151 41.4673508752933, -5.68035050898356 41.4673713118012, -5.68033386280364 41.4673946298192, -5.68032360735973 41.4674199332516, -5.6803201367697 41.4674462497034, -5.68032013676009 41.4674462934938, -5.68032003375412 41.5045088398422, -5.68032349477917 41.5045351571475, -5.68033374500085 41.5045604631036, -5.68035039051581 41.504583785215, -5.68037279165088 41.5046042272236, -5.68040008754463 41.5046210035513, -5.68043122922954 41.5046334694902, -5.6804650199435 41.5046411459787, -5.6805001026058 41.5046437380047, -5.73667487065155 41.5046437380047, -5.73671001070321 41.5046411374224, -5.73674379808549 41.5046334527135, -5.73677493436131 41.5046209791989, -5.73680222297642 41.5046041962314, -5.73682461524303 41.504583748774, -5.73684125064053 41.5045604226142, -5.73685148988454 41.5045351141657, -5.73685493950322 41.5045088398422), (-5.71119165112396 41.4857090384495, -5.71118140558165 41.485734344792, -5.71116476647128 41.485757667531, -5.71114237321992 41.4857781103841, -5.71111508638603 41.4857948877408, -5.71108395458896 41.4858073548538, -5.71105017421083 41.4858150326162, -5.71101504341955 41.4858176259742, -5.70230660556656 41.4858176259742, -5.70227148366637 41.4858150339412, -5.70223770277232 41.4858073574527, -5.70220657013748 41.4857948915139, -5.70217928217616 41.4857781151862, -5.70215688755104 41.4857576731776, -5.70214024687338 41.4857343510662, -5.70212999963047 41.4857090451101, -5.70212653961117 41.4856827278048, -5.702126558357 41.4789365460862, -5.70212655835724 41.4789365392961, -5.70213001992792 41.478910222132, -5.70214026792586 41.4788849165928, -5.70215690851935 41.4788615951544, -5.70217930221508 41.478841154044, -5.70220658843427 41.478824378799, -5.70223771858442 41.4788119140796, -5.70227149635597 41.4788042388954, -5.70230662369501 41.4788016481976, -5.7110150252911 41.4788016481976, -5.71101503435423 41.4788016481975, -5.71105016151917 41.4788042402199, -5.71108393877502 41.4788119166779, -5.7111150680877 41.4788243825713, -5.71114235317981 41.4788411588452, -5.71116474550218 41.4788616008001, -5.71118138452879 41.478884922866, -5.71119163082657 41.4789102287916, -5.71119509062911 41.4789365460862, -5.71119510937494 41.4856827278048, -5.71119165112396 41.4857090384495))') as geom
)
select `cartodb-data-engineering-team`.vdelacruz_carto.H3_POLYFILL_MODE(geom, 2, 'intersects') AS quadbin
from geo
-- Works

@vdelacruzb vdelacruzb requested a review from cayetanobv January 23, 2025 10:32
@vdelacruzb vdelacruzb added dedicated_bigquery Deploys an instance of the AT into BigQuery dedicated_snowflake Deploys an instance of the AT into Snowflake and removed dedicated_snowflake Deploys an instance of the AT into Snowflake labels Jan 23, 2025
Copy link
Contributor

Dedicated environment removed

Copy link
Contributor

Dedicated dedicated_542_carto environment deployed in Bigquery project carto-dev-data

Copy link
Member

@cayetanobv cayetanobv left a comment

Choose a reason for hiding this comment

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

LGTM, tested with several use cases, including polygon with holes and multipolygons

@vdelacruzb vdelacruzb merged commit 14b2b5c into main Jan 23, 2025
16 of 17 checks passed
@vdelacruzb vdelacruzb deleted the bug/sc-464369/at-bq-polyfill-quadbin-for-geometries-with branch January 23, 2025 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dedicated_bigquery Deploys an instance of the AT into BigQuery
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants