API review

Proposer: Matei Ciocarlie and Lorenz Mosenlechner

Present at review:

  • Bhaskara
  • Caroline
  • Eitan
  • Marius
  • Matei

Useful information

The starting point for the documentation is the sql_database stack page. Most information can be found on the database_interface package page, particularly in the tutorials.

The database_interface does not have a true ROS API. It does have a C++ API, so feel free to discuss that here. Particular implementations of the database interface (such as the household_objects_database) do define ROS messages, so those are also fair game.

Feedback on the documentation and tutorials is also appreciated; write it here or use the separate doc review page.

Question / concerns / comments

Enter your thoughts on the API and any questions / concerns you have here. Please sign your name. Anything you want to address in the API review should be marked down here before the start of the meeting.

  • Eitan
    • Is there no way to auto-generate the C++ class containers. It seems like all the information in the class is contained in the database itself. For customizing private/public access to members or determining read/write permissions on fields, the user could edit the autogenerated template after the fact. I haven't poked around too much, but it seems like something like libpq might do the trick. Actually, it seems like you're using libpq already, does it not allow you to get the information you need from the database to support autogeneration?
    • /!\ not in this release

    • I know that this is an API review, but I'm also a bit worried that we're recreating the wheel. Is there no existing wrapper for databases? I found the following: http://soci.sourceforge.net/, it still requires you to write sql queries, but it doesn't seem so bad and supports a lot of different databases. Have you found that the tools that already exist are lacking?

    • Overall, I think that if the overhead of creating the C++ class containers were to go away, I might consider using the database stuff since it simplifies many common SQL queries. But right now, with my limited knowledge of SQL, I think I'd still find it easier to use something like SOCI. I'm just surprised that nothing like this exists already... I know that there are some decent python-based wrappers... I will admit that in my brief search I found nothing quite comparable in C++.
  • Marius
    • I also have a feeling that this is sort-of reinventing the wheel. There are several C++ database abstraction layers out there that are likely more mature (being developed for a longer time) and support many databases. There are several that support the object relational mapping similar to what database_interface provides.
    • It would be nicer to have an abstract database interface that gets implemented for each database type supported. The client code should not know which database type is dealing with (ideally), it should only use the generic interface.
      • /!\ would happen in a 0.3 release

      • but it make client code rigid! an interface can be added
    • PostgresqlDatabaseInterface is not really an interface (in an UML sense), it's a full implementation (name was misleading to me).

    • Any way to distinguish between a NULL table field and an empty one?
      • /!\ no, must investigate

    • Database schema must be created manually, it could potentially created automatically, all the info seems to be available in the class.
    • Does the object-relational mapping allow for one-to-many and many-to-many relations?
      • /!\ it allows one-to-one, but not others

  • Patrick
    • Let me third the concerns about re-inventing the wheel. To Marius' list I would add POCO Data (click on POCO Data User Guide link in right frame), part of a well-regarded library collection. It looks like it's based on SOCI. These libraries have all the features I want, a nice idiomatic C++ interface, and are already mature with support for various backend DBs.

    • Missing features I'd consider critical include:
      • Can't run arbitrary SQL queries (even simple ones like "select count(*) from...").
        • /!\ arbitrary SQL queries as long as there is a C++ class that wraps around the result - can be implemented in future release

        • run SQL queries that do not return anything
      • Can't create a table.
      • Can't create an index.
      • No prepared statements or parameter binding.
      • No transactions.
    • Can't process results row-by-row, you get them all at once.
      • /*\ generally useful, but not supported (think TB of data)
    • Defining container classes is clunky. I much prefer the non-intrusive POCO/SOCI model, where you tell the library (via template specialization) how to handle arbitrary user data types, which you can then bind to SELECT/INSERT statements.
    • For every subset of fields I want to select/insert, I have to write a different container class. I guess setAllFieldsReadFromDatabase and friends kind of get around this, but it seems like a very poor substitute for SELECT and INSERT to me.

      • /!\ current limitation, not easily fixable

    • What is the error handling model? Exceptions? The tutorial mentions a console error message, is that communicated to the application?
      • /!\ should have used exceptions from the beginning, but not critical for this release

    • I'm not sure Sequence is a primitive you can (or should) generalize across all SQL databases. Certainly SQLite doesn't have it. Instead support AUTOINCREMENT on the primary key, which can be implemented in different ways for different DBs.
      • /!\ handled differently by different backends

    • It's strange that you have to specify TEXT or BINARY for each field; the serialization used should be known from the data type.
      • /!\ this is important but really hard to get right, will look how alternatives support it

    • Power users will want some getNativeHandle() method for exploiting backend-specific features.

    • To be blunt, I think you need to reframe the focus of this stack. Certainly there's a need for it and there are good ideas here, but in the end what you're offering is a very limited object relational mapping to various SQL backends. There are already good solutions to this. As-is I would skip this library and use POCO Data or SOCI. The interesting question is, what value can you add specifically to ROS users?

      • IMO it would be hugely useful to start with something like POCO Data and make it trivial to insert/retrieve ROS message types. The simple approach is to store them as binary blobs, reusing the ROS serialization machinery.
      • Even better if you automatically map message fields to different DB table columns. E.g. I could insert a PoseStamped message and do spatial queries on (x,y) or search by timestamp. Something like this would have made the database munging for my kidnapped robot demo ridiculously easy.

      • Bonus points for both C++ and Python bindings. Actually the Python implementation would probably be much easier since you can introspect the messages, so that might be the best place to start prototyping.
  • Caroline
    • When you add a new entry, can it be automatically assigned a unique primary key? For certain data types? Oops, never mind, just read the advanced tutorial. Perhaps a pointer from the beginner tutorial to the correct section in the advanced tutorial is in order.
    • Simple queries are required.
    • I also think that automatically mapping ROS message fields to columns would be extremely useful and provide clear added value.
    • Really picky, brace yourself: If you're going to have tags like the following in the household_objects_database::ObjectsDatabase virtual bool getScaledModelsIcraExperimentSet (std::vector< boost::shared_ptr< DatabaseScaledModel > > &models) const // Gets a list of scaled models with "icra_experiment_set" true.

then you need to have better tags. Think of the interface in a year, when there are 20 such tags. Perhaps "icra_2010_willow_garage_demo" or "icra_2010_<paper title>".

  • Lorenz
    • It is pretty much work to make a C++ class persistent. I second the idea of a code generator or the use of third party library for the database interface.
    • Using the << and >> operators to serialize/deserialize data might cause problems for double fields. Since you want to index over these fields, they need to be of type TEXT. That means they always get serialized to a string although the lower-level database library supports doubles natively. The problem is that the standard << operator truncates digits. You might want to consider using something that works on the native data types.

    • Why is there a streamableFromString and streamableToString instead of implementing this functionality directly in the DBField class? In particular, if you want to define a custom binary type, there is no reason for implementing this function. But since the DBFieldData class which is the base class for DBField calls to these functions, they must be implemented no matter if they make sense or are used at all.

Meeting agenda

Two aspects:

  • object-oriented interface between C++ and SQL
    • covered here
    • BUT other packages already exist
      • SOCI
      • DTL
      • POCO
    • on the surface there *might* be missing features
      • binary (de)serialization
      • support for complex data types
      • etc.
    • support and maintenance too expensive
  • native interface between ROS data types and SQL database
    • does not really exist
      • we already have tools for generating and (de)serializing messages
      • one table for each message type
      • maybe written over an existing lower level library, such as SOCI
    • not covered by the package discussed here

Conclusion

  • /!\ address short term fixes for 0.2 release

  • {X} investigate use of SOCI as an alternative

  • /!\ establish common interface for database users

    • grasping database (Matei)
    • people database (Caroline)
    • recognition database (Marius)
    • places database (Patrick)
    • world db model (Lorenz)
  • /!\ investigate automated SQL interface for ROS messages


Wiki: sql_database/Reviews/07_2010_api_review (last edited 2010-07-23 23:13:37 by MateiCiocarlie)