Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pg schema #24999

Merged
merged 15 commits into from
Jun 5, 2019
9 changes: 7 additions & 2 deletions libraries/joomla/database/driver/postgresql.php
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ public function getTableCreate($tables)
/**
* Retrieves field information about a given table.
*
* @param string $table The name of the database table.
* @param string $table The name of the database table. For PostgreSQL may starting with a schema
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param string $table The name of the database table. For PostgreSQL may starting with a schema
* @param string $table The name of the database table. For PostgreSQL may start with a schema.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

* @param boolean $typeOnly True to only return field types.
*
* @return array An array of fields for the database table.
Expand All @@ -406,6 +406,11 @@ public function getTableColumns($table, $typeOnly = true)
$result = array();

$tableSub = $this->replacePrefix($table);
$fn = explode('.', $tableSub);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style error reported by Drone for lines 409 to 413: Tabs must be used to indent lines; spaces are not allowed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

if (count($fn) == 2) {
Copy link
Member

@richard67 richard67 May 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind the Joomla CMS code style rules or check the Drone report.
An "if" with "else" should look as follows, where "..." means some code:

if (...)
{
...
}
else
{
...
}

Please correct that to make Drone happy.

Copy link
Member

@richard67 richard67 May 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P.S. And indent with tabs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

$schema = $fn[0];
$tableSub = $fn[1];
} else {$schema = 'public';}

$this->setQuery('
SELECT a.attname AS "column_name",
Expand All @@ -427,7 +432,7 @@ public function getTableColumns($table, $typeOnly = true)
WHERE a.attrelid =
(SELECT oid FROM pg_catalog.pg_class WHERE relname=' . $this->quote($tableSub) . '
AND relnamespace = (SELECT oid FROM pg_catalog.pg_namespace WHERE
nspname = \'public\')
nspname = \'' . $schema . '\')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use $this->quote() for quoting please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

)
AND a.attnum > 0 AND NOT a.attisdropped
ORDER BY a.attnum'
Expand Down
23 changes: 23 additions & 0 deletions schema.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this file from your PR since it is only for testing this PR, so it should not be checked into the Joomla CMS source code. You have that code already in the testing instructions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to convert it into a real test and integrated it into the test suite.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that would be best. If the author of this PR feels not being able to do that, we can do that later with a new PR or help him here. But anyway this files as it is should be removed from this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now file 'schema.php' simply deleted

define( '_JEXEC', 1);
define( 'JPATH_BASE', realpath(dirname(__FILE__) .'/' ) );
require_once ( JPATH_BASE .'/includes/defines.php' );
require_once ( JPATH_BASE .'/includes/framework.php' );
$db = JFactory::getDBO();
$cf=$db->getTableColumns('#__content'); // It returns array with fields


/** In database
create schema test;
CREATE TABLE test.customer (
id SERIAL,
name VARCHAR(100),
PRIMARY KEY(id)
)
WITH (oids = false);
*/

$custfields=$db->getTableColumns('test.customer'); // It returns empty array
var_dump($custfields);

?>