search

codefori / vscode-rpgle

RPGLE language tools for VS Code
MIT License
39 stars 18 forks source link

Lint Option: File Access should be completed using SQL #10

Open priceaj opened 2 years ago

priceaj commented 2 years ago

SETLL, CHAIN READ etc should be flagged and the SQL equilivalents suggested

worksofliam commented 2 years ago

LOVE this one.

@priceaj Would you perhaps be willing to share the SQL on this issue?

e.g.

bmorris1 commented 2 years ago

Usually it's not a great idea to just replace each RPG statement with a matching SQL statement. That can lead to terrible performance. But it would be super complex (if it's even possible to do programmatically) to suggest how to do the all the I/O for some file with one or more SQL statements.

priceaj commented 2 years ago

Hi @bmorris1

I'm quite surprised to hear someone from the RPG development team at IBM mentioning performance issues between SQL and RLA, especially when the "modernization" content I have read coming from IBM seems to be pushing people towards embedded SQL and away from RLA ("Modernizing IBM i Applications from the Database up to the User Interface and Everything in Between", springs to mind!)

Is it the blind programmatic conversion that you would be concerned about or the concept of moving completely away from RLA?

Edit:

just to elaborate on the above, are you thinking of scenarios such as having to chain through an arbitrary number of files to get a piece of data?

  1. If the chains were blindly replaced with select * into :Xds from fileX where z = :key then this would obviously be bad, because of the overhead of the SQL APIs, and also fetching too much data (as far as I know chain only fetches the fields used in the program, is that correct?)

Or 2. are you saying if this was intelligently rewritten, e.g. select a into :a from filex join Filey on :xdata = :ykey join filez on :ydata = :zkey then there would still be a performance hit?

If 1 then could we just have the rule which highlights the relevent keywords but just says "don't do this" or "consider rewriting using a select/cursor" rather then having a "Quick fix". Bearing in mind this will be a user selectable choice to have it enabled.

If 2 then I need to go away and rewrite pretty much every RPG program I have written in the last 5 years (at least)!

bmorris1 commented 2 years ago

Yes, it's the blind conversions of each opcode to an SQL equivalent. I was disagreeing with the idea that the linter would suggest an equivalent SQL operation for each RPG operation.

If people intelligently change what the RPG logic is doing to do the same thing in SQL in a set-based way, then the SQL performance might be much much better than the RPG could ever be.

priceaj commented 2 years ago

@bmorris1 phew! Thanks for confirming!!

worksofliam commented 2 years ago

@priceaj With all that being said, how do you see this working now?

priceaj commented 2 years ago

So we are still making the linter Unopinionated Right? (As in this won't be enabled by default)

Just have it highlight when the relevant RLA op codes are used, you don't have to provide a quick fix for everything. Most linters don't they will just say, this is wrong, fix it.

It's the choice of the user to enable it!

worksofliam commented 2 years ago

@priceaj Right! There is a default configuration that is created when people enable it: https://github.com/halcyon-tech/vscode-rpgle/blob/main/src/models/default.js

Maybe this is similar to #11?