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

Fix SQL keywords breaking the queries #38

Merged
merged 1 commit into from Jul 12, 2019

Conversation

@theFeu
Copy link
Contributor

commented Jul 10, 2019

fixes #34

@theFeu theFeu requested a review from lippserd Jul 10, 2019

@@ -130,11 +130,10 @@ public function getDbName()
*
* @return \Zend_Db_Select
*/
public function innerQuery()
public function getInnerQuery()

This comment has been minimized.

Copy link
@Thomas-Gelf

Thomas-Gelf Jul 10, 2019

Collaborator

Please be careful when changing public methods in the Cube. The Cube provides Hooks for custom Actions (see `library/Cube/Hook/ActionsHook.php' ). Those Hooks receive the Cube instance, and public methods on the Cube make part of the API they might be expecting. Blind guess: this change breaks the Cube Action Hook implementation of the Icinga Director

This comment has been minimized.

Copy link
@theFeu

theFeu Jul 10, 2019

Author Contributor

Oh, I wasn't aware of that.
Thanks for pointing it out!

I will revert the name refactor :)

@@ -91,6 +91,7 @@ public function prepareInnerQuery()
$view->applyFilter($this->getMonitoringRestriction());
$select = $view->getQuery()->getSelectQuery();

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 10, 2019

Member

We don't need this twice, do we? 😆

This comment has been minimized.

Copy link
@theFeu

theFeu Jul 10, 2019

Author Contributor

Oof, how did that happen?
Sorry, I'll pay better attention to the diffs next time!

@theFeu theFeu changed the title Fix SQL keywords breaking the queries with tom Fix SQL keywords breaking the queries Jul 11, 2019

$query->columns($columns + $this->factColumns);
$c = [];
foreach ($columns + $this->factColumns as $k => $v) {
$c[$this->connection->getDbAdapter()->quoteIdentifier($k)] = $v;

This comment has been minimized.

Copy link
@lippserd

lippserd Jul 11, 2019

Member

Either use $this->connection->getDbAdapter() or just $this->db() but not both.

@theFeu theFeu added this to the 1.1.0 milestone Jul 11, 2019

@theFeu theFeu self-assigned this Jul 11, 2019

@theFeu theFeu requested a review from lippserd Jul 12, 2019

@lippserd

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

Please squash your commits.

@theFeu theFeu force-pushed the bugfix/escape-sql-properly-34 branch from 6b078c4 to 3a398f4 Jul 12, 2019

@theFeu theFeu force-pushed the bugfix/escape-sql-properly-34 branch from 3a398f4 to 4e4332d Jul 12, 2019

@lippserd lippserd merged commit 96f02ba into master Jul 12, 2019

@lippserd lippserd deleted the bugfix/escape-sql-properly-34 branch Jul 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.