> Generally, providing some structural information in the machine-readable
> form is a good idea. Parsing plain text is terrible, especially when
> this text changes from version to version.
>
> When I was implementing the explained plan output, the original idea was
> to have it represented in both binary form (some kind of byte code) and
> textual one. The former one could be used by external tools for
> automated processing while the latter one is the user-friendly output
> for API and ISQL. This way, alternative format generators could be
> layered similarly to blob filters, even if implemented differently (e.g.
> as built-in functions).
>
> But I never did the binary format. Why? Because nowadays nobody wants to
> parse yet another proprietary format just to get the data. There are
> de-facto standards like XML and JSON for that purposes, with existing
> parsers and the whole infrastructure.
>
> However, choosing XML over JSON or vice versa is IMHO a way nowhere.
> Whatever one we choose, half of users will be against the chosen format
> and ask us to support the second one. So we should either support both
> out of the box or somehow stack custom format generators as plug-ins.
>
> Just to summarize: I do support your work on the XML plan
> representation, but we need to think carefully how it's going to be done.
>
> Now back to your patch. The coding style issue was always mentioned,
> please respect the existing style.
>
> I don't like extending the print() method with type-of-format argument.
> It wasn't really good even for legacy vs explained output and becomes
> even worse for XML and other formats. Initially I was going to suggest
> separate methods instead: printPlan(), explainAsText(), explainAsXml()
> etc. But now I think that custom builder classes would be a better
> approach - they would allow to separate data access methods from the
> printing stuff and simplify pluggability, if required. I will try to
> refactor the existing printing code into classes, so that you could
> follow this approach for your implementation.
>
good points
> OPT_get_plan() may remain "as is" (with format-type specifier argument),
> but I'd rather use enum instead of int.
>
I thought about it at start
but i do not know in which FB file should i declare this enum type?
I do not know all files and their purpose.
> As for the API usage, a better approach would be to use a two-byte tag
> sequence, i.e.:
>
> { isc_info_sql_explain_plan, isc_info_sql_explain_type_xxx }
> with:
> isc_info_sql_explain_type_text 1
> isc_info_sql_explain_type_xml 2
> isc_info_sql_explain_type_json 3
> ...
>
> rather than wasting the primary tag space with
> isc_info_sql_explain_plan_xml, isc_info_sql_explain_plan_json, etc.
i am not so flexible in FB code and i do not know how to support this sub tags?
> Finally, I'm not sure we need additional output formats in the Trace API.
>
I someone chooses that it need e.g. xml - then i think it should have
possibility to use it everywhere
Will be good to have this customizable, i think.
> As for the XML output itself -- tags / attributes / etc -- I don't have
> something useful to comment as I almost never use XML myself ;-) So I
> invite others to continue discussing this part of the patch.
> Dmitry
i tried to follow MSSQL explained plan format
But it will be looking similar - when we will have more detailed info in plan
regards,
Karol Bieniaszewski
------------------------------------------------------------------------------
Firebird-Devel mailing list, web interface at
https://lists.sourceforge.net/lists/listinfo/firebird-devel
