-
-
Notifications
You must be signed in to change notification settings - Fork 826
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
CRM_Utils_SQL_Select - Allow fluent query execution #10686
Conversation
When I first wrote `CRM_Utils_SQL_Select`, I was a bit dogmatic about loose-coupling and wanted the class to be entirely independent of the SQL runtime. But this is a bit annoying in usage and training. Before ====== To build and execute query, you had to pass the rendered SQL to the execute function, eg ```php $select = CRM_Utils_SQL_Select::from('mytable') ->select('...') $dao = CRM_Core_DAO::executeQuery($select->toSQL()); while ($dao->fetch()) { ... } ``` After ===== You can use more fluent style: ```php $dao = CRM_Utils_SQL_Select::from('mytable') ->select('...') ->execute(); while ($dao->fetch()) { ... } ``` And you can chain with other DAO functions like `fetchAll()` or `fetchValue()`. ```php $records = CRM_Utils_SQL_Select::from('mytable') ->select('...') ->execute() ->fetchAll(); ```
CRM/Utils/SQL/Select.php
Outdated
@@ -594,6 +594,25 @@ public function toSQL() { | |||
} | |||
|
|||
/** | |||
* @return CRM_Core_DAO | |||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to flesh out this docblock by:
- providing a short description
- documenting the parameters with types and descriptions (e.g. what happens when
$i18nRewrite
isTRUE
vsFALSE
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good point. I've updated docblock in c4dcc9c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 😁
I like this improvement. @totten is this PR ready for review? |
@monishdeb Absolutely. If it works for you and seems safe, then lets merge it. |
@totten yup it works fine, merging it now. Also I have raised a doc PR civicrm/civicrm-dev-docs#225. Please have a look!! |
When I first wrote
CRM_Utils_SQL_Select
, I was a bit dogmatic aboutloose-coupling and wanted the class to be entirely independent of the SQL
runtime. But this is a bit annoying in usage and training.
Before
To build and execute query, you had to pass the rendered SQL to the execute
function, eg
After
You can use more fluent style:
And you can chain with other DAO functions like
fetchAll()
,fetchValue()
, orfetchMap()
.