Implementation of explained plan in XML format #223

livius2 · 2019-09-05T08:11:10Z

I forgot about this patch creaded few years ago.
I have modified it slightly and i it migrated from svn to git.

It implement explained plan in format XML.
To test it add e.g in trace option
explain_plan_xml=true

It was discussed previously and need some addoption, but to not forgot about i post this pull request as it will be also simplier to modify something.

PS> if someone can point me how to retrive specified value of e.g. ROWS 5 in plan printing in FirstRowsStream.cpp then i can add more infos probably in other places too.
I mean that i need to know this "5" number. finded self

PS2>make_icu, make_boot, make_all without errors, i do not know why below continuous-integration/travis-ci/pr failed somehow

Implementation of explained plan in XML

asfernandes · 2019-09-05T10:50:59Z

I think a machine readable plan would be very good, but much better in json than xml.

livius2 · 2019-09-05T12:10:02Z

I can add json format too if needed.

PS. User after connected to db need to choose prefered format presented in e.g. MON$STATEMENTS.
Can you point me how can i add some session (connection or transaction) switch to allow choice about plan format? Is there already some switches too look at?

EDIT

Can i use config same as is for set time zone bind native?
But how to do this only for current transaction?
I think about tool where user can set default settings for connection but tool itself use only one specified format and that tool start custom transaction and can set custom settings for that.

AppVeyorBot · 2019-09-05T12:39:02Z

❌ Build firebird 1.0.946 failed (commit 8d6bb0d0ff by @)

aafemt · 2019-09-09T10:04:16Z

I don't see writing of XML header. Is resulting XML valid from standard point of view? I'e does it pass validation at https://www.w3schools.com/xml/xml_validator.asp for example? Is the result still valid for non-ascii object names?

livius2 · 2019-09-09T10:18:16Z

It is valid, e.g. Postgress also do not show header
But i do not know what to add here utf8?
<?xml version="1.0" encoding="UTF-8"?>

AppVeyorBot · 2019-09-09T10:26:31Z

❌ Build firebird 1.0.959 failed (commit 948afa1b92 by @)

aafemt · 2019-09-09T10:27:14Z

But i do not know what to add here utf8?

Yes, UTF-8 seems to be the only option given that table names are kept in it. But you also must ensure that XML plan is not converted to connection charset during delivery to client. BTW, did you check that it works with something like this: "select ... from "<xml>" """""?

livius2 · 2019-09-09T10:47:50Z

Yes, UTF-8 seems to be the only option given that table names are kept in it. But you also must ensure that XML plan is not converted to connection charset during delivery to client.

I did not analysed this because we have currently explained lagacy plan. I supposed this problem was solved somehow (or simply ignored).

BTW, did you check that it works with something like this: "select ... from "" """""?

good catch :)
It will not cause problem on Firebird side but xml will not be a valid from client POV.
I need escape names here.

livius2 · 2019-09-09T13:19:35Z

added header of xml and also escape

example for (with escape for alias):
SELECT * FROM RDB$DATABASE "<xml>" INNER JOIN RDB$RELATIONS R ON "<xml>".RDB$RELATION_ID = R.RDB$RELATION_ID

<?xml version="1.0" encoding="UTF-8"?> 
<Select_Expression xmlns="http://www.firebirdsql.org"> 
	<Node Operation="Nested Loop Join" JoinType="Inner"> 
			<Table alias="&lt;xml&gt;">RDB$DATABASE</Table> 
			<Node Operation="Full Scan"> 
			</Node> 
		<Node Operation="Filter"> 
			<Table Alias="R" Access="By ID">RDB$RELATIONS</Table> 
			<Node Operation="Bitmap"> 
				<Index>RDB$INDEX_1</Index> 
				<Scan>Range</Scan> 
				<Match>full</Match> 
			</Node> 
		</Node> 
	</Node> 
</Select_Expression>

livius2 · 2019-09-10T07:30:05Z

I think a machine readable plan would be very good, but much better in json than xml.

I see that JSON is harder to addapt in the current code then XML or YAML. JSON have separator between elements ,
And in every place before print i must know if i must put comma or not.

hvlad · 2019-09-10T08:33:46Z

src/include/firebird/FirebirdInterface.idl

@@ -448,7 +449,7 @@ interface Statement : ReferenceCounted
 								 uint itemsLength, const uchar* items,
 								 uint bufferLength, uchar* buffer);
 	uint getType(Status status);
-	const string getPlan(Status status, boolean detailed);
+	const string getPlan(Status status, isc_info_sql_plan_format plan_format);


The build is failed because of this line:

/home/appveyor/projects/firebird/gen/Release/cloop/release/bin/cloop /home/appveyor/projects/firebird/src/include/firebird/FirebirdInterface.idl pascal /home/appveyor/projects/firebird/src/include/gen/Firebird.pas Firebird --uses SysUtils
--interfaceFile /home/appveyor/projects/firebird/src/misc/pascal/Pascal.interface.pas
--implementationFile /home/appveyor/projects/firebird/src/misc/pascal/Pascal.implementation.pas
--exceptionClass FbException
--functionsFile /home/appveyor/projects/firebird/temp/Release/func.pas
--prefix I
/home/appveyor/projects/firebird/src/include/firebird/FirebirdInterface.idl:452:38: error: Interface/struct 'isc_info_sql_plan_format' not found.

If I not mistaken, you have to add isc_info_sql_plan_format definition into /src/misc/pascal/Pascal.interface.pas

Also, I'm not sure it is legal to change interface method declaration, you should add new method instead.

AlexPeshkoff · 2019-09-10T09:23:03Z

On 10.09.2019 11:33, Vlad Khorsun wrote: ***@***.**** commented on this pull request. ------------------------------------------------------------------------ In src/include/firebird/FirebirdInterface.idl <#223 (comment)>: > @@ -448,7 +449,7 @@ interface Statement : ReferenceCounted uint itemsLength, const uchar* items, uint bufferLength, uchar* buffer); uint getType(Status status); - const string getPlan(Status status, boolean detailed); + const string getPlan(Status status, isc_info_sql_plan_format plan_format); The build is failed because of this line: /home/appveyor/projects/firebird/gen/Release/cloop/release/bin/cloop /home/appveyor/projects/firebird/src/include/firebird/FirebirdInterface.idl pascal /home/appveyor/projects/firebird/src/include/gen/Firebird.pas Firebird --uses SysUtils --interfaceFile /home/appveyor/projects/firebird/src/misc/pascal/Pascal.interface.pas --implementationFile /home/appveyor/projects/firebird/src/misc/pascal/Pascal.implementation.pas --exceptionClass FbException --functionsFile /home/appveyor/projects/firebird/temp/Release/func.pas --prefix I /home/appveyor/projects/firebird/src/include/firebird/FirebirdInterface.idl:452:38: error: Interface/struct 'isc_info_sql_plan_format' not found.

That struct should be described in .idl file to avoid the error. BTW, I'm unsure (far unsure) is it worth adding new plain struct to API. Better avoid that.

AlexPeshkoff · 2019-09-10T09:40:33Z

On 10.09.2019 12:24, Vlad Khorsun wrote: ***@***.**** commented on this pull request. ------------------------------------------------------------------------ In src/include/firebird/FirebirdInterface.idl <#223 (comment)>: > @@ -448,7 +449,7 @@ interface Statement : ReferenceCounted uint itemsLength, const uchar* items, uint bufferLength, uchar* buffer); uint getType(Status status); - const string getPlan(Status status, boolean detailed); + const string getPlan(Status status, isc_info_sql_plan_format plan_format); If I not mistaken, you have to add |isc_info_sql_plan_format| definition into /src/misc/pascal/Pascal.interface.pas Also, I'm not sure it is legal to change interface method declaration,

Absolutely illegal - did not pay attention first.

…

you should add new method instead. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#223?email_source=notifications&email_token=AA44OUJMFTKHAQENCO2QCRTQI5RU7A5CNFSM4IT2XK32YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCEGEJLY#discussion_r322639161>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA44OUO7UV3TUYAIYCZ3S73QI5RU7ANCNFSM4IT2XK3Q>.

livius2 · 2019-09-10T09:57:00Z

>Absolutely illegal - did not pay attention first.

New method in same interface will be ok as Vlad Korshun suggested?

aafemt · 2019-09-10T10:01:59Z

Absolutely illegal - did not pay attention first.

Isn't it fine as long as size of parameter isn't changed and only value range is expanded in compatible way? Boolean is one byte anyway.

livius2 · 2019-09-10T10:06:19Z

This will be very good to me if accepted. I only will need to change enum to start with `0` with plain plan, `1` will be current explained format, `2` xml.

AlexPeshkoff · 2019-09-10T10:06:29Z

On 10.09.2019 13:02, Dimitry Sibiryakov wrote: Absolutely illegal - did not pay attention first. Isn't it fine as long as size of parameter isn't changed and only value range is expanded in compatible way? Boolean is one byte anyway.

Except one issue - what value will be passed as true by old invoker? 1 or 0377 or 0177 or something else non-zero? How will new engin eprocess it?

aafemt · 2019-09-10T10:17:39Z

Except one issue - what value will be passed as true by old invoker? 1

Yes, true is guaranteed to be 1 in C++ and Delphi.

hvlad · 2019-09-10T10:22:21Z

Absolutely illegal - did not pay attention first.

Isn't it fine as long as size of parameter isn't changed and only value range is expanded in compatible way? Boolean is one byte anyway.

No, it is not fine.
Data type change == declaration change == contract change == interface change == illegal

livius2 · 2019-09-10T12:41:59Z

From user POV declaration is not changed. If someone do not retrive new "header" with new interface "version" than all interface functions still working without change needed. Then interface is not broken and is still compatybile. But you decide. I will adapt somehow (if i only know how to change something). Is this final decision? PS> i did not realized that i answer to git not firebird group ;-) I have edited some posts to make them clearer.

hvlad · 2019-09-11T08:13:14Z

From user POV declaration is not changed.

It is changed

If someone do not retrive new "header" with new interface "version" than all interface functions still working without change needed.

But if someone ask new functionality using old implementation - he will be badly surprised, at least.

Then interface is not broken and is still compatybile.

Not completely true.

But you decide. I will adapt somehow (if i only know how to change something). Is this final decision?

Add new method to the interface and mark it as new version, for example:

interface Statement : ReferenceCounted
{
...
version:  // 3.0 -> 4.0
  const string getPlanXml(Status status);
}

hvlad · 2019-09-11T08:15:15Z

src/include/firebird/FirebirdInterface.idl

@@ -1218,6 +1219,7 @@ interface TraceSQLStatement : TraceStatement
 	TraceParams getInputs();
 	const string getTextUTF8();
 	const string getExplainedPlan();
+	const string getExplainedPlanXml();


Should be marked as new version, add lable version: before the new method

livius2 · 2019-09-11T09:39:06Z

Add new method to the interface and mark it as new version, for example:
interface Statement : ReferenceCounted
{
...
version:  // 3.0 -> 4.0
  const string getPlanXml(Status status);
}

ok.

but is this only place or i must revert change also in e.g.
StatementMetadata::getPlan(isc_info_sql_plan_format plan_format)
and i should add getPlanXml there

And is this supported to have oveloaded functions in above interface? If yes, maybe better choice?

const string getPlan(Status status, boolean detailed);
const string getPlan(Status status, isc_info_sql_plan_format plan_format);

My knowledge about C++ is strongly limited here.

hvlad · 2019-09-11T09:48:07Z

but is this only place or i must revert change also in e.g.
StatementMetadata::getPlan(isc_info_sql_plan_format plan_format)
and i should add getPlanXml there

Yes. Revert change (interface method declaration should never be changed!) and add new method.

And is this supported to have oveloaded functions in above interface? If yes, maybe better choice?

Please, don't add confusion, use different method name.
As you already did with TraceSQLStatementImpl, btw :)

aafemt · 2019-09-11T09:52:36Z

Yes. Revert change (interface method declaration should never be changed!) and add new method.

But rather than getPlanXml I would prefer getPlanEx with choice of format. It is easier to expand later if needed.

livius2 · 2019-09-11T10:06:58Z

Yes. Revert change (interface method declaration should never be changed!) and add new method.
But rather than getPlanXml I would prefer getPlanEx with choice of format. It is easier to expand later if needed.

Good concept for me. All already existing getPlan(bool explained) will simply call one method with appropriate parameter.
But name maybe getPlanInFormat?

dyemanov · 2019-09-11T11:18:10Z

But name maybe getPlanInFormat?

getFormattedPlan() maybe?

hvlad · 2019-09-11T12:36:04Z

But name maybe getPlanInFormat?

getFormattedPlan() maybe?

getPlanEx is better for me, but definitely not getPlanInFormat :)

livius2 · 2019-09-11T13:26:52Z

I suppose i have fixed all problems reported.
I have used name getFormattedPlan if needed i can change it to something else getPlanFormatted, getPlanEx ...

I think about introducing new SET command to choose plan stored in MON$STATEMENTS. Or is there a better way?

PS> it looks like "continuous-integration" is now ok

AppVeyorBot · 2019-09-11T13:37:42Z

✅ Build firebird 1.0.962 completed (commit 02aa1f8427 by @)

Information of First and Skip values. e.g. SELECT FIRST 5 SKIP 2 * FROM RDB$RELATIONS we will see limitRows=5 skipRows=2

Only TYPE_LITERAL nodes are accepted. Any other can contain subquery e.g. SELECT * FROM RDB$RELATIONS R ROWS (SELECT COUNT(*) FROM RDB$DATABASE) SELECT * FROM RDB$RELATIONS R ROWS (5-(SELECT COUNT(*) FROM RDB$DATABASE))

AppVeyorBot · 2019-09-18T10:05:38Z

✅ Build firebird 1.0.988 completed (commit 7486e4e5c5 by @)

… plan store statistics into plan about: - memory; - physical IO global and global per table; - logical IO global and global per table.

livius2 · 2019-09-20T12:11:21Z

store statistics into plan about:

memory;
physical IO global and global per table;
logical IO global and global per table.

Currently can be used to store plan into MON$STATEMENTS with cumulative info during statement execution, runned again get updated statistics.
Slightly related to CORE-6030, but there is need for info about particullar streams.

As still there is no SET command to change format of the plan,
to test it, change format of plan in Monitoring::dumpAttachment from isc_info_sql_plan_format_explain_legacy to isc_info_sql_plan_format_explain_xml.

sample plan:

<?xml version="1.0" encoding="UTF-8"?>
<Statements>
	<Statistics>
		<PhysicalIO>
			<PageReads>54</PageReads>
			<PageWrites>0</PageWrites>
			<PageFetches>7239</PageFetches>
			<PageMarks>0</PageMarks>
		</PhysicalIO>
		<LogicalIO>
			<RecordSeqReads>7022</RecordSeqReads>
			<RecordIdxReads>0</RecordIdxReads>
			<RecordInserts>0</RecordInserts>
			<RecordUpdates>0</RecordUpdates>
			<RecordDeletes>0</RecordDeletes>
			<RecordBackouts>0</RecordBackouts>
			<RecordPurges>0</RecordPurges>
			<RecordExpurges>0</RecordExpurges>
			<RecordLocks>0</RecordLocks>
			<RecordWaits>0</RecordWaits>
			<RecordConflicts>0</RecordConflicts>
			<RecordBackversionReads>0</RecordBackversionReads>
			<RecordFragmentReads>0</RecordFragmentReads>
			<RecordRptReads>0</RecordRptReads>
			<RecordImgc>0</RecordImgc>
		</LogicalIO>
		<TableStats>
			<Table name="WPLATA">
				<LogicalIO>
					<LogicalIO>
						<RecordSeqReads>7022</RecordSeqReads>
						<RecordIdxReads>0</RecordIdxReads>
						<RecordInserts>0</RecordInserts>
						<RecordUpdates>0</RecordUpdates>
						<RecordDeletes>0</RecordDeletes>
						<RecordBackouts>0</RecordBackouts>
						<RecordPurges>0</RecordPurges>
						<RecordExpurges>0</RecordExpurges>
						<RecordLocks>0</RecordLocks>
						<RecordWaits>0</RecordWaits>
						<RecordConflicts>0</RecordConflicts>
						<RecordBackversionReads>0</RecordBackversionReads>
						<RecordFragmentReads>0</RecordFragmentReads>
						<RecordRptReads>0</RecordRptReads>
						<RecordImgc>0</RecordImgc>
					</LogicalIO>
				</LogicalIO>
			</Table>
		</TableStats>
	</Statistics>
	<Statement xmlns="https://www.firebirdsql.org/2019/ExecutionPlan">
		<Node operation="First N Records" limitRows="10000">
			<Node operation="Full Scan">
			<Table alias="WPLATA">WPLATA</Table>
			</Node>
		</Node>
	</Statement>
</Statements>

AppVeyorBot · 2019-09-25T13:41:12Z

✅ Build firebird 1.0.1009 completed (commit 38e61cea5e by @)

liviuslivius@poczta.onet.pl added 3 commits September 5, 2019 09:58

Explained plan format XML

9f91f55

Implementation of explained plan in XML

Fix char case

784b205

Another char case and format

d6f374a

unnescessary line

685cb69

added xmlns

5bc28ce

Fix closing apostrophes when extras occurred

3abc02a

header and escape for xml

2e8d876

hvlad reviewed Sep 10, 2019

View reviewed changes

hvlad reviewed Sep 11, 2019

View reviewed changes

Fixed interface and some minor changes

23105b9

liviuslivius@poczta.onet.pl added 5 commits September 11, 2019 15:52

a little simplification

dfb4906

wrong name used

72ce875

Xml Schema

f0625ac

More info in plan for First and Skip

a1068ee

Information of First and Skip values. e.g. SELECT FIRST 5 SKIP 2 * FROM RDB$RELATIONS we will see limitRows=5 skipRows=2

Fix provide info about First Skip only for TYPE_LITERAL node only

a0d3008

Only TYPE_LITERAL nodes are accepted. Any other can contain subquery e.g. SELECT * FROM RDB$RELATIONS R ROWS (SELECT COUNT(*) FROM RDB$DATABASE) SELECT * FROM RDB$RELATIONS R ROWS (5-(SELECT COUNT(*) FROM RDB$DATABASE))

liviuslivius@poczta.onet.pl added 2 commits September 19, 2019 08:24

remove not needed "Impure" from debug code

9880d9a

store statistic about memory and physical/logical operations into xml…

034593c

… plan store statistics into plan about: - memory; - physical IO global and global per table; - logical IO global and global per table.

liviuslivius@poczta.onet.pl and others added 2 commits September 20, 2019 14:42

Remove double "LogicalIO" node for table stats

757a4ab

updated xml schema and fixed validity

ebb845a

karol.bieniaszewski@zaiks.org.pl added 2 commits September 26, 2019 10:29

fix name in closing tag

cbbd9ad

Update ExplainedPlanXmlSchema.xsd

c5d2cc2

livius2 mentioned this pull request Jul 14, 2023

EXPLAIN statement and RDB$SQL package #7675

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of explained plan in XML format #223

Implementation of explained plan in XML format #223

livius2 commented Sep 5, 2019 •

edited

asfernandes commented Sep 5, 2019

livius2 commented Sep 5, 2019 •

edited

AppVeyorBot commented Sep 5, 2019

aafemt commented Sep 9, 2019

livius2 commented Sep 9, 2019

AppVeyorBot commented Sep 9, 2019

aafemt commented Sep 9, 2019 via email

livius2 commented Sep 9, 2019

livius2 commented Sep 9, 2019 •

edited

livius2 commented Sep 10, 2019

hvlad Sep 10, 2019

hvlad Sep 10, 2019

AlexPeshkoff commented Sep 10, 2019 via email

AlexPeshkoff commented Sep 10, 2019 via email

livius2 commented Sep 10, 2019 via email •

edited

aafemt commented Sep 10, 2019

livius2 commented Sep 10, 2019 via email •

edited

AlexPeshkoff commented Sep 10, 2019 via email

aafemt commented Sep 10, 2019 via email

hvlad commented Sep 10, 2019

livius2 commented Sep 10, 2019 via email •

edited

hvlad commented Sep 11, 2019

hvlad Sep 11, 2019

livius2 commented Sep 11, 2019 •

edited

hvlad commented Sep 11, 2019

aafemt commented Sep 11, 2019 via email

livius2 commented Sep 11, 2019

dyemanov commented Sep 11, 2019

hvlad commented Sep 11, 2019

livius2 commented Sep 11, 2019

AppVeyorBot commented Sep 11, 2019

AppVeyorBot commented Sep 18, 2019

livius2 commented Sep 20, 2019

AppVeyorBot commented Sep 25, 2019

Implementation of explained plan in XML format #223

Are you sure you want to change the base?

Implementation of explained plan in XML format #223

Conversation

livius2 commented Sep 5, 2019 • edited

asfernandes commented Sep 5, 2019

livius2 commented Sep 5, 2019 • edited

AppVeyorBot commented Sep 5, 2019

aafemt commented Sep 9, 2019

livius2 commented Sep 9, 2019

AppVeyorBot commented Sep 9, 2019

aafemt commented Sep 9, 2019 via email

livius2 commented Sep 9, 2019

livius2 commented Sep 9, 2019 • edited

livius2 commented Sep 10, 2019

hvlad Sep 10, 2019

Choose a reason for hiding this comment

hvlad Sep 10, 2019

Choose a reason for hiding this comment

AlexPeshkoff commented Sep 10, 2019 via email

AlexPeshkoff commented Sep 10, 2019 via email

livius2 commented Sep 10, 2019 via email • edited

aafemt commented Sep 10, 2019

livius2 commented Sep 10, 2019 via email • edited

AlexPeshkoff commented Sep 10, 2019 via email

aafemt commented Sep 10, 2019 via email

hvlad commented Sep 10, 2019

livius2 commented Sep 10, 2019 via email • edited

hvlad commented Sep 11, 2019

hvlad Sep 11, 2019

Choose a reason for hiding this comment

livius2 commented Sep 11, 2019 • edited

hvlad commented Sep 11, 2019

aafemt commented Sep 11, 2019 via email

livius2 commented Sep 11, 2019

dyemanov commented Sep 11, 2019

hvlad commented Sep 11, 2019

livius2 commented Sep 11, 2019

AppVeyorBot commented Sep 11, 2019

AppVeyorBot commented Sep 18, 2019

livius2 commented Sep 20, 2019

AppVeyorBot commented Sep 25, 2019

livius2 commented Sep 5, 2019 •

edited

livius2 commented Sep 5, 2019 •

edited

livius2 commented Sep 9, 2019 •

edited

livius2 commented Sep 10, 2019 via email •

edited

livius2 commented Sep 10, 2019 via email •

edited

livius2 commented Sep 10, 2019 via email •

edited

livius2 commented Sep 11, 2019 •

edited