{"action":"created","issue":{"url":"https://api.github.com/repos/pgpointcloud/pointcloud/issues/144","repository_url":"https://api.github.com/repos/pgpointcloud/pointcloud","labels_url":"https://api.github.com/repos/pgpointcloud/pointcloud/issues/144/labels{/name}","comments_url":"https://api.github.com/repos/pgpointcloud/pointcloud/issues/144/comments","events_url":"https://api.github.com/repos/pgpointcloud/pointcloud/issues/144/events","html_url":"https://github.com/pgpointcloud/pointcloud/pull/144","id":212712718,"number":144,"title":"[WIP] Add z and m to PCBounds","user":{"login":"elemoine","id":76594,"avatar_url":"https://avatars0.githubusercontent.com/u/76594?v=3","gravatar_id":"","url":"https://api.github.com/users/elemoine","html_url":"https://github.com/elemoine","followers_url":"https://api.github.com/users/elemoine/followers","following_url":"https://api.github.com/users/elemoine/following{/other_user}","gists_url":"https://api.github.com/users/elemoine/gists{/gist_id}","starred_url":"https://api.github.com/users/elemoine/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/elemoine/subscriptions","organizations_url":"https://api.github.com/users/elemoine/orgs","repos_url":"https://api.github.com/users/elemoine/repos","events_url":"https://api.github.com/users/elemoine/events{/privacy}","received_events_url":"https://api.github.com/users/elemoine/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":15,"created_at":"2017-03-08T11:48:02Z","updated_at":"2017-03-21T14:09:48Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/pgpointcloud/pointcloud/pulls/144","html_url":"https://github.com/pgpointcloud/pointcloud/pull/144","diff_url":"https://github.com/pgpointcloud/pointcloud/pull/144.diff","patch_url":"https://github.com/pgpointcloud/pointcloud/pull/144.patch"},"body":"I am creating this PR for discussion, on behalf of @mbredif and myself.\r\n\r\nCurrently, the [bounds](https://github.com/pgpointcloud/pointcloud/blob/60e10e31ef7114f7819456f4f9ed065e10278013/lib/pc_api.h#L147-L153) that we store in patches are 2D. This is a limitation.\r\n\r\nFor example, `PC_Intersects(p1, p2)`, which checks whether the bounds of `p1` and `p2` intersect, works in [2D only](https://github.com/pgpointcloud/pointcloud/blob/60e10e31ef7114f7819456f4f9ed065e10278013/lib/pc_util.c#L238-L249). Also, `geometry(p)` currently [returns](https://github.com/pgpointcloud/pointcloud/blob/60e10e31ef7114f7819456f4f9ed065e10278013/pgsql_postgis/pointcloud_postgis--1.0.sql#L16-L26) a 2D polygon. So with an index on `geometry(p)` only 2D spatial relationship tests are possible.\r\n\r\nTo address these limitations this PR suggests adding `zmin`, `zmax`, `mmin` and `mmax` to `PCBOUNDS`.\r\n\r\nOne issue is the change to the patch format, making \"old\" patch data incompatible with newer versions of pointcloud. I think this is not acceptable. One way to address that problem might be to use some bits of the [current patch header](https://github.com/pgpointcloud/pointcloud/blob/60e10e31ef7114f7819456f4f9ed065e10278013/lib/pc_api.h#L172-L178) to store some version number. For example, we could steal some bits from `type`, which only requires [4 bits](https://github.com/pgpointcloud/pointcloud/blob/60e10e31ef7114f7819456f4f9ed065e10278013/lib/pc_api.h#L36-L45) at the moment.\r\n\r\nWhat do you think? Do you agree with extending `PCBOUNDS` in general? Does the idea of stealing bits from `type` to encode some version in the data sound valid? Stupid? Do you see other ways to address the \"patch data compatibility\" issue?"},"comment":{"url":"https://api.github.com/repos/pgpointcloud/pointcloud/issues/comments/288089547","html_url":"https://github.com/pgpointcloud/pointcloud/pull/144#issuecomment-288089547","issue_url":"https://api.github.com/repos/pgpointcloud/pointcloud/issues/144","id":288089547,"user":{"login":"pramsey","id":113883,"avatar_url":"https://avatars0.githubusercontent.com/u/113883?v=3","gravatar_id":"","url":"https://api.github.com/users/pramsey","html_url":"https://github.com/pramsey","followers_url":"https://api.github.com/users/pramsey/followers","following_url":"https://api.github.com/users/pramsey/following{/other_user}","gists_url":"https://api.github.com/users/pramsey/gists{/gist_id}","starred_url":"https://api.github.com/users/pramsey/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pramsey/subscriptions","organizations_url":"https://api.github.com/users/pramsey/orgs","repos_url":"https://api.github.com/users/pramsey/repos","events_url":"https://api.github.com/users/pramsey/events{/privacy}","received_events_url":"https://api.github.com/users/pramsey/received_events","type":"User","site_admin":false},"created_at":"2017-03-21T14:09:48Z","updated_at":"2017-03-21T14:09:48Z","body":"Main comment is this is an invasive change for something that in 90% of cases won't help. At least in large scale LIDAR world, no? I found all the selectivity was in x/y, and most queries were taking in the majority of the range of z. Having full dimensional indexes is something that sounds like a good idea but doesn't help that much. If you have a use case where you **need** it, then by all means ignore me, but if you just think it **seems sensible** then take a pause."}}
{"actor":{"display_login":"pramsey"}}