fix(sf|quadbin): QUADBIN_FROMLONGLAT not clamping latitudes and return some quadbin functions return NULL when NULL parameters

[vdelacruzb]

Description

Shortcut

• Story: https://app.shortcut.com/cartoteam/story/322342/quadbin-fromlonglat-not-clamping-latitudes-successfully
• Autolink: [sc-322342]

The issue was only detected in this function. Also started returning NULL from some SQL functions that were returning non easy to debug errors for the user.

Type of change

• Fix

Acceptance

SELECT CARTO_DEV_DATA.carto.QUADBIN_FROMLONGLAT(-3.7038, 100, 4);
-- CRASH: 100043 (2201E): Invalid floating point operation: log(e,-11.4301)
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_FROMLONGLAT(-3.7038, 100, 4);
-- WORKS: 5206548197333270527

-- All these functions return NULL now
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_FROMQUADKEY(NULL);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_FROMZXY(NULL, 7, 6);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_FROMZXY(4, NULL, 6);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_FROMZXY(4, 7, NULL);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_KRING_DISTANCES(NULL, 1);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_KRING_DISTANCES(5207251884775047167, NULL);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_KRING(NULL, 1);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_KRING(5207251884775047167, NULL);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_SIBLING(NULL, 'up');
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_SIBLING(5207251884775047167, NULL);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_TOPARENT(NULL, 3);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_TOPARENT(5207251884775047167, NULL);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_TOPARENT(5207251884775047167, -1);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_TOPARENT(5207251884775047167, 27);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_TOQUADKEY(NULL);
SELECT CARTO_DATA_ENGINEERING_TEAM.vdelacruz_carto.QUADBIN_TOZXY(NULL);

[shortcut-integration]

This pull request has been linked to Shortcut Story #322342: QUADBIN_FROMLONGLAT not clamping latitudes successfully.

[vdelacruzb]

I considered adding tests, but not sure if it's worth it as this maybe should be the default behaviour.

[jgoizueta]

The implementation is fine, and the result NULLs a big improvement.
But I don't agree with the idea of clamping (GREATEST(-85.05, LEAST(85.05, latitude))) because we altering the input point. I think that QUADBIN_FROMLONGLAT should fail with some input coordinates out of range for QUADBIN error.

[jgoizueta]

Perhaps we could add some clamping function, and even some "SAFE" function to cover different use cases:

QUADBIN_FROMLONGLAT(-3.7038, 100, 4) -- ERROR: coordinates out of valid QUADBIN range
QUADBIN_FROMCLAMPLEDLONGLAT(-3.7038, 100, 4) -- 5206548197333270527
SAFE_QUADBIN_FROMLONGLAT(-3.7038, 100, 4) -- NULL

[jgoizueta]

Another detail: for clamping I see no problem with using an approximation to the limit value like 85.05, but for detecting the limit to return an error or NULL (if we implement the functions I propose), we should use the more exact value 85.05112877980659 (SELECT 0.5 * (4.0 * ATAN(EXP(ACOS(-1))) - ACOS(-1)) * 180.0 / ACOS(-1)). Otherwise we would reject some valid points.

[vdelacruzb]

Regarding clamping is the solution implemented in other providers. But you are right maybe it's better to have two versions of this function. Or by default show an error to the user.
Don't know how worth it is to have another public function in the AT because it's supposed to be an edge case.

What do you think @ernesmb?

[jgoizueta]

I don't like clamping by default, but for consistency with the other providers we should apply it in SF as well. We'll discuss what to do next for all providers about this.

[ernesmb]

yeah I agree with @jgoizueta 's last comment. Let's apply the same we're doing with other providers and then later think about a possible better solution in all providers