Fix cql parsing. Fixes #146 #154

ateucher · 2019-12-11T01:08:39Z

WIP for #146

Instead of early evaluation of nested calls as previously attempted, modify the call to wrap nested calls in local() so that dbplyr::partial_eval knows to evaluate them. #146

ateucher · 2019-12-12T22:04:12Z

Sorry to include an unrelated thing in here (fixing/testing RELATE, DWITHIN, etc.). But there's a new error which I can't figure out Using this branch:

local <- bcdc_query_geodata("regional-districts-legally-defined-administrative-areas-of-bc") %>%
  filter(ADMIN_AREA_NAME == "Cariboo Regional District") %>%
  collect()

bcdc_query_geodata("bc-parks-ecological-reserves-and-protected-areas") %>%
  filter(RELATE(local, "*********")) %>%
  collect()
#> Warning: The object is too large to perform exact spatial operations using bcdata.
#>              To simplify the polygon, a bounding box was drawn around the polygon and all
#>              features within the box will be returned. Options include further processing
#>              with on the returned object or simplify the object.
#> Warning in bcdc_number_wfs_records(query_list, cli): NAs introduced by coercion
#> Error in if (number_of_records < 10000) {: missing value where TRUE/FALSE needed

Any thoughts?

ateucher · 2019-12-12T22:16:20Z

If I show_query() and paste the URL into a browser it works:

<url>
<body>
   SERVICE: WFS
   VERSION: 2.0.0
   REQUEST: GetFeature
   outputFormat: application/json
   typeNames: WHSE_TANTALIS.TA_PARK_ECORES_PA_SVW
   SRSNAME: EPSG:3005
   CQL_FILTER: (RELATE(SHAPE, POLYGON ((956376 653960.8, 1397042 653960.8, 1397042 949343.3, 956376 949343.3, 956376 653960.8)), *********))

<full query url>
https://openmaps.gov.bc.ca/geo/pub/wfs?SERVICE=WFS&VERSION=2.0.0&REQUEST=GetFeature&outputFormat=application%2Fjson&typeNames=WHSE_TANTALIS.TA_PARK_ECORES_PA_SVW&SRSNAME=EPSG%3A3005&CQL_FILTER=%28RELATE%28SHAPE%2C%20POLYGON%20%28%28956376%20653960.8%2C%201397042%20653960.8%2C%201397042%20949343.3%2C%20956376%20949343.3%2C%20956376%20653960.8%29%29%2C%20%2A%2A%2A%2A%2A%2A%2A%2A%2A%29%29

boshek · 2019-12-12T23:44:23Z

Interesting. The argument isn't even recognized using the CRAN version but the error also shows up in the master branch of this repo.

library(bcdata)
#> 
#> Attaching package: 'bcdata'
#> The following object is masked from 'package:stats':
#> 
#>     filter

packageVersion("bcdata")
#> [1] '0.1.1'

local <- bcdc_query_geodata("regional-districts-legally-defined-administrative-areas-of-bc") %>%
  filter(ADMIN_AREA_NAME == "Cariboo Regional District") %>%
  collect()
#> Warning: It is advised to use the permanent id ('d1aff64e-dbfe-45a6-af97-582b7f6418b9') rather than the name of the record ('regional-districts-legally-defined-administrative-areas-of-bc') to guard against future name changes.

bcdc_query_geodata("bc-parks-ecological-reserves-and-protected-areas") %>%
  filter(RELATE(local, "*********")) 
#> Error in RELATE(structure(list(id = "WHSE_LEGAL_ADMIN_BOUNDARIES.ABMS_REGIONAL_DISTRICTS_SP.18", : unused argument ("*********")

^{Created on 2019-12-12 by the reprex package (v0.3.0)}

boshek · 2019-12-13T00:18:55Z

Some further info which doesn't directly address the problem but is worth noting. COUNT (which is used for the print method for bcdc_promise) appeards not be a valid argument when using RELATE

With COUNT (fail):

https://openmaps.gov.bc.ca/geo/pub/wfs?SERVICE=WFS&VERSION=2.0.0&REQUEST=GetFeature&outputFormat=application%2Fjson&typeNames=WHSE_TANTALIS.TA_PARK_ECORES_PA_SVW&SRSNAME=EPSG%3A3005&CQL_FILTER=%28WITHIN%28SHAPE%2C%20POLYGON%20%28%28956376%20653960.8%2C%201397042%20653960.8%2C%201397042%20949343.3%2C%20956376%20949343.3%2C%20956376%20653960.8%29%29%29%29&COUNT=6

Without COUNT (success):

