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

[Db] Generated DialectClass referring to a raw PHP class won't be found #13867

Merged
merged 1 commit into from Mar 10, 2019

Conversation

Projects
None yet
6 participants
@lubber-de
Copy link

commented Feb 27, 2019

Hello!

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the Contributing Guidelines?
  • I have checked that another pull request for this purpose does not exist.
    #12686 exists, but closed and messed up (indeed it was myself on another github account years ago)
  • I wrote some tests for this PR.

Description

On Instantiating a Db-Instance, the DialectClass is generated by Zephir if not given as a property. The generated Class is always lowercase which does only work for internal zephir classes, but not for PHP-Classes for example the Oracle adapter given by the incubator.

This won't work...

... because the dialectClass Property is not given and phalcon tries to guess it on its own.
As zephir works case insensitive the generated class phalcon\db\dialect\oracle won't be found, because it's not inside Phalcon since 3.0, but available via incubator only

$test = new \Phalcon\Db\Adapter\Pdo\Oracle([
            'username'    => '',
            'password'    => '',
            'dbname'      => ''
        ]);

Result

Fatal error: Class 'phalcon\db\dialect\oracle' not found in ....

Workaround

If the dialectClass is given as property, then it works

$test = new \Phalcon\Db\Adapter\Pdo\Oracle([
            'username'    => '',
            'password'    => '',
            'dbname'      => '',
            'dialectClass' => \Phalcon\Db\Dialect\Oracle::class
        ]);

Closes

phalcon/incubator#663
More Info: phalcon/incubator#663 (comment)

fix(db): Generated DialectClass referring to a raw pgp class (like in…
…cubator files) won't load because of missing CamelCase
@niden

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

@sergeyklay

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

@niden It just looks good to me :) I can even merge it to 3.4.x branch, but it will be there for a long time.

@niden

This comment has been minimized.

Copy link
Member

commented Mar 1, 2019

@sergeyklay the issue is not that it will go to 3.4 and stay there a long time, the issue is that we will need to release yet again one more v3 release....

I am fine if you want to merge this.

@lubber-de lubber-de closed this Mar 1, 2019

@lubber-de

This comment has been minimized.

Copy link
Author

commented Mar 1, 2019

@niden FYI you probably have to release another 3.4 anyway, because I recently got lots of segfault crashes when using the incubator and instantiating oracle with php 7.3 and the latest 3.4.3 on windows .. But I am still testing what might triggering it or if it also affects other incubator components. I'll open another issue or even PR then.

@lubber-de lubber-de reopened this Mar 1, 2019

@dschissler

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2019

Since my PDO "radical options" PR was merged then I'm going to look into this for 4.x. I won't be able to test for it though.

@niden niden merged commit 7416dd4 into phalcon:3.4.x Mar 10, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@niden

This comment has been minimized.

Copy link
Member

commented Mar 10, 2019

Thank you @lubber-de

@niden niden added the Bug - Low label Mar 10, 2019

@sergeyklay sergeyklay added this to the 3.4.4 milestone Mar 10, 2019

@lubber-de lubber-de deleted the lubber-de:fix/dialectclass_camelcase branch Mar 11, 2019

@niden niden added the Docs needed label Apr 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.