Skip to content
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.

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

Refactor to use pgmini for SQL construction and pycql2 for CQL parsing #149

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

bitner
Copy link
Contributor

@bitner bitner commented Dec 18, 2023

WORK IN PROGRESS

There are tons of print statements littered all over the place and other remnants from iterating to get tests to pass.

This PR refactors the building of SQL to use pgmini and pycql2.

Reasons for pgmini

  • pgmini has a cleaner syntax and requires far less use of 'escape hatches' with raw sql than buildpg.
  • pgmini has built in options to be able to support both asyncpg and psycopg bind parameter syntaxes
  • pgmini has easier hooks to be able to make stricter type casts while allowing more native use of postgres text input

Reasons for pycql2

  • pycql2 has a much smaller focused code base
  • pycql2 has a better test suite that can be leveraged to ensure full support of CQL2 syntax - these tests also provide examples of many different queries that should be supported
  • pycql2 has fuller support of the CQL2 text and json specification
  • pycql2 Lark much more closely derives from the actual BNF specification for CQL2 text

Additional Enhancements in this PR

  • Fixes numerous bugs with CQL2 expressions
  • Support for Array Operators
  • Support for all Temporal Operators
  • Enhancements for spatial operators to make sure that any transforms are performed to constant parameters when dealing with data that is not in geographic coordinates
  • Allows for the use of geography in addition to geometry which had very limited support previously

TODO

  • Update dependencies to remove buildpg / pygeofilter and add pycql2 and pgmini - this does require the non-merged branch of pycql2 with pydantic 2
  • Remove all print statements
  • Clean code up
  • Figure out how to get past ruff/mypy errors with reuse of sql function in cql2sql.py using plum dispatch (current ci set up is bombing and won't allow use of combined noref/ignore comments to get rid of errors
  • Add tests that compare CQL2 input to desired SQL output
  • Add tests for array operators
  • Add tests for temporal operators
  • Add tests for geography

@vincentsarago
Copy link
Member

@bitner 👋 anything I can do to help here?

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