https://openmaps.gov.bc.ca/geo/pub/wfs?SERVICE=WFS&VERSION=2.0.0&REQUEST=GetFeature&outputFormat=application%2Fjson&typeNames=WHS
E_TANTALIS.TA_PARK_ECORES_PA_SVW&SRSNAME=EPSG%3A3005&CQL_FILTER=%28RELATE%28SHAPE%2C%20POLYGON%20%28%28956376%20653960.8%2C%201397042%20653
960.8%2C%201397042%20949343.3%2C%20956376%20949343.3%2C%20956376%20653960.8%29%29%2C%20%2A%2A%2A%2A%2A%2A%2A%2A%2A%29%29

With COUNT and using WITHIN (success):

https://openmaps.gov.bc.ca/geo/pub/wfs?SERVICE=WFS&VERSION=2.0.0&REQUEST=GetFeature&outputFormat=application%2Fjson&typeNames=WHSE_TANTALIS.TA_PARK_ECORES_PA_SVW&SRSNAME=EPSG%3A3005&CQL_FILTER=%28WITHIN%28SHAPE%2C%20POLYGON%20%28%28956376%20653960.8%2C%201397042%20653960.8%2C%201397042%20949343.3%2C%20956376%20949343.3%2C%20956376%20653960.8%29%29%29%29&COUNT=6

boshek · 2019-12-13T00:29:27Z

Ok so I haven't completely figured out why but when using RELATE adding any additional parameters to the query list doesn't seem to work. So at this line:

bcdata/R/utils.R

Line 28 in e81c926

query_list <- c(resultType = "hits", query_list)

we add the parameter to get the total number of records. That wfs sever doesn't like that request and so no number is returned. If I am right, I think at this time our only option is to remove RELATE as a possible predicate.

ateucher · 2019-12-13T16:51:36Z

Ok, thanks for digging in. I'm fine with removing it. Should we just not export it? Or do you think remove the code completely?

boshek · 2019-12-13T17:28:36Z

Fine to just not export. We need a news item as well.

ateucher added 9 commits December 9, 2019 17:45

DWITHIN and BEYOND like unquoted unit names

4b64a26

eval nested functions

9cc2c7a

tests for CQL predicates with nested functions

2242424

scratch

56f1915

fix tests for DWITHIN & BEYOND (unquoted units)

9ec168a

More tests for #146

c15a132

Use call2 to wrap function calls in local()

7b1a93c

Instead of early evaluation of nested calls as previously attempted, modify the call to wrap nested calls in local() so that dbplyr::partial_eval knows to evaluate them. #146

Merge branch 'master' into fix-cql-parsing

66defa0

Import := from rlang, document and fix test

8fe02ec

ateucher changed the title ~~Fix cql parsing~~ Fix cql parsing. Fixes #146 Dec 12, 2019

ateucher marked this pull request as ready for review December 12, 2019 21:29

ateucher requested a review from boshek December 12, 2019 21:29

ateucher added 2 commits December 12, 2019 13:48

RELATE string should also be unquoted

711b385

Test for RELATE

e4063f5

ateucher added 2 commits December 13, 2019 09:09

Don't export or test RELATE. #154

8bb8d7e

Merge remote-tracking branch 'origin/master' into fix-cql-parsing

b403a37

ateucher added 2 commits December 13, 2019 10:25

BEYOND not supported, don't export/test

e00b5a6

Update NEWS

58bfe43

boshek approved these changes Dec 13, 2019

View reviewed changes

boshek merged commit 2dd503e into master Dec 13, 2019

boshek deleted the fix-cql-parsing branch December 13, 2019 20:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix cql parsing. Fixes #146 #154

Fix cql parsing. Fixes #146 #154

ateucher commented Dec 11, 2019

ateucher commented Dec 12, 2019 •

edited

Loading

ateucher commented Dec 12, 2019 •

edited

Loading

boshek commented Dec 12, 2019 •

edited

Loading

boshek commented Dec 13, 2019

boshek commented Dec 13, 2019

ateucher commented Dec 13, 2019

boshek commented Dec 13, 2019

Fix cql parsing. Fixes #146 #154

Fix cql parsing. Fixes #146 #154

Conversation

ateucher commented Dec 11, 2019

ateucher commented Dec 12, 2019 • edited Loading

ateucher commented Dec 12, 2019 • edited Loading

boshek commented Dec 12, 2019 • edited Loading

boshek commented Dec 13, 2019

boshek commented Dec 13, 2019

ateucher commented Dec 13, 2019

boshek commented Dec 13, 2019

ateucher commented Dec 12, 2019 •

edited

Loading

ateucher commented Dec 12, 2019 •

edited

Loading

boshek commented Dec 12, 2019 •

edited

Loading