SQL: Status of the legacy mode?

[jorisvandenbossche]

To discuss: do we regard the sql legacy mode (using a DBAPI2 connection, for sqlite and mysql) as a 'first-class citizen' in pandas? Or is it mainly for backwards compatibility?
Is it useful to have something that is not dependent on sqlalchemy? And consequently we alo try to enhance it /accept enhancements to it? Or is it really just 'legacy' and we don't develop it further (only bug fixes)?

I think it is important to discuss this (and 'formally' decide on this), as there are some issues with this:

• If it is mainly for backwards compatibility, are we also planning to deprecate it? (which is not necessarily the consequence, if we just regard it as what the name says: legacy)

• If it is not only for backwards compatibility, why do we raise a warning?:

  UserWarning: Not an SQLAlchemy engine,
  attempting to use as legacy DBAPI connection
  

  As this implies to me that it is not really preferred to use this.

• Do we want to keep the functionality equal to the sqlalchemy version? (as we are using the same functions for both, it would make it complex if it is not)

  • Eg, we could easily provide a read_table functionality to legacy mode, by just doing a read_sql("SELECT * FROM specified_table", ..).

• We should clarify the docs on this status: http://pandas-docs.github.io/pandas-docs-travis/io.html#legacy (as it is not really clear now)

• If we decide we want to fully support this, we should maybe try to find another name than 'legacy' .. :-)

Maybe the bottom line we should ask ourselves is: do we want that we can really recommend it to users to use the legacy sqlite without sqlalchemy version? (and in that case I think it should be fully supported and not regarded as 'legacy')

[jorisvandenbossche]

We already started the discussion here (#6881 (comment)), to quote @danielballan:

In the long term, I'd like to keep full-fledged support for connections generated by sqlite3. For light applications, I still favor those over sqlalchemy for code portability (e.g., to collaborators who might not have sqlaclemy). Since all other flavors already require dependencies that are not built in to Python, I think it is reasonable to expect users to also obtain sqlalchemy if they want the very best support. One man's opinion.

So, I'd like to build MultiIndex support for legacy connections.

[jreback]

I think it might make sense to have full-fledged support for sqlite3 DBAPI only as it doesn't have any deps, while all other flavors have the python driver deps at the very least.

So support sqlite3 thru direct DBAPI and sqlalchemy, whilst other flavors ONLY with sqlalchemy. If at some point their is sufficient interest / PR's for supporting other direct DBAPI then could be added later.

[jorisvandenbossche]

@hayd @danielballan @mangecoeur @jtratner

And if we decide for a full-fledged support of sqlite3, this does also mean we deprecate the MySQL support via DBAPI connection (without sqlalchemy)?
Also, only supporting sqlite3 without sqlalchemy makes sense in the sense that it is the simplest flavor to support fully.

[hayd]

I kind of thing we should try and bring back the DBAPI support for this release (and potentially depreciate it, at least for mysql/postgres etc), it's going to shock people that their sql code is broken without warning/dep (which is what's happening atm??).

I've tried a few times to get the old tests working, but haven't been able to fix the mysql issues, so would be great if someone else could have a try!

[jorisvandenbossche]

@hayd There is nothing to 'bring back', as DBAPI is still supported via legacy mode. Normally all code that did run previously, should also run now (as only sqlite and mysql flavors were supported, postgresql was never supported). That is certainly something we have to guarantee. But of course if the tests say otherwise, we should look at that ..

Previously, only 'sqlite' and 'mysql' flavors were really supported, and those are now both supported via the legacy mode. Of course, some things previously also worked for other flavors, eg the read_sql to read a query is rather flavor independent and did also work for eg postgresql.
So the question is also if we continue to 'allow' this, or explicitly raise an error and point to sqlalchemy mode if a DPAPI connection is provided that is not sqlite (or mysql at this moment).

[danielballan]

Yes, let's make sure the legacy tests are in order but deprecate legacy connections in future releases.

