Sqlserver driver: add an flag option to disable cursor client buffer

[celsowm]

Description

In some situations, its desired to avoid the use of the SQL Server Driver Client buffer (examples: performance, large result sets, use of varbinary etc.)
Currently, on CakePHP4.*, the are only "two" ways:

• First: using disableBufferedResults() in all queries that can/could have potential to overflow the client buffer
• Second: creating a CustomDriver extending Cake\Database\Driver\Sqlserver and overwriting the prepare method literally removing this line:
  PDO::SQLSRV_ATTR_CURSOR_SCROLL_TYPE => PDO::SQLSRV_CURSOR_BUFFERED

I would like to propose a simple modification of on Sqlserver driver:

public function prepare($query): StatementInterface
    {
        $this->connect();

        $sql = $query;
        $options = [
            PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL,
            $this->_config['flags'][PDO::SQLSRV_ATTR_CURSOR_SCROLL_TYPE]
        ];
        if ($query instanceof Query) {
            $sql = $query->sql();
            if (count($query->getValueBinder()->bindings()) > 2100) {
                throw new InvalidArgumentException(
                    'Exceeded maximum number of parameters (2100) for prepared statements in Sql Server. ' .
                    'This is probably due to a very large WHERE IN () clause which generates a parameter ' .
                    'for each value in the array. ' .
                    'If using an Association, try changing the `strategy` from select to subquery.'
                );
            }

            if (!$query->isBufferedResultsEnabled()) {
                $options = [];
            }
        }

        /** @psalm-suppress PossiblyInvalidArgument */
        $statement = $this->_connection->prepare($sql, $options);

        return new SqlserverStatement($statement, $this);
    }

and of course a default setting for CURSOR on connect() method, something like that:

if (!$config['flags'][PDO::SQLSRV_ATTR_CURSOR_SCROLL_TYPE]) {
            $config['flags'][PDO::SQLSRV_ATTR_CURSOR_SCROLL_TYPE] = PDO::SQLSRV_CURSOR_BUFFERED
        }

I hope the devs consider this because currently I have had to create a custom driver to run my real application with sql server

CakePHP Version

4.*

[othercorey]

This was added to improve performance on typical result sets. Are you saying you prefer to disable it for all queries?

https://book.cakephp.org/4/en/appendices/4-2-migration-guide.html#id3

[celsowm]

This was added to improve performance on typical result sets. Are you saying you prefer to disable it for all queries?

https://book.cakephp.org/4/en/appendices/4-2-migration-guide.html#id3

Yes !

Even small result sets are faster (at least) in my environment disabling client buffer !

And, in my very humble opinion, would be a nice option to advanced users who prefers to disable this

EDIT:
A good example of better performance without client cursor is pagination of big and complex results

[othercorey]

Well, if this does not have the desired effect, then we should consider reverting it.

Unfortunately, we've had no other feedback on this.

[celsowm]

Please no 😞

I do not think reverting is the best way, just a flag for people like me that uses lots of varbinary and had problems with client cursors.

[othercorey]

Well, we'd like to understand why you have such poor performance with small result sets.

[celsowm]

Well, we'd like to understand why you have such poor performance with small result sets.

I made others tests here and problem is beyond that:

If I try:

function myTest() {

        $table = TableRegistry::getTableLocator()->get('TableWithVarBinary');

        $conteudo = file_get_contents('arquivo.pdf'); //more than 50MB pdf
        $entity = [
            'name' => 'Very big file',
            'binario' => $conteudo, //my varbinary
            'mimetype' => 'application/pdf',
        ];

       $entity =  $table->newEntity($entity);
       $table->save($entity);
    }

I got this error (using the original Driver):

Erro: SQLSTATE[IMSSP]: Memory limit of 50000 KB exceeded for buffered query

Is there a way to disable Buffer on save() ?

[othercorey]

Erro: SQLSTATE[IMSSP]: Memory limit of 50000 KB exceeded for buffered query

Is there a way to disable Buffer on save() ?

I don't think so. I didn't expect that to be affected either.

We should definitely allow this to be disabled. For some reason, I thought it could be disabled with driver config, but we can look into it.

[bor-attila]

Well, if this does not have the desired effect, then we should consider reverting it.

Unfortunately, we've had no other feedback on this.

Azure SQL Server without this option is useless. Very slow. Poor performance.
Please consider to not revert it : )
A flag should do the job : )

[github-actions]

This issue is stale because it has been open for 120 days with no activity. Remove the stale label or comment or this will be closed in 15 days