-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
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
Sql alchemy tests #4475
Sql alchemy tests #4475
Conversation
if retry: | ||
return tquery(sql, con=con, retry=False) | ||
if engine: | ||
engine.execute(str(sql), *params) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should probably use compat.text_type
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
embarrassingly don't even need to stringify :)
cc @danielballan, I went a bit mental creating engine specific methods/functions, I think it will make the logic easier to follow (need to clean a bit more), engine it makes dtype writing much easier. There is more refactoring to do. At the moment I just test with engine, but should probably have another test with connection (or even cursor) which hits the stuff you put together (again just be subclassing the test classes). I think abstracting away tests and main code ought to make all this much easier... I see light at the end of the tunnel. Currently seem to be having engine.execute issues when passing tuples (which I'm sure were ok earlier)... Looks like need to rebase after the 2to3 thing :s |
@hayd The tests look great, and I agree with your edits to |
hold off on the rebase, I think there was a big edit which hit sql last week, so I'll squash my commits and rebase this branch off master (I think there may be conflicts due to 2to3 and some pep8 cleaning. |
my objective for this pr is to have all the sqlalchemy tests working with SQLlite, without any platform specific stuff. Also, I think I'm using some bits wrong meta/execute so there is a lot of overhead to writing to database here, but we can sort that out once it's working. :S |
WIP first test working WIP more tests pass
@hayd shall we move SQL stuff to 0.14? put in some in 0.13? |
I think so, I'm not sure what we could include in 0.13, worried that it is completely untested (in real life and not even green lighting yet), and I don't think it properly falls back to the tested stuff (yet?), I think decide on class structure and merge soon after 0.13 release. Then add tests, and if by 0.14 it's not fully working then keep sql_legacy as the default. |
@hayd ok by me......will change the SQL stuff to 0.14 |
I agree - sorry folks wanting this in 0.13 - good to have more time. |
Yes, that would definitely be related, but as I wrote in the other issue, I |
Pandas has matured and sees less "earthquake" features since it handles it's a chance for someone to distinguish themselves by nailing it for pydata ppl. |
I'd like to have another try with this, to at least start the ball rolling. The master plan was to get SQLAlchemy working (for general dbs) but - as mentioned by many - we need the low level ODBC for performance, I think a good start to that project is the major refactor (which we've basically already done), cleaning up the API and the codebase. (That way it'll be easier to fit in the lower level ODBC late on.. ideally soon.) |
@mangecoeur, where is this in relation to #5950, superceded, subsumed? can you cherry pick something? |
@y-p I think this is mostly included in @mangecoeur's? The general idea was to write the all the tests in the base test class, which dialect test classes derive from, and include dialect specific parts as private methods. Old sql code has some things tested in one dialect that weren't in another... want to avoid this. |
Ok, that's good so can we close? |
WIP for #4163. Not ready to merge yet
New test structure for sql, and working through sql/test_sql with abstract syntax (aim is to not be sql-platform specific anywhere).