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

PDO compatibility methods fetch() and fetchColumn() for ADORecordSet #205

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ralfbecker
Copy link
Contributor

I was contributing to ADOdb before, mostly in PostgreSQL datadictionary. At some point John Lim stopped responding and our version became a fork. As ADOdb seems to be active again, I would like to contribute my modifications, if they make sense to you, and update our codebase (www.egroupware.org) to current ADOdb version. If everything is acceptable I would use composer to integrate ADOdb in EGroupware. I will publish my modifications as small pull requests. Here's the first one:

These two methods allow to use an ADOdb record-set, as you would a PDO one:

Ralf

svn r25043, r25070, r26667, r53097: without deprecated fetchSingle()
@mnewnham
Copy link
Contributor

Ralf,

You've got so much stuff we need to look through, and I've some more general questions to ask. Do you mind if we move this conversation to Gitter? https://gitter.im/ADOdb/ADOdb

@ralfbecker
Copy link
Contributor Author

Not at all. How do you want to start that?

I don't mind a lot how all the pull requests are implemented, I just need the facilities to be able to use a stock ADOdb in EGroupware. If you which a different interface for some of the pull requests, I'm happy to implement it.

@mnewnham
Copy link
Contributor

I've just posted a couple of questions. By the way, I'm in Mountain Standard Time (GMT +7)

@ralfbecker
Copy link
Contributor Author

I'm in Germany GMT-1, so 8 hours earlier to you.

I answered your questions on gitter. Let's chat when you're up.

@mnewnham
Copy link
Contributor

I'm not a great fan of iterating through columns in a row, reading the 120th column in a 100,000 record set is a lot of loop iterations. can I propose an alternative to the fetchColumn method?

/**
     * PDO compatibility function: fetch the n-th column of the next row
     *
     * @param int       $columnNumber [optional, default 0]
     * @return mixed    false if there's nothing to fetch
     */
    function fetchColumn($columnNumber=0)
    {
        static $keyMap = false;
        if(!($row = $this->FetchRow())) {
            return false;
        }

        /*
        * Note that in this switch statement, there are no breaks
        * this is deliberate
        */
        switch ($this->fetchMode)
        {
        case ADODB_FETCH_BOTH:
        case ADODB_FETCH_DEFAULT:
            /*
            * Extract the associative keys only
            * from a mixed associative/numeric array
            */
            if (!$keyMap)
                $keyMap = array_fill(0,$this->_numOfFields,0);
            $row = array_diff_key($row,$keyMap);

        case ADODB_FETCH_ASSOC:
            /*
            * Change the associative row to a numeric one
            */
            $row = array_values($row);

        case ADODB_FETCH_NUM:
            $ret = $row[$columnNumber];
        }

        return $ret;
    }

@mnewnham
Copy link
Contributor

Regarding the fetch() method, how would you deal with the issue that most drivers do not support changing the fetch mode at a row level? Do you envision it as a core method, but with reduced functionality for most drivers, or as a non-portable method defined in the PDO driver?

@ralfbecker
Copy link
Contributor Author

Your above code to not iterate through the columns is fine with me. Looking at our codebase we only use fetchColumn to get 1 column results like SELECT COUNT(*) FROM table ...

I use fetch as a core method in all our code base for iterating over result sets, eg:

foreach($link->Execute("SELECT * FROM table WHERE type='x'")->fetch() as $row) {
   // do something with each $row
}

EGroupware does not use ADOdb methods directly, but an own database abstraction relying on ADOdb. In that one we choose to use PDO compatible fetch() method to iterate over result sets. Greping over our code I do not find any usage of changing fetch-mode in there. We have an older query method using ADODB_FETCH_BOTH by default and a select method defaulting to ADODB_FETCH_ASSOC.

Which drivers support to change fetch-mode on row level? We currently only support MySQLi and PostgreSQL drivers. We used to also support MsSQL and MaxDB.

For my usage case (big codebase using fetch all over the place, with many third party applications) I don't mind to state that support for changing fetch mode on row-level is driver specific and you better not rely on it, or even not support that parameter at all.

FetchRow does not return a reference, nor does PDOStatement::fetch()
@dregad
Copy link
Member

dregad commented Sep 14, 2016

Is this ready for merging ?

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

Successfully merging this pull request may close these issues.

None yet

3 participants