adodb-datadict: adding of comments to tables and fields is missing

[peterdd]

addColumSQL()
alterColumnSQL()
changeTableSQL()

Postgresql: https://www.postgresql.org/docs/9.1/static/sql-comment.html
(extra sql command separated from create table definitions)

Mysql: http://dev.mysql.com/doc/refman/5.7/en/create-table.html
(at end of a field definition; at end of table definition)

Sqlite: http://stackoverflow.com/questions/7426205/sqlite-adding-comments-to-tables-and-columns
(seems to be possible with a workaround of sql comments (-- commenttext ) on the line behind table or field definition)

What I try to achieve: add comments to table and field definitions in an xmlschema03.xml, so a database created based on this xml contains all the comments and so is selfdocumenting.

This is supported by xmlschema03.dtd :

<descr>a comment for table or field inside a TABLE or FIELD tag</descr>

test/xmlschema03-test1.php

<?php

error_reporting(E_ALL);
include_once( "../adodb.inc.php" );
include_once( "../adodb-xmlschema03.inc.php" );

$dbs=array('mysql','postgres','sqlite','oci8');

$db = ADONewConnection( $dbs[0]);
$schema = new adoSchema( $db);
$sql = $schema->convertSchemaFile( "xmlschema.xml" );
#$sql = $schema->parseSchema( "xmlschema.xml" );
var_dump( $sql );

Methods setCommentSQL() and getCommentSQL() exists in adodb-datadict.inc.php as stub, but only datadict/datadict-oci.php adds some real code.

[mnewnham]

Have you considered adding the missing core functions yourself to move this issue along?

[peterdd]

How do you mean that? Should I start working on it and do pull request?

[mnewnham]

Yes most certainly, if you have the expertise and the time to build the addcommentsql() methods for any of MySQL, postgres and SQLite drivers that you mention above, we would be more than happy to pull them into the development branch

[peterdd]

I'm still waiting for review of #424, #425, and #265

Other blockers:

• The tests/* are so unflexible to configure and unsystematic and I'm focusing there first (will do some improvement pull requests, not perfect but a step forward and no dependencies)
• The other alterTableSQL() , changeTableSQL(), ... issues with different db types ( Postgres, but also others)

So I'm writing tests (primarly xmlschema) so we can get the big pictures across db types what works and what not/missing.

What @mnewnham and @dregad are using for testing?

[mnewnham]

@peterdd , there are currently no modern, standardized, automated tests that can be successfully applied to this product, across all databases, and as you yourself have noted, the tests/* product are weak and inflexible. That you are focusing on building a standardized test platform is great, and given the time constraints that @dregad and myself are under, you would find us extremely accommodating if you were able to come up with anything based on any well known testing platform.

That being said, adding something as granular as the addCommentSql() would likely not interfere with any of the other pull requests that you have mentioned, and merely adding support for the postgres database to further your own work on other projects would be entirely acceptable.

We recognize that nobody can hope to be able to cross-database test across the 20 or so supported platforms, and that addition of feature support has always been incremental, so if you need a working addCommentSql() method in the postgres driver, either for a current need or simply to further your work on testing, then you should go ahead and add it, then ask for a pull. Don't worry about the other drivers, or whether the code conforms to xyz standards or whether it will pass an abc system test, those things are to be concerned about in the future.

[peterdd]

@mnewnham , @dregad : Great to see increased activity for ADOdb, please not forget review/commit #256, #424, #425

I'm still interested in improving of xmlschema03 handling by ADOdb. DESCR-tags, but also other stuff (platform specific like setting prefered mysql 'engine' per table for instance).

[dregad]

please not forget review/commit #256, #424, #425

@peterdd I'm not sure what the status of this issue is. The PRs referenced above have been merged (I assume you actually meant #265)

Please confirm if this can be closed, or if there is more to be done - in this case are you planning to submit a PR ?

[peterdd]

Please keep it open as it is not implemented yet. Whenever I started I stumbled over other issues that needs to be fixed first...

I probably do some simple PR first, step by step. Maybe some function names and parameter changes need to be discussed too.