Skip to content

Fix malformed CQL from spatial filters#369

Open
boshek wants to merge 4 commits into
mainfrom
fix/cql-translation
Open

Fix malformed CQL from spatial filters#369
boshek wants to merge 4 commits into
mainfrom
fix/cql-translation

Conversation

@boshek

@boshek boshek commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Fixes a bug where filter() calls using CQL geometry predicates (such as INTERSECTS()) produced malformed CQL that the server rejected with a 400 error.

The reason the CQL was malformed was because CQL leaked a spurious TRUE AS "drop_null" clause and extra parentheses following the removal of c.sql() method in dbplyr 2.6.0 (tidyverse/dbplyr#1691).

Closes (#368)

@boshek boshek requested a review from ateucher June 23, 2026 16:42
@boshek boshek marked this pull request as ready for review June 23, 2026 16:42

@ateucher ateucher left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this @boshek! A couple of comments below

Comment on lines +199 to +206
single <- cql(filter(promise, INTERSECTS(bbox)))
expect_false(grepl("drop_null", single))
expect_false(grepl("^\\(\\(", single))

# A chained second clause AND-joins cleanly, still without the artifact.
multi <- cql(filter(filter(promise, INTERSECTS(bbox)), BGC_LABEL != "ZZZ"))
expect_false(grepl("drop_null", multi))
expect_match(multi, "AND \\(\"BGC_LABEL\" != 'ZZZ'\\)")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this is a good place for snapshot tests - less messy grepping, and I think it would be nice to visually inspect the CQL

Comment thread R/utils-classes.R
Comment on lines +332 to +334
combined_cql <- c(unclass(.data$query_list$CQL_FILTER), unclass(current_cql))
combined_cql <- combined_cql[nzchar(combined_cql)]
.data$query_list$CQL_FILTER <- dbplyr::sql(combined_cql)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be simplified to just combine them with dbplyr::sql():

Suggested change
combined_cql <- c(unclass(.data$query_list$CQL_FILTER), unclass(current_cql))
combined_cql <- combined_cql[nzchar(combined_cql)]
.data$query_list$CQL_FILTER <- dbplyr::sql(combined_cql)
.data$query_list$CQL_FILTER <- dbplyr::sql(
.data$query_list$CQL_FILTER,
current_cql
)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants