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

How to support duplicate column names? #151

Open
clue opened this issue Dec 20, 2021 · 2 comments
Open

How to support duplicate column names? #151

clue opened this issue Dec 20, 2021 · 2 comments

Comments

@clue
Copy link
Contributor

clue commented Dec 20, 2021

How should we handle queries that use the same name column name multiple times?

-- Bob wins
SELECT "Alice" AS name, "Bob" AS name

-- Bob wins
SELECT "Alice" AS name, user.* FROM (SELECT "Bob" AS name) AS user

-- Alice wins
SELECT user.*, "Alice" AS name FROM (SELECT "Bob" AS name) AS user

At the moment, the value from the last occurrence wins because we expose each row as an associative array (later values with the same key overwrite previous values).

Technically, the protocol exposes a list of all values, so we do have access to all values inside the protocol implementation, but do not currently expose this to the user. Other projects expose a "fetch mode" to control whether the data should be exposed as a plain list (PDO::FETCH_NUM) or as an associative array (PDO::FETCH_ASSOC) or object (PDO::FETCH_OBJ / PDO::FETCH_CLASS), etc. (https://www.php.net/manual/en/pdostatement.fetch.php)

The current behavior can be seen as inconvenient or inconsistent, given that QueryResult::$resultFields contains a list of all(!) column names, yet QueryResult::$resultRows[0] contains an associative array with unique column names only. If you run any of the above queries with the examples/01-query.php script, you'll see that it reports a table with 2 header columns, but only reports one value per row.

Providing this kind of API isn't too much work, but I wonder how big of a problem this actually is and whether this actually warrants increasing our API surface?

@boenrobot
Copy link

boenrobot commented Sep 16, 2022

IMHO, adding a fetch mode in general is worth it.

Keep the current default of taking the last value in an associate array (both for the sake of BC, but also for the sake of people coming from PDO), but add an indexed array option, which would return all column values mapped to the same indexes (i.e. count($queryResult->resultFields) === count($queryResult->resultRows[0])).

Adding a object mode could be worth it if that object combines the two other fetch modes, i.e. it provides ArrayAccess that would give the column at that index when given an integer, or the last column of that name if given a string. On such an object, count() and foreach behavior could perhaps default to the associative array behavior, but provide a setter to switch both to an indexed mode. This object mode would obviously be more memory consuming, since it would need to contain a reference to some "string to index" map which would add some overhead.

A "compact" object mode (akin to PDO's object mode) is not worth it I think, as in PHP, it has the same ergonomics as an associative array. If you need an stdClass, you can always just cast the associative array to object.

A "class" mode is also not worth it I think, as you still need to define a mapping anyway - might as well define it out of the built in fetch modes. Less magic = easier to reason about.

@SimonFrings
Copy link
Contributor

Hey @boenrobot, sounds like an interesting approach 👍

Providing this kind of API isn't too much work, but I wonder how big of a problem this actually is and whether this actually warrants increasing our API surface?

As @clue wrote above, I am not sure how big of a problem this really is or in which use cases you run into this exact scenario. As I see it there are still ways to get around this without adding new functionality to this project. But if you have a specific use case and an idea to implement this, PR's are always welcome ;)

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

No branches or pull requests

3 participants