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

Symbol "#" is valid for identifiers for Oracle platform #237

Open
ZVanoZ opened this issue Feb 9, 2022 · 5 comments
Open

Symbol "#" is valid for identifiers for Oracle platform #237

ZVanoZ opened this issue Feb 9, 2022 · 5 comments
Labels
Bug Something isn't working

Comments

@ZVanoZ
Copy link
Contributor

ZVanoZ commented Feb 9, 2022

Bug Report

Q A
Version(s) 2.13.4

Summary

-- For example, this SQL is valid
SELECT t.ROLE# AS ROLE_ID, t.NOTE AS NOTE, t.IS_USED AS IS_USED FROM BILL.V$LAW_ROLES t

-- laminas-db do invalid SQL
 SELECT "t"."ROLE""#" AS "ROLE_ID", "t"."NOTE" AS "NOTE", "t"."IS_USED" AS "IS_USED" FROM "BILL.V$LAW_ROLES" "t"
/*
Laminas\Db\Adapter\Exception\RuntimeException

File:    /app/vendor/laminas/laminas-db/src/Adapter/Driver/Oci8/Statement.php:243
Message:    ORA-03001: unimplemented feature
*/

For fix the error, just add symbol "#" in platform pattern

// src/Adapter/Platform/Oracle.php
namespace Laminas\Db\Adapter\Platform;
class Oracle extends AbstractPlatform
{
   /**
   * Override pattern '/([^0-9,a-z,A-Z$_:])/i' 
   * This pattern in "src/Adapter/Platform/AbstractPlatform.php "
   */
    protected $quoteIdentifierFragmentPattern = '/([^0-9,a-z,A-Z$_:#])/i';
//...

Current behavior

Generated SQL is invalid.

How to reproduce

Re-generate the request above

Expected behavior

Symbol "#" does not quote.

@ZVanoZ ZVanoZ added the Bug Something isn't working label Feb 9, 2022
@Ocramius
Copy link
Member

Ocramius commented Feb 9, 2022

Can this somehow be unit-tested in the Oracle platform tests?

@ZVanoZ
Copy link
Contributor Author

ZVanoZ commented Feb 9, 2022

No, in current test system.
Pull requst #227 can help do the test.
But this request is raw and fix some other bugs.

I asked to give correct test system for laminas-db in laminas/technical-steering-committee#99

@Ocramius
Copy link
Member

Ocramius commented Feb 9, 2022

Yeah, setting up OracleDB testing on GHA is potentially gonna be painful, but we can add it under services:

See for example: https://github.com/doctrine/dbal/blob/005871fd4f5f938981360b91be6fa11eee64dbb2/.github/workflows/continuous-integration.yml#L131-L158

@ZVanoZ
Copy link
Contributor Author

ZVanoZ commented Feb 11, 2022

I am still doing tests and fix.
For example, this test give an error.

trait TraitSetup
{
    protected function getDriverConfig()
    {
        $result = [
            'driver' => 'Oci8',
            'hostname' => $this->variables['hostname'],
            'username' => $this->variables['username'],
            'password' => $this->variables['password'],
            'charset' => 'AL32UTF8',
//            'platform_options' => [
//                'quote_identifiers' => true
//            ],
        ];
        return $result;
    }

class CharsetTest extends TestCase
{
    public function testSelectAdapterAndDbSelect()
    {
        $adapter = $this->createAdapterWithQuoteIdentifiers();
        $select = new \Laminas\Db\Sql\Select();
        $select->from([
           't' => 'test_charset'
        ]);
        $select->columns([
            'field$',
            'field_',
            'field#',
        ]);
        $plarform = $adapter->getPlatform();
        $sqlString = $select->getSqlString($plarform);// Invalid SQL here
        $statement = $adapter->createStatement($sqlString);
        $result = $statement->execute(); // Exception here
        $this->assertInstanceOf(ResultInterface::class, $result);
    }

@ZVanoZ
Copy link
Contributor Author

ZVanoZ commented Feb 15, 2022

I did test and fix in #238
But github test system is broken!

  1. I dont touch mysql tests, but have an error.
https://github.com/laminas/laminas-db/runs/5199365331?check_suite_focus=true

1) LaminasIntegrationTest\Db\Adapter\Driver\Pdo\Mysql\AdapterTest::testDriverDisconnectAfterQuoteWithPlatform
Failed asserting that true is false.

Error: /github/workspace/test/integration/Adapter/Driver/Pdo/AbstractAdapterTest.php:41
phpvfscomposer:///github/workspace/vendor/phpunit/phpunit/phpunit:97
  1. Environment for PHP 8.1 is broken
https://github.com/laminas/laminas-db/runs/5199365672?check_suite_focus=true

E: Unable to locate package php8.1-sqlsrv
E: Couldn't find any package by glob 'php8.1-sqlsrv'
  1. Environment for PHP 8.1 with latest dependencies is broken
https://github.com/laminas/laminas-db/runs/5199365731?check_suite_focus=true

E: Unable to locate package php8.1-sqlsrv
E: Couldn't find any package by glob 'php8.1-sqlsrv'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants