Add ability to get execution plan for embedded SQL

[isc-lindensc]

Hi,
we have the language server that helps a lot with formatting and other issue during coding.

There is another code formatting/linting solution currently available codeTidy.

This does more or less the same as the language server, but expands it in some areas e.g. it can add SQL execution plans on save/compile, force commenting style (e.g. replace // with #; etc)

For this to work though codetidy needs to be installed into a namespace and sourcecontrol hooks need to be enabled for the namespace.

Could we look into moving the functionality of codetidy into the language server?

Best Regards
Timo

[gjsjohnmurray]

A disadvantage I see of this approach is that it's likely to hamper the chances of community contribution to its improvement. Currently it is written in ObjectScript and has a public GitHub repo. It's also published on Open Exchange and installable using ZPM.

Moving it into the language server might mean reimplementation in TypeScript of even C. Plus, only part of the LS source is public.

I'd prefer the effort to go into bringing IPM "over the line" so every IRIS instance has it built in.

[isc-bsaviano]

@isc-lindensc There's no way to integrate codetidy to with this extension since it's a server-side technology. I'm not that familiar with its features. Can you list the ones you'd like to see implemented in the Language Server's formatter? I can comment on the feasibility of each.

[isc-bsaviano]

@isc-lindensc I looked at CodeTidy's config options and I didn't see any features I would consider "must-add". Indentation is covered by #293. Converting comments to macro versions isn't terribly difficult, but I wouldn't classify that as high-value. Which specific features do you want to see implemented here?

[isc-lindensc]

Hi Brett, The most useful functionality of codetidy atm is adding the query execution plan as comments to embedded sql queries. Most other features are covered by VScode and/or language server already. From: Brett Saviano ***@***.***> Sent: Friday, 10 November 2023 11:10 PM To: intersystems/language-server ***@***.***> Cc: Timo Lindenschmid ***@***.***>; Mention ***@***.***> Subject: Re: [intersystems/language-server] codetidy (Issue #297) @isc-lindensc<https://github.com/isc-lindensc> I looked at CodeTidy's config options<https://github.com/intersystems/isc-codetidy/blob/main/documentation.md#configuration-options> and I didn't see any features I would consider "must-add". Indentation is covered by #293<#293>. Converting comments to macro versions isn't terribly difficult, but I wouldn't classify that as high-value. Which specific features do you want to see implemented here? — Reply to this email directly, view it on GitHub<#297 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AM3MPOKJUBV6RIV7INM5HQ3YDYVCLAVCNFSM6AAAAAA7AQBDNSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBVG42DKNRUGQ>. You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>

[isc-bsaviano]

@isc-lindensc Thanks for clarifying. I can see the value of having that information, but I don't think it should be part of the formatter. The goal of formatting is make code more readable and reduce style-based source control diffs. I think adding a big block of XML to your document goes against these principles.

[bdeboe]

Hi @isc-lindensc ,

I'm not sure this is a practical place to show the query plan. Query plans are meant to adapt to the actual data distribution (aka Tune Table stats) in the environment where you're running the statement, and to the supplied runtime parameter values in case they'd warrant a separate query plan (aka Run Time Plan Choice). That context is not available as part of the "code" the language server sees. Unless you have completely disabled those features (no hard feelings, but the team worked hard to make them a reality ;-) ) and use /compileembedded, you'd have to add a lot of disclaimers about this being a "probable" plan that's only based on what is statically available.

Given all of the above, I don't think it's a very practical extension, but I'm happy to hear more context so maybe we find a practical way to get you what you want.

thanks,
benjamin

[isc-lindensc]

Hi @bdeboe ,

this extension is based on code used by TC development internally. Its very useful.

Usually what's happening is that either the tune table stats are exported from production and imported into a development environment or deployed code comes with stats predesigned.
It allows the developer to see during development if a sql query designed badly , e.g. doing table scans and not using an index.

But yes agreed developers need to understand that the plan is based on existing statistics.

regards
Timo

[bdeboe]

Hi @isc-lindensc ,

Thanks for the additional context. I can see how this is useful, but the need for proper disclaimers remains. Would something crude in the form of an assist link (similar to "Debug" and "Copy invocation" for methods) that just show the output of EXPLAIN in the new WebTerminal be helpful? That may not be a lot of work and still get you the important feedback, whereas any actual rendering of the query plan or something inline would require a lot more dev effort. This may not be a language server question anymore though.

Thanks,
benjamin

[isc-bsaviano]

Given the number and severity of the disclaimers pointed out by Ben, I don't think this feature is worth adding.