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

Case insensitive username and email indexing and query planning for Postgres #6506

Merged
merged 54 commits into from
Apr 3, 2020

Conversation

cbaker6
Copy link
Contributor

@cbaker6 cbaker6 commented Mar 14, 2020

Initial code, still needs to be fixed

cbaker6 added 17 commits March 10, 2020 22:11
testing error to see what happens...
Attempting to resolve postgres in CL by installing postgis via sudo instead of through apt/packages
Removed extra lines of postgres that were under "services" and "addons". I believe the "postgresql" line under "services" was installing the default of 9.6 and "addons" was installing postgres 11. My guess is the fail was occurring due to 9.6 being called sometimes and it never had postgis installed. If this is true, the solution is to only install one version of postgres, which is version 11 with postgis 2.5.
Adding test case for verifying indexing for caseInsensitive
…has a minimum requirement of postgis 2.2. Documented the change in the readme. This is address #6441
…Also switched postgis image it points to as the other one hasn't been updated in over a year.
…contributions. The official image automatically creates a user named 'postgres', but it does require a password, which the command sets to 'postgres'
…will always take a few seconds because the db is installing from scratch everytime. If postgres/postgis images aren't already downloaded locally, it will take even longer. Worst case, if the command times out on first run. Stop and remove the parse-postgres container and run the command again, 20 seconds should be enough wait time then
@codecov
Copy link

codecov bot commented Mar 21, 2020

Codecov Report

Merging #6506 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6506   +/-   ##
=======================================
  Coverage   93.89%   93.89%           
=======================================
  Files         169      169           
  Lines       11959    11959           
=======================================
  Hits        11229    11229           
  Misses        730      730           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5759694...5759694. Read the comment docs.

@dplewis
Copy link
Member

dplewis commented Mar 21, 2020

@cbaker6 Can you check the travis build? You need to run the linter.

cbaker6 added 11 commits March 21, 2020 19:00
Adding test case for verifying indexing for caseInsensitive
…has a minimum requirement of postgis 2.2. Documented the change in the readme. This is address #6441
…Also switched postgis image it points to as the other one hasn't been updated in over a year.
…contributions. The official image automatically creates a user named 'postgres', but it does require a password, which the command sets to 'postgres'
…will always take a few seconds because the db is installing from scratch everytime. If postgres/postgis images aren't already downloaded locally, it will take even longer. Worst case, if the command times out on first run. Stop and remove the parse-postgres container and run the command again, 20 seconds should be enough wait time then
cbaker6 added 7 commits March 22, 2020 21:56
…(this can be bad when looking at a query plan for Insert, Delete, etc.) the query or to just setup the query plan (default, previous versions defaulted to 'analyze'). Alse added some comparsons on sequential vs index searches for postgres
…p time comparison between searches as this seemed to be variable
…an username and password. Also added explain to aggregate method
cbaker6 added 4 commits March 23, 2020 20:05
…gres (similar to old style). Note that the official postgres docker image for postgres requires POSTGRES_PASSWORD to be set in order to use the image
…version to be backwards compatibile with older versions of parse server
@cbaker6 cbaker6 changed the title Case insensitive username and email indexing for Postgres Case insensitive username and email indexing and query planning for Postgres Mar 24, 2020
@cbaker6
Copy link
Contributor Author

cbaker6 commented Mar 26, 2020

@cbaker6 Can you fix the travis build? Also are you able to run the tests locally for PG?

I missed this comment, but yes I can run the postgres adapter tests locally. I couldn't run the whole spec locally because of the bugs in #6531. I run postgres via docker locally along with some remote servers I have deployed. Both fail, which led me to fixes in 6531. Now I can run locally and remotely

@acinader
Copy link
Contributor

acinader commented Apr 3, 2020

@dplewis you ok with this?

also want to see the tests pass as master is currently hanging on the postgres test.

@acinader
Copy link
Contributor

acinader commented Apr 3, 2020

ok. tests look good....

@dplewis
Copy link
Member

dplewis commented Apr 3, 2020

@cbaker6 Since #6531 is merged can you resolve the conflicts? Also checkout #6531 (comment)

Copy link
Member

@dplewis dplewis left a comment

Choose a reason for hiding this comment

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

LGTM! I also like the query.hint() support on Postgres

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.

3 participants