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

Postgresql indexes #97

Closed
michalbundyra opened this issue Jan 16, 2020 · 7 comments
Closed

Postgresql indexes #97

michalbundyra opened this issue Jan 16, 2020 · 7 comments

Comments

@michalbundyra
Copy link
Member

Add support for Index creation with PostgreSQL platform.
$table = CreateTable();
$table->addConstraint(new Index())
crashed with invalid syntax error. Same for AlterTable.

Can be considered as first step towards cleaning up #67
Continuing to work on finding the rest of the incompatibilities, but not sure if submit one large PR with full PgSQL support or do smaller more review manageable parts at a time. But then cannot make PRs depend on one another so, unless told otherwise will continue adding more commits here.


Originally posted by @alextech at zendframework/zend-db#163

@michalbundyra
Copy link
Member Author

@alextech thanks for this PR but it will require some time to review. In the meantime, because this is a new feature, can you send againt develop branch? I'm putting in zend-db 3.0.0 milestone.


Originally posted by @ezimuel at zendframework/zend-db#163 (comment)

@michalbundyra
Copy link
Member Author

Changed.


Originally posted by @alextech at zendframework/zend-db#163 (comment)

@michalbundyra
Copy link
Member Author

I checked the output of by generating a CreateTable for PostgreSQL.

The general syntax is nice and correct. Where it fails, is the correct escaping, if the escape character is inside column name.

The table name is escaped correctly, but the indices inside and outside of CREATE TABLE are going rogue.

$table = new CreateTable('t\'e"s`t');
$table->addColumn(new BigInteger('t\'e"s`tCol'))
    ->addColumn(new BigInteger('t\'e"s`tCol2'))
    ->addConstraint(new Constraint\PrimaryKey('t\'e"s`tCol'));
    ->addConstraint(new Index('t\'e"s`tCol2', 't\'e"s`tIndex'));

Expected result:

CREATE TABLE "t'e""s`t" ( 
    "t'e""s`tCol" BIGINT NOT NULL,
    "t'e""s`tCol2" BIGINT NOT NULL,
    PRIMARY KEY ("t'e""s`tCol") 
); 
CREATE INDEX "t'e""s`tIndex" ON "t'e""s`tCol2");

Actual result:

CREATE TABLE "t'e""s`t" ( 
    "t""'""e""""""s""`""tCol" BIGINT NOT NULL,
    "t""'""e""""""s""`""tCol2" BIGINT NOT NULL,
    PRIMARY KEY ("t""'""e""""""s""`""tCol") 
); 
CREATE INDEX "t""'""e""""""s""`""tIndex" ON "t""'""e""""""s""`""t"("t""'""e""""""s""`""tCol2");

Instead of just doubling the escape character, there seems to be done some wrong and multiple escaping.

This behaviour is heavily bugged for the code base, too. For this I a separate bug report #267 and perhaps we could work on a fix together for all components.


Originally posted by @rarog at zendframework/zend-db#163 (comment)

@michalbundyra
Copy link
Member Author

Thanks of great magnitude giving this a test after all this time. One thing to note, to make non-MySQL tests more realistic, try to always test with schema as part of the name. There are reports of escaper not working when using semantics outlined in last comment of zendframework/zend-db#206 and fixed at zendframework/zend-db#232

Then, there is also zendframework/zend-db#233 to consider, which fixes inconsistency in applying identifier vs identifier chain. I do not think it relates to this issue, I doubt applying it will magically fix it, but sounds familiar. Will look into it.


Originally posted by @alextech at zendframework/zend-db#163 (comment)

@michalbundyra
Copy link
Member Author

Good point, but so far the Ddl doesn't seem to offer a possibility to use schema as contrary to Select syntax, where TableIdentifier receives schema as 2nd parameter.

The other problem with the broken column and index names doesn't result in any of your parts but from function processExpression in Zend\Db\Sql\AbstractSql

                } elseif ($type == ExpressionInterface::TYPE_IDENTIFIER) {
                    $values[$vIndex] = $platform->quoteIdentifierInFragment($value);

I'm about to comment on this in #267.


Originally posted by @rarog at zendframework/zend-db#163 (comment)

@michalbundyra
Copy link
Member Author

Thanks @rarog I will see if table identifer can be used in DDL too.


Originally posted by @alextech at zendframework/zend-db#163 (comment)

@weierophinney
Copy link
Member

This package is considered feature-complete, and is now in security-only maintenance mode, following a decision by the Technical Steering Committee.
If you have a security issue, please follow our security reporting guidelines.
If you wish to take on the role of maintainer, please nominate yourself

If you are looking for an actively maintained package alternative, we recommend:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants