Skip to content

[STBox] Do not set SRID for boxes without coordinates#17

Merged
estebanzimanyi merged 2 commits intoMobilityDB:developfrom
chaitan94:stbox-no-srid-for-time
Jul 7, 2020
Merged

[STBox] Do not set SRID for boxes without coordinates#17
estebanzimanyi merged 2 commits intoMobilityDB:developfrom
chaitan94:stbox-no-srid-for-time

Conversation

@chaitan94
Copy link
Copy Markdown
Member

@chaitan94 chaitan94 commented Jul 7, 2020 •
edited
Loading

Problem

I noticed this while working on MobilityDB/MobilityDB-python#3

The following query is not allowed by MobilityDB:

mobilitydb=# select stbox 'SRID=4326;GEODSTBOX T((,,2001-01-02 23:00:00+00),(,,2001-01-02 23:00:00+00))';
ERROR:  An SRID is specified but not coordinates are given
LINE 1: select stbox 'SRID=4326;GEODSTBOX T((,,2001-01-02 23:00:00+0...
                     ^

Which is fair, and a perfectly valid behavior - having an SRID where there are no coordinates does not make sense.

However, when you run the following, MobilityDB returns back with the SRID set to default value, even though it is not a valid representation according to itself:

mobilitydb=# select stbox 'GEODSTBOX T((,,2001-01-02 23:00:00+00),(,,2001-01-02 23:00:00+00))';
                                    stbox                                     
------------------------------------------------------------------------------
 SRID=4326;GEODSTBOX T((,,2001-01-02 23:00:00+00),(,,2001-01-02 23:00:00+00))
(1 row)

I feel MobilityDB should not output invalid representations. This results in failures when doing some kind of passthrough tests, and may result in unintended bugs with no direct reason for the underlying issue.

New behavior

After this change, this would be the resulting behavior:

mobilitydb=# select stbox 'GEODSTBOX T((,,2001-01-02 23:00:00+00),(,,2001-01-02 23:00:00+00))';
                               stbox                                
--------------------------------------------------------------------
 GEODSTBOX T((,,2001-01-02 23:00:00+00),(,,2001-01-02 23:00:00+00))
(1 row)

The fix is a one-line change (we add the condition to only append SRID prefix when coordinates present). Most other changes are in this PR are for tests.

Please let me know your thoughts on this.

@chaitan94 chaitan94 force-pushed the stbox-no-srid-for-time branch from a16987d to 8aeb5bd Compare July 7, 2020 07:02
@chaitan94 chaitan94 changed the base branch from master to develop July 7, 2020 08:14
@chaitan94 chaitan94 force-pushed the stbox-no-srid-for-time branch from 8aeb5bd to f3d91d9 Compare July 7, 2020 08:16
@estebanzimanyi estebanzimanyi merged commit 2bc456e into MobilityDB:develop Jul 7, 2020
@chaitan94 chaitan94 mentioned this pull request Jul 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@chaitan94 @estebanzimanyi