public inbox for [email protected]
help / color / mirror / Atom feedFrom: Akshay Joshi <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch]: RM #3397 Add support for JIT stats in EXPLAIN output in PG11
Date: Fri, 6 Jul 2018 11:14:25 +0530
Message-ID: <CANxoLDenxf+JgEUmoPCLT9uLFx7tQtkg+q5M-v+EAnCp6DJykg@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoyfAKdfMD2aEZziXGU=DGBVw+ApXq7Sa6YnyOESJLVjdw@mail.gmail.com>
References: <CANxoLDdnaPB8gt-v+4aVY2urY8suGqK=pFiWFZ-Agxkgkx_VYw@mail.gmail.com>
<CA+OCxozxjz+m=zy_AN4QPQtNHKuYTpQ6YpihoS=ugpzLe_jvEQ@mail.gmail.com>
<CANxoLDcKr_A+gCBwgdG6GGrc78Rm2mtLsvmL-pEx7UPSPg8kBA@mail.gmail.com>
<CA+OCxow4L5AL8QNYxVckjGVtaN9h+J+72Kg_wjuQ+qJA5ffShw@mail.gmail.com>
<CANxoLDd7P5QKxbhbQ93Hw=gF0NQdFt2pyy5XnQ_LThTVmk8Bnw@mail.gmail.com>
<CA+OCxox5_-7PK787BN9K_OQf_8Bo54Arsw-jFjT8sDsZFgmTBw@mail.gmail.com>
<CANxoLDctoTPfKdhV8t06y=8ROtZVSfifhsbSyCot0Jga9LRHDg@mail.gmail.com>
<CANxoLDeJXAQuG=L50jR=e82Fh89VzbM94Sj90ECENrurBFV5bw@mail.gmail.com>
<CANxoLDco102yaz9Ez0jS5XTCyQNg6a4f1Q35UawpZCf2G1SFEw@mail.gmail.com>
<CA+OCxoyfAKdfMD2aEZziXGU=DGBVw+ApXq7Sa6YnyOESJLVjdw@mail.gmail.com>
Hi
On Thu, Jul 5, 2018 at 7:17 PM, Dave Page <[email protected]> wrote:
> Hi
>
> Looks good to me - the only remaining issue is that the button still isn't
> the same colours as the zoom ones. I haven't found all the differences, but
> there seems to be an 'opacity: 0.5" on the stats area, and the disabled
> attribute is still being set.
>
I have set the disabled attribute, so that the button shouldn't be
clickable and because of that there is difference in colour. 'opacity: 0.5'
is also there for zoom area. I have fixed the issue by removing disabled
attribute and add '
*pointer-events: none*' for the statistics button. Attached is the patch
please review it.
>
> On Thu, Jul 5, 2018 at 11:21 AM, Akshay Joshi <
> [email protected]> wrote:
>
>> Hi Dave
>>
>> As per discussion attached is the modified patch with following review
>> comments has been fixed :
>>
>> - Hide statistics button if not applicable(no statistics to show).
>> - Extract 'StatisticsModel' into a separate file and added jasmine
>> test for this.
>>
>>
>>
>>
>> On Tue, Jul 3, 2018 at 6:40 PM, Akshay Joshi <
>> [email protected]> wrote:
>>
>>> Hi Dave,
>>>
>>> Please ignore the previous patch, I have made some more changes related
>>> to background colour on enabled/disabled state.
>>> Attached is the modified patch. Please review it.
>>>
>>> On Tue, Jul 3, 2018 at 12:35 PM, Akshay Joshi <
>>> [email protected]> wrote:
>>>
>>>> Hi Dave,
>>>>
>>>> On Mon, Jul 2, 2018 at 4:10 PM, Dave Page <[email protected]> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> On Sat, Jun 30, 2018 at 9:15 AM, Akshay Joshi <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi Dave
>>>>>>
>>>>>> On Fri, Jun 29, 2018 at 7:45 PM, Dave Page <[email protected]> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Jun 29, 2018 at 3:12 PM, Akshay Joshi <
>>>>>>> [email protected]> wrote:
>>>>>>>
>>>>>>>> Hi Dave
>>>>>>>>
>>>>>>>> On Fri, Jun 29, 2018 at 6:56 PM, Dave Page <[email protected]>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> On Fri, Jun 29, 2018 at 9:55 AM, Akshay Joshi <
>>>>>>>>> [email protected]> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Hackers,
>>>>>>>>>>
>>>>>>>>>> Attached is the patch to fix the RM #3397 Add support for JIT
>>>>>>>>>> stats in EXPLAIN output in PG11. Please review it.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> A couple of immediate thoughts:
>>>>>>>>>
>>>>>>>>> - When the canvas is first rendered, there's a vertical scrollbar
>>>>>>>>> now. As soon as I mouseover the new icon, it vanishes and the icon jumps to
>>>>>>>>> the right.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Will look into it. Vertical scrollbar comes even if you remove
>>>>>>>> my patch and try to hover any image.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> - The icon seems lighter than the other controls on the left.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Same css has been applied, only difference is button is
>>>>>>>> disabled.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> - The icon isn't disabled when there is no info to show.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Button is always disabled, I have just change the opacity.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks.
>>>>>>>>>
>>>>>>>>
>>>>>>> Maybe - but I can still click it and it reacts as if it's active. It
>>>>>>> may be lighter to indicate that it's disabled, but its not behaving as
>>>>>>> such.
>>>>>>>
>>>>>>
>>>>>> Attached is the modified patch. Please review it.
>>>>>>
>>>>>
>>>>> The button still changes foreground colour on mouseover when
>>>>> disabled. I think it needs to be completely non-reactive when disabled. It
>>>>> should also be a noticably lighter shade when disabled; right now it seems
>>>>> to be darker than the other buttons (see attached).
>>>>>
>>>>
>>>> Attached is the modified patch. Please review it.
>>>>
>>>>>
>>>>> --
>>>>> Dave Page
>>>>> Blog: http://pgsnake.blogspot.com
>>>>> Twitter: @pgsnake
>>>>>
>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>> The Enterprise PostgreSQL Company
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> *Akshay Joshi*
>>>>
>>>> *Sr. Software Architect *
>>>>
>>>>
>>>>
>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>
>>>
>>>
>>>
>>> --
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect *
>>>
>>>
>>>
>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>
>>
>>
>>
>> --
>> *Akshay Joshi*
>>
>> *Sr. Software Architect *
>>
>>
>>
>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
--
*Akshay Joshi*
*Sr. Software Architect *
*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
Attachments:
[application/octet-stream] RM_3397_v6.patch (12.9K, 3-RM_3397_v6.patch)
download | inline diff:
diff --git a/web/pgadmin/feature_tests/query_tool_tests.py b/web/pgadmin/feature_tests/query_tool_tests.py
index ac463d3..226b323 100644
--- a/web/pgadmin/feature_tests/query_tool_tests.py
+++ b/web/pgadmin/feature_tests/query_tool_tests.py
@@ -104,6 +104,16 @@ class QueryToolFeatureTest(BaseFeatureTest):
self._query_tool_notify_statements()
self._clear_query_tool()
+ # explain query with JIT stats
+ print("Explain query with JIT stats... ",
+ file=sys.stderr, end="")
+ if self._supported_jit_on_server():
+ self._query_tool_explain_check_jit_stats()
+ print("OK.", file=sys.stderr)
+ self._clear_query_tool()
+ else:
+ print("Skipped.", file=sys.stderr)
+
def after(self):
self.page.remove_server(self.server)
connection = test_utils.get_db_connection(
@@ -660,9 +670,62 @@ SELECT 1, pg_sleep(300)"""
wait.until(WaitForAnyElementWithText(
(By.CSS_SELECTOR, 'td.payload'), "Hello"))
print("OK.", file=sys.stderr)
+ self._clear_query_tool()
else:
print("Skipped.", file=sys.stderr)
+ def _supported_jit_on_server(self):
+ connection = test_utils.get_db_connection(
+ self.server['db'],
+ self.server['username'],
+ self.server['db_password'],
+ self.server['host'],
+ self.server['port'],
+ self.server['sslmode']
+ )
+
+ pg_cursor = connection.cursor()
+ pg_cursor.execute('select version()')
+ version_string = pg_cursor.fetchone()
+
+ is_edb = False
+ if len(version_string) > 0:
+ is_edb = 'EnterpriseDB' in version_string[0]
+
+ connection.close()
+
+ return connection.server_version >= 110000 and not is_edb
+
+ def _query_tool_explain_check_jit_stats(self):
+ wait = WebDriverWait(self.page.driver, 10)
+
+ self.page.fill_codemirror_area_with("SET jit_above_cost=10;")
+ self.page.find_by_id("btn-flash").click()
+ self.page.wait_for_query_tool_loading_indicator_to_disappear()
+ self._clear_query_tool()
+
+ self.page.fill_codemirror_area_with("SELECT count(*) FROM pg_class;")
+ query_op = self.page.find_by_id("btn-query-dropdown")
+ query_op.click()
+ ActionChains(self.driver).move_to_element(
+ query_op.find_element_by_xpath(
+ "//li[contains(.,'Explain Options')]")).perform()
+
+ self.page.find_by_id("btn-explain-verbose").click()
+ self.page.find_by_id("btn-explain-costs").click()
+ self.page.find_by_id("btn-explain-analyze").click()
+
+ self.page.wait_for_query_tool_loading_indicator_to_disappear()
+ self.page.click_tab('Data Output')
+
+ canvas = wait.until(EC.presence_of_element_located(
+ (By.CSS_SELECTOR, "#datagrid .slick-viewport .grid-canvas"))
+ )
+ # Search for 'Output' word in result (verbose option)
+ canvas.find_element_by_xpath("//*[contains(string(), 'JIT')]")
+
+ self._clear_query_tool()
+
class WaitForAnyElementWithText(object):
def __init__(self, locator, text):
diff --git a/web/pgadmin/misc/static/explain/css/explain.css b/web/pgadmin/misc/static/explain/css/explain.css
index d549f85..98a0775 100644
--- a/web/pgadmin/misc/static/explain/css/explain.css
+++ b/web/pgadmin/misc/static/explain/css/explain.css
@@ -17,6 +17,24 @@
opacity: 1;
}
+.pg-explain-stats-area {
+ position: absolute;
+ top: 5px;
+ right: 25px;
+ opacity: 0.5;
+}
+
+.pg-explain-stats-btn {
+ top: 5px;
+ min-width: 25px;
+ border: 1px solid transparent;
+ pointer-events: none;
+}
+
+.pg-explain-stats-area:hover {
+ opacity: 1;
+}
+
.explain-tooltip {
display: table-cell;
text-align: left;
@@ -37,8 +55,6 @@ td.explain-tooltip-val {
.pgadmin-explain-tooltip {
position: absolute;
- padding:5px;
- border: 1px solid white;
opacity:0;
color: cornsilk;
background-color: #010125;
@@ -55,4 +71,4 @@ td.explain-tooltip-val {
height: 100%;
width: 100%;
overflow: auto;
-}
\ No newline at end of file
+}
diff --git a/web/pgadmin/misc/static/explain/js/explain.js b/web/pgadmin/misc/static/explain/js/explain.js
index e028774..e27d810 100644
--- a/web/pgadmin/misc/static/explain/js/explain.js
+++ b/web/pgadmin/misc/static/explain/js/explain.js
@@ -1,7 +1,7 @@
define('pgadmin.misc.explain', [
'sources/url_for', 'jquery', 'underscore', 'underscore.string',
- 'sources/pgadmin', 'backbone', 'snapsvg',
-], function(url_for, $, _, S, pgAdmin, Backbone, Snap) {
+ 'sources/pgadmin', 'backbone', 'snapsvg', 'explain_statistics',
+], function(url_for, $, _, S, pgAdmin, Backbone, Snap, StatisticsModel) {
pgAdmin = pgAdmin || window.pgAdmin || {};
@@ -615,6 +615,9 @@ define('pgadmin.misc.explain', [
});
toolTipContainer.css('left', toolTipX);
toolTipContainer.css('top', toolTipY);
+
+ $('.pgadmin-explain-tooltip').css('padding', '5px');
+ $('.pgadmin-explain-tooltip').css('border', '1px solid white');
});
// Remove tooltip when mouse is out from node's area
@@ -696,6 +699,7 @@ define('pgadmin.misc.explain', [
},
initialize: function() {
this.set('Plan', new PlanModel());
+ this.set('Statistics', new StatisticsModel());
},
// Parse the JSON data and fetch its children plans
@@ -718,6 +722,17 @@ define('pgadmin.misc.explain', [
delete data['Plan'];
}
+ var statistics = this.get('Statistics');
+ if (data && 'JIT' in data) {
+ statistics.set('JIT', data['JIT']);
+ delete data ['JIT'];
+ }
+
+ if (data && 'Triggers' in data) {
+ statistics.set('Triggers', data['Triggers']);
+ delete data ['Triggers'];
+ }
+
return data;
},
toJSON: function() {
@@ -745,6 +760,10 @@ define('pgadmin.misc.explain', [
plan.draw(
g, xpos, ypos, undefined, undefined, graphContainer, toolTipContainer
);
+
+ //Set the Statistics as tooltip
+ var statistics = this.get('Statistics');
+ statistics.set_statistics(toolTipContainer);
},
});
@@ -784,6 +803,21 @@ define('pgadmin.misc.explain', [
class: 'fa fa-search-minus',
}));
+ var statsArea = $('<div></div>', {
+ class: 'pg-explain-stats-area btn-group hidden',
+ role: 'group',
+ }).appendTo(container);
+
+ $('<button></button>', {
+ id: 'btn-explain-stats',
+ class: 'btn pg-explain-stats-btn badge',
+ title: 'Statistics',
+ tabindex: 0,
+ }).appendTo(statsArea).append(
+ $('<i></i>', {
+ class: 'fa fa-line-chart',
+ }));
+
// Main div to be drawn all images on
var planDiv = $('<div></div>', {
class: 'pgadmin-explain-container',
diff --git a/web/pgadmin/misc/static/explain/js/explain_statistics.js b/web/pgadmin/misc/static/explain/js/explain_statistics.js
new file mode 100644
index 0000000..369a671
--- /dev/null
+++ b/web/pgadmin/misc/static/explain/js/explain_statistics.js
@@ -0,0 +1,90 @@
+import $ from 'jquery';
+import Backbone from 'backbone';
+
+// Backbone model for other statistics
+let StatisticsModel = Backbone.Model.extend({
+ defaults: {
+ JIT: [],
+ Triggers: [],
+ },
+
+ set_statistics: function(toolTipContainer) {
+ var jit_stats = this.get('JIT'),
+ triggers_stats = this.get('Triggers');
+
+ if (Object.keys(jit_stats).length > 0 ||
+ Object.keys(triggers_stats).length > 0) {
+ $('.pg-explain-stats-area').removeClass('hidden');
+ }
+
+ $('.pg-explain-stats-area').on('mouseover', () => {
+
+ // Empty the tooltip content if it has any and add new data
+ toolTipContainer.empty();
+ if (Object.keys(jit_stats).length == 0 &&
+ Object.keys(triggers_stats).length == 0) {
+ return;
+ }
+
+ var tooltip = $('<table></table>', {
+ class: 'pgadmin-tooltip-table',
+ }).appendTo(toolTipContainer);
+
+ if (Object.keys(jit_stats).length > 0){
+ tooltip.append('<tr><td class="label explain-tooltip">JIT:</td></tr>');
+ _.each(jit_stats, function(value, key) {
+ tooltip.append('<tr><td class="label explain-tooltip">  '
+ + key + '</td><td class="label explain-tooltip-val">'
+ + value + '</td></tr>');
+ });
+ }
+
+ if (Object.keys(triggers_stats).length > 0){
+ tooltip.append('<tr><td class="label explain-tooltip">Triggers:</td></tr>');
+ _.each(triggers_stats, function(triggers, key_id) {
+ if (triggers instanceof Object) {
+ _.each(triggers, function(value, key) {
+ if (key === 'Trigger Name') {
+ tooltip.append('<tr><td class="label explain-tooltip"> '
+ + key + '</td><td class="label explain-tooltip-val">'
+ + value + '</td></tr>');
+ } else {
+ tooltip.append('<tr><td class="label explain-tooltip"> '
+ + key + '</td><td class="label explain-tooltip-val">'
+ + value + '</td></tr>');
+ }
+ });
+ }
+ else {
+ tooltip.append('<tr><td class="label explain-tooltip"> '
+ + key_id + '</td><td class="label explain-tooltip-val">'
+ + triggers + '</td></tr>');
+ }
+ });
+ }
+
+ // Show toolTip at respective x,y coordinates
+ toolTipContainer.css({
+ 'opacity': '0.8',
+ 'left': '',
+ 'right': '65px',
+ 'top': '15px',
+ });
+
+ $('.pgadmin-explain-tooltip').css('padding', '5px');
+ $('.pgadmin-explain-tooltip').css('border', '1px solid white');
+ });
+
+ // Remove tooltip when mouse is out from node's area
+ $('.pg-explain-stats-area').on('mouseout', () => {
+ toolTipContainer.empty();
+ toolTipContainer.css({
+ 'opacity': '0',
+ 'left': 0,
+ 'top': 0,
+ });
+ });
+ },
+});
+
+module.exports = StatisticsModel;
diff --git a/web/regression/javascript/misc/explain/explain_statistics_spec.js b/web/regression/javascript/misc/explain/explain_statistics_spec.js
new file mode 100644
index 0000000..0b2ec67
--- /dev/null
+++ b/web/regression/javascript/misc/explain/explain_statistics_spec.js
@@ -0,0 +1,87 @@
+import StatisticsModel from '../../../../pgadmin/misc/static/explain/js/explain_statistics';
+import $ from 'jquery';
+
+describe('ExplainStatistics', () => {
+ let statsModel;
+ let statsDiv;
+ let tooltipContainer;
+
+ beforeEach(function() {
+ statsModel = new StatisticsModel();
+ statsDiv = '<div class="pg-explain-stats-area btn-group hidden"></div>';
+ tooltipContainer = $('<div></div>', {
+ id: 'toolTip',
+ class: 'pgadmin-explain-tooltip',
+ });
+ });
+
+ describe('No Statistics', () => {
+ it('Statistics button should be hidden', () => {
+ $('body').append(statsDiv);
+
+ statsModel.set('JIT', []);
+ statsModel.set('Triggers', []);
+ statsModel.set_statistics(tooltipContainer);
+
+ expect($('.pg-explain-stats-area').hasClass('hidden')).toBe(true);
+ });
+ });
+
+ describe('JIT Statistics', () => {
+ beforeEach(function() {
+ $('body').append(statsDiv);
+ statsModel.set('JIT', [{'cost': '100'}]);
+ statsModel.set('Triggers', []);
+ statsModel.set_statistics(tooltipContainer);
+ });
+
+ it('Statistics button should be visible', () => {
+ expect($('.pg-explain-stats-area').hasClass('hidden')).toBe(false);
+ });
+
+ it('Mouse over event should be trigger', () => {
+ // Trigger mouse over event
+ var hoverEvent = new $.Event('mouseover');
+ $('.pg-explain-stats-area').trigger(hoverEvent);
+
+ expect(tooltipContainer.css('opacity')).toBe('0.8');
+ });
+
+ it('Mouse out event should be trigger', () => {
+ // Trigger mouse out event
+ var hoverEvent = new $.Event('mouseout');
+ $('.pg-explain-stats-area').trigger(hoverEvent);
+
+ expect(tooltipContainer.css('opacity')).toBe('0');
+ });
+ });
+
+ describe('Triggers Statistics', () => {
+ beforeEach(function() {
+ $('body').append(statsDiv);
+ statsModel.set('JIT', []);
+ statsModel.set('Triggers', [{'name': 'test_trigger'}]);
+ statsModel.set_statistics(tooltipContainer);
+ });
+
+ it('Statistics button should be visible', () => {
+ expect($('.pg-explain-stats-area').hasClass('hidden')).toBe(false);
+ });
+
+ it('Mouse over event should be trigger', () => {
+ // Trigger mouse over event
+ var hoverEvent = new $.Event('mouseover');
+ $('.pg-explain-stats-area').trigger(hoverEvent);
+
+ expect(tooltipContainer.css('opacity')).toBe('0.8');
+ });
+
+ it('Mouse out event should be trigger', () => {
+ // Trigger mouse out event
+ var hoverEvent = new $.Event('mouseout');
+ $('.pg-explain-stats-area').trigger(hoverEvent);
+
+ expect(tooltipContainer.css('opacity')).toBe('0');
+ });
+ });
+});
view thread (12+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [pgAdmin4][Patch]: RM #3397 Add support for JIT stats in EXPLAIN output in PG11
In-Reply-To: <CANxoLDenxf+JgEUmoPCLT9uLFx7tQtkg+q5M-v+EAnCp6DJykg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox