fflib_QueryFactory subqueries produce incorrect soql

[rsoesemann]

I tried to create a query that also fetches related child records but I was unable as the syntax of building such subqueries doesn't produce correct SOQL. Even test methods from the library did work.

Account acct = new Account();
acct.Name = 'testchildqueriesacct';
insert acct;
Contact cont = new Contact();
cont.FirstName = 'test';
cont.LastName = 'test';
cont.AccountId = acct.Id;
insert cont;

String soql = new fflib_QueryFactory(Account.SObjectType)
    .selectField('Name')
    .subselectQuery(Contact.SObjectType)
    .selectField('FirstName')
    .toSoql();

System.debug(soql);
List<Account> accounts = Database.query(soql);

Prints out this incorrect soql

SELECT FirstName FROM Contacts

and throws this exception:

System.QueryException: sObject type 'Contacts' is not supported. If you are attempting to use a
custom object, be sure to append the '__c' after the entity name. Please reference your WSDL or the > describe call for the appropriate names.

[capeterson]

Sorry for taking so long to get back to you on this. I was able to reproduce the issue, I'll see about getting me or the dev who initially wrote in subquery support to get a fix out.

[capeterson]

Gah, looking at the code again I realize what's going on. When you call .subSelectQuery it returns a new fflib_QueryFactory instance for the subquery. This is what you're calling .toSOQL() on, and in the context of a subquery SELECT FirstName FROM Contacts is indeed valid.

If you change your code to this:

fflib_QueryFactory qf = new fflib_QueryFactory(Account.SObjectType)
    .selectField('Name');
qf.subselectQuery(Contact.SObjectType)
    .selectField('FirstName');

String soql = qf.toSOQL();
System.debug(soql);
List<Account> accounts = Database.query(soql);

You'll end up with the query you're expecting: SELECT Name, (SELECT FirstName FROM Contacts) FROM Account.

I don't think there's a way to build that whole query in a single statement unless we added something like a getParentQuery(), which has some internal implications.

Going to close the issue for now, but please re-open it if you disagree with this analysis.