createTableSql() not quoting reserved keyword fieldnames

[peterdd]

The function accepts parameter $flds as string or array (contrary to what is written on https://adodb.org/dokuwiki/doku.php?id=v5:dictionary:createtablesql)

adodb-xmlschema03.inc.php calls it:

$sql[] = $xmls->dict->createTableSQL( $this->name, $fldarray, $this->opts );

passing the array of fields.

Maybe need to be solved by function nameQuote().
It only quotes the names if there are special chars or brackets are used. But probably should quote also reserved keywords.

The default could use standard sql
( https://www.postgresql.org/docs/current/sql-keywords-appendix.html ) and adjust
the list by overwriting by drivers datadict/datadict-$driver.php

Or we just quote every fieldname except the fieldnames that have yet some kind of quoting ( ` , brackets)

Example with postgresql

<?php

include_once "ADOdb/adodb.inc.php";
include_once "ADOdb/adodb-xmlschema03.inc.php";

$driver='postgres';
$host='127.0.0.1';
$user='adodb';
$password='adodb';
$database='adodbtest';

$db = ADONewConnection($driver);
$db->debug=true;
$db->connect($host, $user, $password, $database);
$schema = new adoSchema($db);

$xml='<?xml version="1.0"?>
<schema version="0.3">
        <table name="meta_database">
                <field name="id" type="I" size="10">
                        <AUTOINCREMENT/>
                        <UNSIGNED/>
                </field>
                <field name="name" type="C" size="50">
                        <NOTNULL/>
                </field>
                <field name="dbtype" type="C" size="20"/>
                <field name="connection" type="C" size="10"/>
                <field name="host" type="C" size="255"/>
                <field name="port" type="I" size="5">
                        <UNSIGNED/>
                </field>
                <field name="user" type="C" size="32"/>
                <field name="password" type="C" size="255"/>
        </table>
</schema>';

$schema->parseSchemaString($xml);
$res=$schema->executeSchema();

-----<hr>
(postgres9): CREATE TABLE meta_database (
id                       SERIAL,
name                     VARCHAR(50) NOT NULL,
dbtype                   VARCHAR(20),
connection               VARCHAR(10),
host                     VARCHAR(255),
port                     INTEGER,
user                     VARCHAR(32),
password                 VARCHAR(255),
                 PRIMARY KEY (id)
) 
   
-----<hr>
PHP Warning:  pg_query(): Query failed: ERROR:  syntax error at or near "user"
LINE 8: user                     VARCHAR(32),
        ^ in /srv/www/htdocs/ADOdb/drivers/adodb-postgres7.inc.php on line 259
-2: ERROR:  syntax error at or near "user"
LINE 8: user                     VARCHAR(32),
        ^
		ADOConnection._Execute(CREATE TABLE meta_database ( id                       SERIAL, name                     VARCHAR(50) NOT NULL, dbtype             ..., false)% line 1381, file: /srv/www/htdocs/ADOdb/adodb.inc.php
	ADOConnection.Execute(CREATE TABLE meta_database ( id                       SERIAL, name                     VARCHAR(50) NOT NULL, dbtype             ...)% line  393, file: /srv/www/htdocs/ADOdb/adodb-datadict.inc.php
ADODB_DataDict.executeSQLArray(Array[12], false)% line 1725, file: /srv/www/htdocs/ADOdb/adodb-xmlschema03.inc.php
adoSchema.executeSchema()% line   19, file: /srv/www/htdocs/importschema.php

[peterdd]

I think PR #209 handles it, so needs review @mnewnham

Only datadict/datadict-firebird.inc.php overwrites nameQuote() to always always quoting of table names, field names, indexnames. So after adding always quote this overwrite in datadict/datadict-firebird.inc.php can be removed to reduce duplicate code.

[mnewnham]

I'm guessing that when you say everywhere, that is in the context of data dictionary. My opinion on this is:

• We should always call namequote on every field/index/table name, whether reserved or not in the dictionary. There is no reason not to, but
• There should be an option to disregard the quoting request, something like public $alwaysQuoteNmes=false should be in the namequote function itself. For the immediate future, disregarding the quoting should be the default. I am concerned for unexpected reverse compatibility, suddenly introducing quote marks into generated SQL statements might make things break.
• We need to toughen up nameQuote(). Simply repeating the same character isn't going to work with t-sql type statements. In fact, SQL Server lets us use 'table'.'column' or [table].[column]. We already address this in _adodb_getupdateSql(). We might need to be able to distinguish what quoting needs to be used, for example

$this->nameQuoteCharacters = array("`","`")

or

$this->nameQuoteCharcters = array("[","]");

[peterdd]

Yes, and do not mix meaning of ( ) (for partial indexes) and [ ] (mssql/t-sql)

There exist some settings yet:

grep -rn Bracket *
adodb-datadict.inc.php:177:	var $nameRegexBrackets = 'a-zA-Z0-9_\(\)';
adodb-datadict.inc.php:347:	function nameQuote($name = NULL,$allowBrackets=false)
adodb-datadict.inc.php:367:		$regex = ($allowBrackets) ? $this->nameRegexBrackets : $this->nameRegex;
adodb-datadict.inc.php:451:			$flds[$key] = $this->nameQuote($fld,$allowBrackets=true);
adodb.inc.php:428:	var $leftBracket = '[';		/// left square bracked for t-sql styled column names
adodb.inc.php:429:	var $rightBracket = ']';	/// right square bracked for t-sql styled column names
adodb-lib.inc.php:685:							$fnameq = $zthis->leftBracket.$upperfname.$zthis->rightBracket;break;
adodb-lib.inc.php:876:					$fnameq = $zthis->leftBracket.$upperfname.$zthis->rightBracket;break;
datadict/datadict-mssqlnative.inc.php:24:	Bracketed identifiers are delimited by brackets ([ ]):
datadict/datadict-mssql.inc.php:24:	Bracketed identifiers are delimited by brackets ([ ]):
datadict/datadict-firebird.inc.php:61:	function NameQuote($name = NULL,$allowBrackets=false)

[peterdd]

Another thing is that replace() with ADOdb 5.21.0 adds quoting of table names. Our thin db wrapper around ADOdb made this before so without change replace() quotes the tablename a second time.

My workaround is to send unquoted tablename now:
flyspray/flyspray@00c5702

I saw that nameQuote() has some detection if a tablename string parameter has '`' quoting yet.

Maybe replace() should take care of yet quoted tablenames for backward compatibility?

[peterdd]

Any news?

Quick fix for datadicts function nameQuote() commenting out the the if(!preg_match.. ?

// we must always quote as there could be reserved words used too as field name
#if (!preg_match('/^[' . $regex . ']+$/', $name)) {
    return $quote . $name . $quote;
#}

[peterdd]

And please label the issue as bug.