Provide explain plan feature #315

Merged
merged 18 commits into from Mar 26, 2012

Conversation

Projects
None yet
4 participants
@ddalmais
Contributor

ddalmais commented Mar 16, 2012

Provide explain plan for mysql and oracle : ModelCriteria::explain($con)
Add the capacity to log the connectionName debugpdo.logging.connection which allow to execute a doExplainPlan on the good adapter for symfony debug bar and profiler

@cedriclombardot cedriclombardot referenced this pull request in propelorm/sfPropelORMPlugin Mar 16, 2012

Merged

Activate explain plan on the symfony1 debug bar #126

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 16, 2012

Owner

Nice Fedex work!
You have 30min to write a PR on Propel2 now :p

Le 16 mars 2012 à 15:21, Denis Dalmaisreply@reply.github.com a écrit :

Provide explain plan for mysql and oracle : ModelCriteria::explain($con)
Add the capacity to log the connectionName debugpdo.logging.connection which allow to execute a doExplainPlan on the good adapter for symfony debug bar and profiler

You can merge this Pull Request by running:

git pull https://github.com/ddalmais/Propel feat-explain-plan

Or you can view, comment on it, or merge it online at:

#315

-- Commit Summary --

  • Add doExplainPlan on DBAdapter
  • Add specific Mysql doExplainPlan function
  • Add explain function
  • Add explain plan unit test on mysql bookstore
  • Fix text in ExplainPlanTest
  • Enable doExplainPlan to take a binded query
  • Enable to get the connection name in log
  • Fix phpdoc function title
  • Remove forced database read connection
  • Add DBOracle doExplainPlan function
  • Add unit test on getExplainPlanQuery
  • Make ExplainPlanTest independent from Adapter
  • Format DBMysql code
  • Add unit test on explain plan compute from text query

-- File Changes --

M runtime/lib/Propel.php (1)
M runtime/lib/adapter/DBAdapter.php (12)
M runtime/lib/adapter/DBMySQL.php (28)
M runtime/lib/adapter/DBOracle.php (60)
M runtime/lib/connection/DebugPDO.php (3)
M runtime/lib/connection/PropelPDO.php (22)
M runtime/lib/query/ModelCriteria.php (30)
M test/testsuite/runtime/adapter/DBOracleTest.php (7)
A test/testsuite/runtime/query/ExplainPlanTest.php (87)

-- Patch Links --

https://github.com/propelorm/Propel/pull/315.patch
https://github.com/propelorm/Propel/pull/315.diff


Reply to this email directly or view it on GitHub:
#315

Owner

willdurand commented Mar 16, 2012

Nice Fedex work!
You have 30min to write a PR on Propel2 now :p

Le 16 mars 2012 à 15:21, Denis Dalmaisreply@reply.github.com a écrit :

Provide explain plan for mysql and oracle : ModelCriteria::explain($con)
Add the capacity to log the connectionName debugpdo.logging.connection which allow to execute a doExplainPlan on the good adapter for symfony debug bar and profiler

You can merge this Pull Request by running:

git pull https://github.com/ddalmais/Propel feat-explain-plan

Or you can view, comment on it, or merge it online at:

#315

-- Commit Summary --

  • Add doExplainPlan on DBAdapter
  • Add specific Mysql doExplainPlan function
  • Add explain function
  • Add explain plan unit test on mysql bookstore
  • Fix text in ExplainPlanTest
  • Enable doExplainPlan to take a binded query
  • Enable to get the connection name in log
  • Fix phpdoc function title
  • Remove forced database read connection
  • Add DBOracle doExplainPlan function
  • Add unit test on getExplainPlanQuery
  • Make ExplainPlanTest independent from Adapter
  • Format DBMysql code
  • Add unit test on explain plan compute from text query

-- File Changes --

M runtime/lib/Propel.php (1)
M runtime/lib/adapter/DBAdapter.php (12)
M runtime/lib/adapter/DBMySQL.php (28)
M runtime/lib/adapter/DBOracle.php (60)
M runtime/lib/connection/DebugPDO.php (3)
M runtime/lib/connection/PropelPDO.php (22)
M runtime/lib/query/ModelCriteria.php (30)
M test/testsuite/runtime/adapter/DBOracleTest.php (7)
A test/testsuite/runtime/query/ExplainPlanTest.php (87)

-- Patch Links --

https://github.com/propelorm/Propel/pull/315.patch
https://github.com/propelorm/Propel/pull/315.diff


Reply to this email directly or view it on GitHub:
#315

@cedriclombardot cedriclombardot referenced this pull request in propelorm/PropelBundle Mar 16, 2012

Merged

Allow to show explain plans on each queries #129

@fzaninotto

This comment has been minimized.

Show comment Hide comment
@fzaninotto

fzaninotto Mar 16, 2012

Member

👑

I think we can make an exception to the "no new feature in the 1.x branch" for this amazing work.

Member

fzaninotto commented Mar 16, 2012

👑

I think we can make an exception to the "no new feature in the 1.x branch" for this amazing work.

runtime/lib/adapter/DBMySQL.php
+ * @throws PropelException
+ * @return PDOStatement A PDO statement executed using the connection, ready to be fetched
+ */
+ public function doExplainPlan(PropelPDO $con, $query) {

This comment has been minimized.

Show comment Hide comment
@fzaninotto

fzaninotto Mar 16, 2012

Member

please place opening bracket on a newline

@fzaninotto

fzaninotto Mar 16, 2012

Member

please place opening bracket on a newline

runtime/lib/adapter/DBAdapter.php
+ * @throws PropelException if explain plan is not implemented for adapter
+ * @return PDOStatement A PDO statement executed using the connection, ready to be fetched
+ */
+ public function doExplainPlan(PropelPDO $con, $query) {

This comment has been minimized.

Show comment Hide comment
@fzaninotto

fzaninotto Mar 16, 2012

Member

please place opening bracket on a newline

@fzaninotto

fzaninotto Mar 16, 2012

Member

please place opening bracket on a newline

runtime/lib/connection/PropelPDO.php
@@ -326,6 +338,9 @@ public function setAttribute($attribute, $value)
case self::PROPEL_ATTR_CACHE_PREPARES:
$this->cachePreparedStatements = $value;
break;
+ case self::PROPEL_ATTR_CONNECTION_NAME:
+ $this->connectionName = $value;

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 16, 2012

Owner

bad indentation

@willdurand

willdurand Mar 16, 2012

Owner

bad indentation

runtime/lib/connection/PropelPDO.php
@@ -345,6 +360,9 @@ public function getAttribute($attribute)
case self::PROPEL_ATTR_CACHE_PREPARES:
return $this->cachePreparedStatements;
break;
+ case self::PROPEL_ATTR_CONNECTION_NAME:

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 16, 2012

Owner

bad indentation

@willdurand

willdurand Mar 16, 2012

Owner

bad indentation

runtime/lib/connection/PropelPDO.php
@@ -724,6 +742,10 @@ protected function getLogPrefix($methodName, $debugSnapshot)
$value = str_pad($methodName, $this->getLoggingConfig('details.method.pad', 28), ' ', STR_PAD_RIGHT);
break;
+ case 'connection':
+ $value = $this->connectionName;

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 16, 2012

Owner

bad indentation

@willdurand

willdurand Mar 16, 2012

Owner

bad indentation

+ $db = new DBOracle();
+ $explainQuery = $db->getExplainPlanQuery('SELECT B.* FROM (SELECT A.*, rownum AS PROPEL_ROWNUM FROM (SELECT book.ID AS ORA_COL_ALIAS_0, book.TITLE AS ORA_COL_ALIAS_1, book.ISBN AS ORA_COL_ALIAS_2, book.PRICE AS ORA_COL_ALIAS_3, book.PUBLISHER_ID AS ORA_COL_ALIAS_4, book.AUTHOR_ID AS ORA_COL_ALIAS_5, author.ID AS ORA_COL_ALIAS_6, author.FIRST_NAME AS ORA_COL_ALIAS_7, author.LAST_NAME AS ORA_COL_ALIAS_8, author.EMAIL AS ORA_COL_ALIAS_9, author.AGE AS ORA_COL_ALIAS_10, book.PRICE AS BOOK_PRICE FROM book, author) A ) B WHERE B.PROPEL_ROWNUM <= 1', 'iuyiuyiu');
+ $this->assertEquals('EXPLAIN PLAN SET STATEMENT_ID = \'iuyiuyiu\' FOR SELECT B.* FROM (SELECT A.*, rownum AS PROPEL_ROWNUM FROM (SELECT book.ID AS ORA_COL_ALIAS_0, book.TITLE AS ORA_COL_ALIAS_1, book.ISBN AS ORA_COL_ALIAS_2, book.PRICE AS ORA_COL_ALIAS_3, book.PUBLISHER_ID AS ORA_COL_ALIAS_4, book.AUTHOR_ID AS ORA_COL_ALIAS_5, author.ID AS ORA_COL_ALIAS_6, author.FIRST_NAME AS ORA_COL_ALIAS_7, author.LAST_NAME AS ORA_COL_ALIAS_8, author.EMAIL AS ORA_COL_ALIAS_9, author.AGE AS ORA_COL_ALIAS_10, book.PRICE AS BOOK_PRICE FROM book, author) A ) B WHERE B.PROPEL_ROWNUM <= 1', $explainQuery, 'getExplainPlanQuery() returns a SQL Explain query');
+

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 16, 2012

Owner

useless blank line

@willdurand

willdurand Mar 16, 2012

Owner

useless blank line

+ $this->assertTrue(!empty($explain[1]['table']), 'Line 2, table is equal to "author"');
+ $this->assertTrue(!empty($explain[1]['type']), 'Line 2, type is equal to "eq_ref"');
+ $this->assertTrue(!empty($explain[1]['possible_keys']), 'Line 2, possible_keys is equal to "PRIMARY"');
+ } elseif($db instanceof DBOracle) {

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 16, 2012

Owner

I guess it should be else if

@willdurand

willdurand Mar 16, 2012

Owner

I guess it should be else if

+ $this->assertEquals(sizeof($explain), 2, 'Explain plan return two lines');
+
+ // explain can change sometime, test can't be strict
+ $this->assertTrue(!empty($explain[0]['select_type']), 'Line 1, select_type is equal to "SIMPLE"');

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 16, 2012

Owner

You should use assertArrayHasKey and/or assertFalse instead

@willdurand

willdurand Mar 16, 2012

Owner

You should use assertArrayHasKey and/or assertFalse instead

+ $this->assertEquals(sizeof($explain), 2, 'Explain plan return two lines');
+
+ // explain can change sometime, test can't be strict
+ $this->assertTrue(!empty($explain[0]['select_type']), 'Line 1, select_type is equal to "SIMPLE"');

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 16, 2012

Owner

same here

@willdurand

willdurand Mar 16, 2012

Owner

same here

+ $this->markTestSkipped('Cannot test explain plan on adapter ' . get_class($db));
+ }
+ }
+

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 16, 2012

Owner

useless blank line

@willdurand

willdurand Mar 16, 2012

Owner

useless blank line

runtime/lib/adapter/DBOracle.php
@@ -236,4 +236,64 @@ public function bindValue(PDOStatement $stmt, $parameter, $value, ColumnMap $cMa
return $stmt->bindValue($parameter, $value, $cMap->getPdoType());
}
+
+ /**
+ * Do Explain Plan for query object or query string

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 16, 2012

Owner

bad indentation for this comment block

@willdurand

willdurand Mar 16, 2012

Owner

bad indentation for this comment block

runtime/lib/adapter/DBMySQL.php
+ }
+
+ $stmt->execute();
+ return $stmt;

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 16, 2012

Owner

you should add a blank line before the return statement

@willdurand

willdurand Mar 16, 2012

Owner

you should add a blank line before the return statement

runtime/lib/adapter/DBMySQL.php
@@ -250,4 +250,33 @@ public function prepareParams($params)
return $params;
}
+
+ /**
+ * Do Explain Plan for query object or query string

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 16, 2012

Owner

bad indentation for this comment block

@willdurand

willdurand Mar 16, 2012

Owner

bad indentation for this comment block

runtime/lib/adapter/DBAdapter.php
@@ -580,4 +580,17 @@ public function bindValue(PDOStatement $stmt, $parameter, $value, ColumnMap $cMa
return $stmt->bindValue($parameter, $value, $cMap->getPdoType());
}
+
+ /**
+ * Do Explain Plan for query object or query string

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 16, 2012

Owner

bad indentation for this comment block

@willdurand

willdurand Mar 16, 2012

Owner

bad indentation for this comment block

@ddalmais

This comment has been minimized.

Show comment Hide comment
@ddalmais

ddalmais Mar 19, 2012

Contributor

CS fixed

Contributor

ddalmais commented Mar 19, 2012

CS fixed

willdurand added a commit that referenced this pull request Mar 26, 2012

@willdurand willdurand merged commit 45e5c3e into propelorm:master Mar 26, 2012

fabpot added a commit to symfony/symfony that referenced this pull request Mar 26, 2012

merged branch cedriclombardot/feat-propel-explain (PR #3616)
Commits
-------

9ef5e95 Add connection name in the propel data collector

Discussion
----------

Add connection name in the propel data collector

Bug fix: no
Feature addition: yes, This will allow to explain a propel query on a specific connection
Backwards compatibility break: no
Symfony2 tests pass: yes

- Require PR propelorm/Propel#315
- Related to PR propelorm/PropelBundle#129

cc @willdurand

---------------------------------------------------------------------------

by willdurand at 2012-03-16T18:17:26Z

@fabpot please, let me merge Propel related PRs before that one, thanks!

---------------------------------------------------------------------------

by willdurand at 2012-03-26T08:38:36Z

@fabpot good to go from my point of view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment