Skip to content

Commit

Permalink
Merge pull request #8748 from kenjis/feat-db-tablename-object-4.6
Browse files Browse the repository at this point in the history
feat: fix spark db:table causes errors with table name including special chars
  • Loading branch information
kenjis authored Aug 1, 2024
2 parents b3d957d + 58197a4 commit 6a9aedf
Show file tree
Hide file tree
Showing 14 changed files with 399 additions and 45 deletions.
6 changes: 0 additions & 6 deletions phpstan-baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -2137,12 +2137,6 @@
'count' => 1,
'path' => __DIR__ . '/system/Database/BaseConnection.php',
];
$ignoreErrors[] = [
// identifier: missingType.iterableValue
'message' => '#^Property CodeIgniter\\\\Database\\\\BaseConnection\\:\\:\\$aliasedTables type has no value type specified in iterable type array\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Database/BaseConnection.php',
];
$ignoreErrors[] = [
// identifier: missingType.iterableValue
'message' => '#^Property CodeIgniter\\\\Database\\\\BaseConnection\\:\\:\\$dataCache type has no value type specified in iterable type array\\.$#',
Expand Down
5 changes: 3 additions & 2 deletions system/Commands/Database/ShowTableInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use CodeIgniter\CLI\BaseCommand;
use CodeIgniter\CLI\CLI;
use CodeIgniter\Database\BaseConnection;
use CodeIgniter\Database\TableName;
use CodeIgniter\Exceptions\InvalidArgumentException;
use Config\Database;

Expand Down Expand Up @@ -199,7 +200,7 @@ private function showDataOfTable(string $tableName, int $limitRows, int $limitFi
CLI::newLine();

$this->removeDBPrefix();
$thead = $this->db->getFieldNames($tableName);
$thead = $this->db->getFieldNames(TableName::fromActualName($this->db, $tableName));
$this->restoreDBPrefix();

// If there is a field named `id`, sort by it.
Expand Down Expand Up @@ -277,7 +278,7 @@ private function makeTableRows(
$this->tbody = [];

$this->removeDBPrefix();
$builder = $this->db->table($tableName);
$builder = $this->db->table(TableName::fromActualName($this->db, $tableName));
$builder->limit($limitRows);
if ($sortField !== null) {
$builder->orderBy($sortField, $this->sortDesc ? 'DESC' : 'ASC');
Expand Down
17 changes: 11 additions & 6 deletions system/Database/BaseBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ class BaseBuilder
/**
* Constructor
*
* @param array|string $tableName tablename or tablenames with or without aliases
* @param array|string|TableName $tableName tablename or tablenames with or without aliases
*
* Examples of $tableName: `mytable`, `jobs j`, `jobs j, users u`, `['jobs j','users u']`
*
Expand All @@ -315,15 +315,20 @@ public function __construct($tableName, ConnectionInterface $db, ?array $options
*/
$this->db = $db;

if ($tableName instanceof TableName) {
$this->tableName = $tableName->getTableName();
$this->QBFrom[] = $this->db->escapeIdentifier($tableName);
$this->db->addTableAlias($tableName->getAlias());
}
// If it contains `,`, it has multiple tables
if (is_string($tableName) && ! str_contains($tableName, ',')) {
elseif (is_string($tableName) && ! str_contains($tableName, ',')) {
$this->tableName = $tableName; // @TODO remove alias if exists
$this->from($tableName);
} else {
$this->tableName = '';
$this->from($tableName);
}

$this->from($tableName);

if ($options !== null && $options !== []) {
foreach ($options as $key => $value) {
if (property_exists($this, $key)) {
Expand Down Expand Up @@ -3038,10 +3043,10 @@ protected function trackAliases($table)
$table = preg_replace('/\s+AS\s+/i', ' ', $table);

// Grab the alias
$table = trim(strrchr($table, ' '));
$alias = trim(strrchr($table, ' '));

// Store the alias, if it doesn't already exist
$this->db->addTableAlias($table);
$this->db->addTableAlias($alias);
}
}

Expand Down
62 changes: 48 additions & 14 deletions system/Database/BaseConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ abstract class BaseConnection implements ConnectionInterface
/**
* Array of table aliases.
*
* @var array
* @var list<string>
*/
protected $aliasedTables = [];

Expand Down Expand Up @@ -576,10 +576,14 @@ public function setAliasedTables(array $aliases)
*
* @return $this
*/
public function addTableAlias(string $table)
public function addTableAlias(string $alias)
{
if (! in_array($table, $this->aliasedTables, true)) {
$this->aliasedTables[] = $table;
if ($alias === '') {
return $this;
}

if (! in_array($alias, $this->aliasedTables, true)) {
$this->aliasedTables[] = $alias;
}

return $this;
Expand Down Expand Up @@ -925,7 +929,7 @@ abstract protected function _transRollback(): bool;
/**
* Returns a non-shared new instance of the query builder for this connection.
*
* @param array|string $tableName
* @param array|string|TableName $tableName
*
* @return BaseBuilder
*
Expand Down Expand Up @@ -1054,10 +1058,10 @@ public function getConnectDuration(int $decimals = 6): string
* insert the table prefix (if it exists) in the proper position, and escape only
* the correct identifiers.
*
* @param array|int|string $item
* @param bool $prefixSingle Prefix a table name with no segments?
* @param bool $protectIdentifiers Protect table or column names?
* @param bool $fieldExists Supplied $item contains a column name?
* @param array|int|string|TableName $item
* @param bool $prefixSingle Prefix a table name with no segments?
* @param bool $protectIdentifiers Protect table or column names?
* @param bool $fieldExists Supplied $item contains a column name?
*
* @return array|string
* @phpstan-return ($item is array ? array : string)
Expand All @@ -1078,6 +1082,11 @@ public function protectIdentifiers($item, bool $prefixSingle = false, ?bool $pro
return $escapedArray;
}

if ($item instanceof TableName) {
/** @psalm-suppress NoValue I don't know why ERROR. */
return $this->escapeTableName($item);
}

// If you pass `['column1', 'column2']`, `$item` will be int because the array keys are int.
$item = (string) $item;

Expand Down Expand Up @@ -1220,10 +1229,18 @@ private function protectDotItem(string $item, string $alias, bool $protectIdenti
*
* This function escapes single identifier.
*
* @param non-empty-string $item
* @param non-empty-string|TableName $item
*/
public function escapeIdentifier(string $item): string
public function escapeIdentifier($item): string
{
if ($item === '') {
return '';
}

if ($item instanceof TableName) {
return $this->escapeTableName($item);
}

return $this->escapeChar
. str_replace(
$this->escapeChar,
Expand All @@ -1233,6 +1250,17 @@ public function escapeIdentifier(string $item): string
. $this->escapeChar;
}

/**
* Returns escaped table name with alias.
*/
private function escapeTableName(TableName $tableName): string
{
$alias = $tableName->getAlias();

return $this->escapeIdentifier($tableName->getActualTableName())
. (($alias !== '') ? ' ' . $this->escapeIdentifier($alias) : '');
}

/**
* Escape the SQL Identifiers
*
Expand Down Expand Up @@ -1546,12 +1574,16 @@ public function tableExists(string $tableName, bool $cached = true): bool
/**
* Fetch Field Names
*
* @param string|TableName $tableName
*
* @return false|list<string>
*
* @throws DatabaseException
*/
public function getFieldNames(string $table)
public function getFieldNames($tableName)
{
$table = ($tableName instanceof TableName) ? $tableName->getTableName() : $tableName;

// Is there a cached result?
if (isset($this->dataCache['field_names'][$table])) {
return $this->dataCache['field_names'][$table];
Expand All @@ -1561,7 +1593,7 @@ public function getFieldNames(string $table)
$this->initialize();
}

if (false === ($sql = $this->_listColumns($table))) {
if (false === ($sql = $this->_listColumns($tableName))) {
if ($this->DBDebug) {
throw new DatabaseException('This feature is not available for the database you are using.');
}
Expand Down Expand Up @@ -1777,9 +1809,11 @@ abstract protected function _listTables(bool $constrainByPrefix = false, ?string
/**
* Generates a platform-specific query string so that the column names can be fetched.
*
* @param string|TableName $table
*
* @return false|string
*/
abstract protected function _listColumns(string $table = '');
abstract protected function _listColumns($table = '');

/**
* Platform-specific field data information.
Expand Down
14 changes: 12 additions & 2 deletions system/Database/MySQLi/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

use CodeIgniter\Database\BaseConnection;
use CodeIgniter\Database\Exceptions\DatabaseException;
use CodeIgniter\Database\TableName;
use CodeIgniter\Exceptions\LogicException;
use mysqli;
use mysqli_result;
Expand Down Expand Up @@ -422,10 +423,19 @@ protected function _listTables(bool $prefixLimit = false, ?string $tableName = n

/**
* Generates a platform-specific query string so that the column names can be fetched.
*
* @param string|TableName $table
*/
protected function _listColumns(string $table = ''): string
protected function _listColumns($table = ''): string
{
return 'SHOW COLUMNS FROM ' . $this->protectIdentifiers($table, true, null, false);
$tableName = $this->protectIdentifiers(
$table,
true,
null,
false
);

return 'SHOW COLUMNS FROM ' . $tableName;
}

/**
Expand Down
18 changes: 13 additions & 5 deletions system/Database/OCI8/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use CodeIgniter\Database\BaseConnection;
use CodeIgniter\Database\Exceptions\DatabaseException;
use CodeIgniter\Database\Query;
use CodeIgniter\Database\TableName;
use ErrorException;
use stdClass;

Expand Down Expand Up @@ -284,18 +285,25 @@ protected function _listTables(bool $prefixLimit = false, ?string $tableName = n

/**
* Generates a platform-specific query string so that the column names can be fetched.
*
* @param string|TableName $table
*/
protected function _listColumns(string $table = ''): string
protected function _listColumns($table = ''): string
{
if (str_contains($table, '.')) {
sscanf($table, '%[^.].%s', $owner, $table);
if ($table instanceof TableName) {
$tableName = $this->escape(strtoupper($table->getActualTableName()));
$owner = $this->username;
} elseif (str_contains($table, '.')) {
sscanf($table, '%[^.].%s', $owner, $tableName);
$tableName = $this->escape(strtoupper($this->DBPrefix . $tableName));
} else {
$owner = $this->username;
$owner = $this->username;
$tableName = $this->escape(strtoupper($this->DBPrefix . $table));
}

return 'SELECT COLUMN_NAME FROM ALL_TAB_COLUMNS
WHERE UPPER(OWNER) = ' . $this->escape(strtoupper($owner)) . '
AND UPPER(TABLE_NAME) = ' . $this->escape(strtoupper($this->DBPrefix . $table));
AND UPPER(TABLE_NAME) = ' . $tableName;
}

/**
Expand Down
14 changes: 11 additions & 3 deletions system/Database/Postgre/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use CodeIgniter\Database\BaseConnection;
use CodeIgniter\Database\Exceptions\DatabaseException;
use CodeIgniter\Database\RawSql;
use CodeIgniter\Database\TableName;
use ErrorException;
use PgSql\Connection as PgSqlConnection;
use PgSql\Result as PgSqlResult;
Expand Down Expand Up @@ -300,13 +301,20 @@ protected function _listTables(bool $prefixLimit = false, ?string $tableName = n

/**
* Generates a platform-specific query string so that the column names can be fetched.
*
* @param string|TableName $table
*/
protected function _listColumns(string $table = ''): string
protected function _listColumns($table = ''): string
{
if ($table instanceof TableName) {
$tableName = $this->escape($table->getActualTableName());
} else {
$tableName = $this->escape($this->DBPrefix . strtolower($table));
}

return 'SELECT "column_name"
FROM "information_schema"."columns"
WHERE LOWER("table_name") = '
. $this->escape($this->DBPrefix . strtolower($table))
WHERE LOWER("table_name") = ' . $tableName
. ' ORDER BY "ordinal_position"';
}

Expand Down
13 changes: 11 additions & 2 deletions system/Database/SQLSRV/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

use CodeIgniter\Database\BaseConnection;
use CodeIgniter\Database\Exceptions\DatabaseException;
use CodeIgniter\Database\TableName;
use stdClass;

/**
Expand Down Expand Up @@ -225,12 +226,20 @@ protected function _listTables(bool $prefixLimit = false, ?string $tableName = n

/**
* Generates a platform-specific query string so that the column names can be fetched.
*
* @param string|TableName $table
*/
protected function _listColumns(string $table = ''): string
protected function _listColumns($table = ''): string
{
if ($table instanceof TableName) {
$tableName = $this->escape(strtolower($table->getActualTableName()));
} else {
$tableName = $this->escape($this->DBPrefix . strtolower($table));
}

return 'SELECT [COLUMN_NAME] '
. ' FROM [INFORMATION_SCHEMA].[COLUMNS]'
. ' WHERE [TABLE_NAME] = ' . $this->escape($this->DBPrefix . $table)
. ' WHERE [TABLE_NAME] = ' . $tableName
. ' AND [TABLE_SCHEMA] = ' . $this->escape($this->schema);
}

Expand Down
Loading

0 comments on commit 6a9aedf

Please sign in to comment.