Ticket #1492 (closed defect: fixed)

Opened 9 months ago

Last modified 9 months ago

Non-selected fields appear in query results

Reported by: colnector Owned by: romanb
Priority: major Milestone:
Component: Query/Hydration Version: 1.0.0
Severity: Keywords:
Cc: Has Test: no
Status: Pending Core Response Has Patch: no

Description

Fields that were NOT specifically selected appear in the results set. This is inflating the results set.

For example:

			$q = Doctrine_Query::create() ## New query
				->select('inv.item_id, inv.list_type')
				->from('Inventory inv')
				->addWhere('inv.category_id = ?', array(1))
				
				;

inv.category_id appears in the results set.

Change History

Changed 9 months ago by elbouillon

I have the same problem with my ticket #588

Changed 9 months ago by colnector

Seems that solving this ticket may help your ticket. I really don't understand why redundant fields are added to the result set.

Changed 9 months ago by romanb

If category_id is a primary key then this is normal. Without primary keys array/object graph hydration is not possible. Primary keys are the only fields that are automatically added by doctrine afaik.

Changed 9 months ago by colnector

Yes, it is a part of the primary key. However, this is not the expected behavior and fields should not be added to the results set. In this simple query I have written as an example, there's no excuse (IMHO) for adding category_id to the results set. This wrong behavior incurs an obvious performance penalty (in addition to delivering unexpected results).

Changed 9 months ago by romanb

Again: there is no way to hydrate properly without primary keys (at least we dont know any but if you have a good idea please share it). So the choice is between automatically adding the PKs or throwing an exception every time a PK field is missing. Which one would you prefer?

Changed 9 months ago by elbouillon

I understand that it's not possible to hydrate without primary keys, but if you take a look to my ticket #588, I use a group by and if the primary_key is added my request don't work until I add the primary key field in the group by, which makes my query false.

So, how to do queries w/o hydrating ? I tryied to use Doctrine_Raw_Sql, but the selected fields are added the same way.

Thank you for your answer!

Changed 9 months ago by elbouillon

if we specify Doctrine::HYDRATE_NONE, what does it do ? Because, as I've understood, the problem is the hydration. I tried to make queries with HYDRATE_NONE, but there were no differences.

Changed 9 months ago by colnector

I've now opened a ticket about the lack of documentation of Doctrine::HYDRATE_NONE  #1497

I cannot understand why you would have to have all the PKs for array hydration or no hydration. When the PK is needed (for object hydration) then an exception is the right solution. There already is an exception raised if a PK of a JOINed table isn't selected.

If you wish for me to go over the code, please refer me to relevant sections as I don't know it.

Thanks

Changed 9 months ago by jwage

If we don't have primary keys then the data can't be properly hydrated in to the object graph. It is simple, primary keys allow us to identify one record from another. Without them we aren't able to hydrate the records in to the right place.

I think using HYDRATE_NONE might help you, or maybe the new scalar hydration we're implementing in 1.1 will help solve some problems.

 http://trac.doctrine-project.org/ticket/1141

Changed 9 months ago by romanb

Array graph hydration (thats the current HYDRATE_ARRAY) needs the PKs for the reason object hydration does: it needs to construct a graph, decide which data belongs to what, where a new entity starts and where data belongs.

The only hydration mode currently that does NOT need the PKs is HYDRATE_NONE. If the PKs are added with this hydration mode I consider this to be a bug.

I have 2 new hydration modes in the pipeline that both will not require the PKs (HYDRATE_SCALAR/HYDRATE_SINGLE_SCALAR). I think these will fill the gap for a lot of usecases.

HYDRATE_SCALAR => Returns flat, rectangular result set but with column name <-> fieldname conversions, data type conversions etc. HYDRATE_SINGLE_SCALAR => Returns a single scalar value. I.e. the following DQL: "SELECT u.name FROM User u WHERE u.id = ?" executed with this hydration mode would give the value directly as the result ($name = $q->...->execute($userId)).

These will be available starting with version 1.1 i suppose.

Changed 9 months ago by elbouillon

Thank you for those explanations. I confirm that HYDRATE_NONE add PK, I wait for the 1.1 release so ;-)

Changed 9 months ago by romanb

Then please file a bug for this issue (HYDRATE_NONE adds PKs). I will address this.

Changed 9 months ago by colnector

jwage, I believe what you've written does not apply to array hydration in simple queries in which the result set is simply an array(array()) and not deeper. I think adding PKs should be revised.

I've tested now and HYDRATE_NONE does add the keys. I'm not sure Doctrine_RawSql::getSqlQuery is where this happens but it does happen there as well.

romanb, this is exactly what this ticket is about. Adding the PKs when not needed. On my side, you can mark this one as fixed when PKs will be added only when mandatory (object hydration and complex array hydration). Thanks.

Also, it's very hard to use HYDRATE_NONE since all results are not fetched as associate arrays but numeric arrays such as:

Array
(
    [0] => Array
        (
            [0] => 1075
            [1] => 36949
        )

    [1] => Array
        (
            [0] => 1075
            [1] => 36949
        )

)

I think this can be tweaked in PDO and would be helpful if offered as an option.

Last thing: the HDRATE_SCALAR and HYDRATE_SINGLE_SCALAR seem like very good additions :) They don't, however, address the issues raised in this ticket.

Changed 9 months ago by romanb

I dont see any good reason for making subtle differentiations in array hydration. Not only is this a non-trivial decision to make in many cases (when do we need the PKs, when not) but it would also be very confusing if the same hydration mode behaves differently, sometimes adding PKs, sometimes not. The current Array hydration is a graph hydration and is based on entity identity, thats it.

As for HYDRATE_NONE, yes it returns only numeric keys, this has been asked many times before and the reason here is again quite simple. Just watch the SQL statements that get generated and you will notice that all columns in the select list are aliased in a special way that makes hydration possible/easy. Hence if HYDRATE_NONE would return a normal associative array you would not really have a clue what the resulting column names in the result set are. Not very helpful.

I will mark this ticket closed together with the new ticket on HYDRATE_NONE once i fixed the issue where the PKs are added with HYDRATE_NONE.

Changed 9 months ago by colnector

IMO the decision can be trivial enough: if only field of the class in the from() section are selected, we can skip adding the other PKs. I am, however, not as proficient as you with Doctrine's code so it's your call to make.

Regarding HYDRATE_NONE returning numbers, I have two suggestion: * Since Doctrine probably knows how to map these numbers to column names (which is done in hydration), would it be possible to get a function returning the column name mapping as an array (ex: array('user_id' => 1, 'id' => 2)) so that HYDRATE_NONE would be somewhat more usable? * Adding HYDRATE_ASSOC (which is not that cost effective but probably would still be faster than HYDRATE_ARRAY).

Thanks.

Changed 9 months ago by romanb

I think HYDRATE_SCALAR will be the better option since that basically gives you an associative array right away, without doing graph construction. I think with HYDRATE_SCALAR and HYDRATE_SINGLE_SCALAR HYDRATE_NONE will only be useful for some debugging purposes. It was originally intended to make it easier to fetch single scalar values (like select count(foo.val) from...) but thats what HYDRATE_SINGLE_SCALAR will be there for. This will be as efficient as a single PDOStatement::fetchColumn() (of course with a single column name <-> field name lookup and type conversions if necessary).

Changed 9 months ago by romanb

  • status changed from new to closed
  • resolution set to fixed

Changed 9 months ago by colnector

Associative mapping for HYDRATE_NONE may still be useful in other scenarios when Doctrine's hydration needs to be bypassed for performance (or other) reasons. I can open a new ticket for it if you prefer.

Thanks for the fix

Changed 9 months ago by colnector

  • status changed from closed to reopened
  • resolution fixed deleted

It seems that UNIQUE indexes are also added (not only PKs). This is what's happening with sfGuardUser which adds the username field.

Changed 9 months ago by colnector

  • status changed from reopened to closed
  • resolution set to fixed

Sorry, my mistake.

Note: See TracTickets for help on using tickets.