Skip to content

fix: ST_CoordDim failing with wrong return type #15203

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

fix: ST_CoordDim failing with wrong return type #15203

wants to merge 1 commit into from

Conversation

@jkhaliqi
Copy link
Collaborator

ST_CoordDim return type should be tinyint based on : prestodb/presto#26344

@netlify
Copy link

netlify bot commented Oct 17, 2025 •
edited
Loading

✅ Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 8cf1f42
🔍 Latest deploy log https://app.netlify.com/projects/meta-velox/deploys/68f273e2344bd60008be0a39

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 17, 2025
@jkhaliqi jkhaliqi requested a review from jagill October 17, 2025 16:15
@jagill
Copy link
Contributor

jagill commented Oct 17, 2025

Interesting. Presto docs have it returning bigint (which is also different than integer): https://prestodb.io/docs/current/functions/geospatial.html#ST_CoordDim-Geometry-bigint

Let me check the code...

@jagill
Copy link
Contributor

jagill commented Oct 17, 2025

The documentation is wrong, the Presto code has return type tinyint. I'll approve this but let's make sure the CI passes.

@jkhaliqi
Copy link
Collaborator Author

@jagill sounds good: posted a PR for presto documentation update as well : prestodb/presto#26355

@jagill jagill added the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Oct 17, 2025
@meta-codesync
Copy link

meta-codesync bot commented Oct 20, 2025

@pedroerp has imported this pull request. If you are a Meta employee, you can view this in D85060868.

@meta-codesync meta-codesync bot closed this in 956d630 Oct 20, 2025
@meta-codesync
Copy link

meta-codesync bot commented Oct 20, 2025

@pedroerp merged this pull request in 956d630.

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

Reviewers

@jagill jagill jagill approved these changes

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jkhaliqi @jagill @facebook-github-bot