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

feat: Support creating/dropping indices on tables #753

Merged
merged 7 commits into from
Sep 26, 2023

Conversation

tazarov
Copy link
Contributor

@tazarov tazarov commented Sep 21, 2023

  • Added support for creating/dropping indices (tested against SQLite and PG)

@tazarov tazarov requested a review from a team as a code owner September 21, 2023 07:18
@AzisK
Copy link

AzisK commented Sep 21, 2023

Could you also add this possibility in the documentation in the README?

@tazarov
Copy link
Contributor Author

tazarov commented Sep 21, 2023

@AzisK done, sorry for the oversight :)

README.rst Outdated Show resolved Hide resolved
pypika/tests/test_create.py Outdated Show resolved Hide resolved
- Fixed typo in docs
- Removed unnecessary str() in test_create.py's CreateIndexTests
pypika/queries.py Outdated Show resolved Hide resolved
@AzisK
Copy link

AzisK commented Sep 22, 2023

Good job in general, left one more comment

pypika/queries.py Outdated Show resolved Hide resolved
@AzisK
Copy link

AzisK commented Sep 22, 2023

It would be great to test on probably to most popular database, i.e. MySQL

@tazarov
Copy link
Contributor Author

tazarov commented Sep 22, 2023

It would be great to test on probably to most popular database, i.e. MySQL

let me do that real quick.

@tazarov
Copy link
Contributor Author

tazarov commented Sep 22, 2023

MySQL seems to work:

mysql> CREATE DATABASE my_database;
Query OK, 1 row affected (0.01 sec)

mysql> USE my_database;
Database changed
mysql> CREATE TABLE person (
    ->     id INT PRIMARY KEY AUTO_INCREMENT,
    ->     first_name VARCHAR(50),
    ->     last_name VARCHAR(50),
    ->     email VARCHAR(100),
    ->     date_of_birth DATE
    -> );
Query OK, 0 rows affected (0.05 sec)

mysql> INSERT INTO person (first_name, last_name, email, date_of_birth) VALUES
    -> ('John', 'Doe', '[email protected]', '1980-01-01'),
    -> ('Jane', 'Doe', '[email protected]', '1985-05-15');
Query OK, 2 rows affected (0.03 sec)
Records: 2  Duplicates: 0  Warnings: 0

mysql> CREATE INDEX my_index
    ->     ON person (first_name, last_name)
    -> ;
Query OK, 0 rows affected (0.09 sec)
Records: 0  Duplicates: 0  Warnings: 0

mysql> EXPLAIN SELECT * FROM person WHERE first_name = 'John' AND last_name = 'Doe';
+----+-------------+--------+------------+------+---------------+----------+---------+-------------+------+----------+-------+
| id | select_type | table  | partitions | type | possible_keys | key      | key_len | ref         | rows | filtered | Extra |
+----+-------------+--------+------------+------+---------------+----------+---------+-------------+------+----------+-------+
|  1 | SIMPLE      | person | NULL       | ref  | my_index      | my_index | 106     | const,const |    1 |   100.00 | NULL  |
+----+-------------+--------+------------+------+---------------+----------+---------+-------------+------+----------+-------+
1 row in set, 1 warning (0.05 sec)

- Removed return statements for @builder methods to align with rest of the code
- Removed unused dialect param
@tazarov
Copy link
Contributor Author

tazarov commented Sep 25, 2023

@AzisK do you want me to fix the lining issues?

@AzisK
Copy link

AzisK commented Sep 25, 2023

You mean linting, right? It would be good. You can also setup https://pre-commit.com/ and it will do it for you automatically

@tazarov
Copy link
Contributor Author

tazarov commented Sep 25, 2023

You mean linting, right? It would be good. You can also setup https://pre-commit.com/ and it will do it for you automatically

let me do that now.

@tazarov
Copy link
Contributor Author

tazarov commented Sep 25, 2023

I have the following pre-commit hooks yaml if you want I can commit that too for those that want linting locally:

repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.4.0
    hooks:
      - id: trailing-whitespace
      - id: mixed-line-ending
      - id: end-of-file-fixer
      - id: requirements-txt-fixer
      - id: check-yaml
        args: ["--allow-multiple-documents"]
      - id: check-xml
      - id: check-merge-conflict
      - id: check-case-conflict
      - id: check-docstring-first

  - repo: https://github.com/psf/black
    # https://github.com/psf/black/issues/2493
    rev: "refs/tags/23.3.0:refs/tags/23.3.0"
    hooks:
      - id: black

  - repo: https://github.com/PyCQA/flake8
    rev: 6.0.0
    hooks:
      - id: flake8
        args:
          - "--extend-ignore=E203,E501,E503"
          - "--max-line-length=88"

@AzisK
Copy link

AzisK commented Sep 25, 2023

I believe there is no need since the repo already has pre-commit-config defined

@tazarov
Copy link
Contributor Author

tazarov commented Sep 25, 2023

@AzisK I think we're good to go then

@AzisK
Copy link

AzisK commented Sep 26, 2023

Yes, everything looks good. I am merging this but the version bump will happen later. Thanks for your work again

@AzisK AzisK merged commit 5087884 into kayak:master Sep 26, 2023
5 checks passed
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