search

veekun / pokedex

more than you ever wanted to know about Pokémon
MIT License
1.44k stars 637 forks source link

Fix `pokedex search` command #301

Closed magical closed 4 years ago

magical commented 4 years ago

Filtering on pokemon name was broken. Not sure why, since the search CLI was added way after the i18n stuff was added. The error is related to AssociationProxy, which figures because nothing about association proxies ever seems to work right. I don't know enough about SQLAlchemy internals to know if what it was trying to do was supposed to work, or how to fix it if so. So, fix it by using the same boring join-based filtering that spline-pokedex uses.

Running pokedex search --name=gloom caused the following error with PostgreSQL and SQLAlchemy 1.3.5:

sqlalchemy.exc.ProgrammingError: (psycopg2.ProgrammingError) can't adapt type 'ColumnAssociationProxyInstance'

...and this error with SQLite.

sqlalchemy.exc.InterfaceError: (sqlite3.InterfaceError) Error binding parameter 4 - probably unsupported type.
[parameters: (9, 9, 9, 9, ColumnAssociationProxyInstance(AssociationProxy('names_local', 'name')), 'gloom')]

Additionaly, the following error would happen with PostgreSQL and SQLAlchemy 0.9.7, but that's probably unrelated:

sqlalchemy.exc.ProgrammingError: (ProgrammingError) subquery in FROM must have an alias
LINE 4: FROM pokemon_species, (SELECT pokemon_species_names.name AS ...
                               ^

Fixes #296

magical commented 4 years ago

@rluzuriaga, could you review this?

rluzuriaga commented 4 years ago

Okay so I deleted the previous comment since I forgot to check on PostgreSQL and just wanted to check on MySQL too. I just checked it on both 2.7.17 and 3.6.9 with SQLAlchemy 1.3.5 and 1.3.16 (newest version from requirement in setup.py). I used SQLite, PostgreSQL, and MySQL. It seems to be working correctly now (no errors at least).

I'm still not sure what can be done with the search command. When entering pokedex search --name=gloom all that gets returned is the text Gloom. It could just be me not knowing how it is supposed to be used.

magical commented 4 years ago

@rluzuriaga Thanks.

Yeah it's pretty useless. Eevee stubbed it out a few years ago but it never progressed any further. Ideally it would print out more info, and accept more filtering options. I think right now you can only filter on name and stats.

But it's super low priority.