This way, we will not waste effort trying to fix bugs in the legacy code -- the bugs that motivated us to adopt sqlalchemy to begin with -- but neither will we pull out the rug from people who are using the legacy connections now and finding them adequate.

[jorisvandenbossche]

@danielballan With 'deprecate legacy connections in future releases', do you also mean sqlite? Or only the others, because in the other thread you were in favor of a fully supported sqlite fallback?

I will take a look at the old tests, and try to get them running with the new code.

[danielballan]

Yes, that was ambiguous. I am still in favor of continued fully sqlite3 support. I mean that we should deprecate the others.

(Incidentally, supporting sqlite3 is a little less work because it only supports five datatypes.)

[jorisvandenbossche]

yeah, that is what I meant with it being the 'simplest flavor to support fully.'. Actually just int, real and text (and null and blob). No bool, no datetime in different forms, ..

[jreback]

so sqlalchemy prob writes datetimes as string and does the conversions? (for sqlite3)

[danielballan]

@jreback Yes, that's right. As I explored that, I discovered that sqlalchemy cannot write timedeltas. It raises a StatementError. I'll open a separate issue.

[jorisvandenbossche]

And actually (I didn't realize before), sqlalchemy converts it to string, but keeps it as a 'datetime' column in the database (in sqlite, you can use declared data type as datetime as an alias, it's just not a datetime but a string), but in this way, also when reading that frame back in, sqlalchemy automatically converts it back to a datetime

[hayd]

@jorisvandenbossche bring back the old test file (to ensure we're not breaking too much stuff), sorry.

[danielballan]

@hayd Is the latest copy of test_legacy_sql.py sitting the history of the master branch? I know I have an old copy in one of my SQL branches. Have you done recent work on it?

[hayd]

@danielballan I haven't done any recent work on it, although there was a couple of additional tests (e.g. not deleteing the database when in append mode) which could be added. I had tweaked it a little (e.g remove removed arguments), but mysql was choking/taking forever on the tests.

[jorisvandenbossche]

@hayd @danielballan shouldn't we just look at the test_sql.py from the 0.13.1 tagged version? Or were there changes done to the tests after that?

[hayd]

@jorisvandenbossche that's right, no changes were added so we should just use that version's tests (potentially could add on for table deleting when trying to append ?).

[jorisvandenbossche]

I looked at the old tests, and most are running now (submitted PR #6987 to fix some issues).

What do the others think of deprecating the 'mysql' flavor for the legacy code with DBAPI connections? And only keep full support for sqlite without sqlalchemy?
Because if we would do that, we could better do it now I think.

[danielballan]

@jorisvandenbossche Yes.

We want to get out of the business of supporting flavors' various idiosyncrasies. We've established that making an exception for sqlite3 makes sense. But if we support 'mysql' and not postgresql, oracle, etc. it will only be for historical reasons, and I worry that the code won't be well maintained.

Speaking as someone who will have a lot of his code broken by this change, I still think it's the right choice. If the deprecation warning incites a riot we can always reevaluate....

[jorisvandenbossche]

So I will go forward:

• deprecate 'mysql' flavor (and point to use sqlalchemy)
• remove the UserWarning of not being a sqlalchemy engine

Somebody a proposal for another name than 'legacy'? Something like sqlite3 'fallback'?

@danielballan Normally your code should not be broken (yet), as the changes should be almost fully backwards compatible. And to adapt, I think it changing the connection an sqlalchemy engine will be enough in most cases as the interface stayed the same (but of course, this can also be a lot of work ..)

[jorisvandenbossche]

OK, then going forward with:

• deprecate mysql flavor
• remove the UserWarning of not using an sqlalchemy engine

Somebody a good idea how to name the 'legacy' mode, eg sqlite 'fallback'?

Normally, your code should not be broken (yet), as the changes are almost fully backwards compatible. And to adapt, the main thing to change is replacing the mysql connections with sqlalchemy engines as the interface should be the same for the rest (but of course, not saying that this + some of the corner cases could not be a lot of work ...)