laminas / laminas-db

Database abstraction layer, SQL abstraction, result set abstraction, and RowDataGateway and TableDataGateway implementations
https://docs.laminas.dev/laminas-db/
BSD 3-Clause "New" or "Revised" License
119 stars 66 forks source link

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

Open ZVanoZ opened 2 years ago

ZVanoZ commented 2 years ago

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.

Ocramius commented 2 years ago

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

ZVanoZ commented 2 years ago

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 https://github.com/laminas/technical-steering-committee/pull/99

Ocramius commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

2. 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'

3. 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'