joyfullservice / msaccess-vcs-addin

Synchronize your Access Forms, Macros, Modules, Queries, Reports, and more with a version control system.
Other
215 stars 40 forks source link

For complex SQL queries, use separate *.sql file instead of dbMemo SQL variable in *.bas #336

Open grv87 opened 2 years ago

grv87 commented 2 years ago

Complex queries have *.bas file starting with dbMemo "SQL" line. SQL code in this line can't be easily formatted, diffed or edited. It hinders the purpose of version control.

I can also export, reformat and edit *.sql file. But this file won't be used in building from source unless I copy it back to *.bas. Also, storing the second copy of SQL query would violate DRY.

I propose that for complex queries (with dbMemo "SQL"…) the tool:

hecon5 commented 2 years ago

@grv87, have you checked Wiki Options Build Options

See "Force import of original SQL for queries". It does exactly what you are asking. The *.bas is still exported, as for queries built in Access with the query designer, there are many layout and visual styles stored in that file.

As to parsing and formatting the *.bas file...that becomes rather complicated rather quickly. See #184 and #185 for some notes I started on that process. As you can tell, it isn't as cut and dried as "delete the lines", because that corrupts the import rather badly.

Using the SQL import force option will import the query, and then set the SQL to the SQL file; there isn't really a great way to do it without building a whole new sanitize and parse and build routine (which I'm working on...slowly).

grv87 commented 2 years ago

@hecon5, thanks. But this option doesn't solve the problem in general. With this option on, I have to view/diff/edit *.sql for simple queries whereas I'd like to use *.bas for them.

I understand the difficulty of editing *.bas. Maybe for temporary solution no editing is necessary? Keep these lines with SQL in *.bas, but ignore them/replace on import.

hecon5 commented 2 years ago

Ok, to start off, the issue isn't that this tool can't, it's that to do this requires:

  1. Building an intelligent and dynamic Sanitize which can accurately parse exported VBA line by line. This decides what to keep based on various settings and configurations.
  2. Build (essentially) all-new Query export and build parse tools (this is separate from the sanitize routines; this is the file parser itself, the Sanitize is the "chooser" and the query file parser just says what's in there). As-is we can just export the file and don't process it except convert to UTF-8. Therefore, we would need to read and understand the file structure, and (more importantly) re-build the file structure correctly.
  3. It will have significant impacts to established user's code base as we'd be stripping lines that previously existed.
  4. We'd need to deal with collisions (what happens if the dbMemo "SQL" = is already there because a user reverted a change, is that the correct one or not). Presently, we just ignore it because the .SQL file is used to set the SQL property after import, that may not be possible any more.
  5. We would need to re-build the .bas file upon import (if you import without the dbMemo "SQL" = line the import will fail).

All that may take considerable processing time and resources for a tool that is designed to be light and fast, and does 99% of what you're looking for now.

So, it may not be ideal, but given Force Import of SQL already exists and does (I think?) exactly what you're asking and works. The only thing missing from what you're asking as best I can tell is the tool wouldn't export the SQL into the .bas file and the .SQL file. Given importing a query without the SQL line in it causes the whole thing to be skipped and fail, the occasional code noise may be worth it, IMO.

Can you help me understand the following statements?

removes this line from *.bas file, maybe replaces it with some comment/placeholder.

With this option on, I have to view/diff/edit .sql for simple queries whereas I'd like to use .bas for them.

These two statements seem (to me?) to be at odds with each other. Either you're looking to export the SQL and edit the SQL directly (and force import of the SQL) or you want to edit in the .bas file (which, agreed, is difficult given the intricacies of the file structure).

While we do some fancy Sanitizing in the Form and Report export/import for PrtMip, it isn't as straightforward for Query exports because the file structure is different (see above links to issues where I delve into the intricacies), for long and verbose SQL there could be numerous lines, and lastly failing to build the SQL correctly will cause file import failure, the risk/reward balance is heavily in the risk category and "costs" only a bit of code noise.

Lastly, are you editing the Query in the query editor, or are you editing in a text file and then importing? For the vast majority of cases, it's much better to edit in Access itself, export, and then build from the exported code, and not directly edit the Query.Bas files directly because of all the above.

In closing, I'm trying to build a file parser for a separate setting (Connection link), but as I said above, it's not straightforward and the risks are high, the rewards are low, and so the progress is very slow.

grv87 commented 2 years ago

Can you help me understand the following statements?

These statements don't contradict. I'd like to use a file format best appropriate for each kind of code.

So, 'Force Import of SQL' doesn't do what I ask. What I ask is a middle between this option turned on and turned off.

are you editing the Query in the query editor, or are you editing in a text file and then importing?

Right now, I edit simple queries in Design mode. And I try to edit complex queries in SQL mode. But since Access doesn't have normal SQL editor, I usually open SQL mode in Access, copy query code, paste it into Visual Studio Code, format it there, edit, and copy-paste back to Access. I got tired of this copy-paste action, so, for complex queries, I'd like to switch to editing files and importing them.

One of my projects have two instances (DBs with identical structure but different data). I used to copy queries from one to another and backwards, but at some point they anyway diverged. In order to fix this drift, I'd like to export all their code to files, manually compare and merge them (with text editor) and import back.

@hecon5, do you see any technical difficulties with proposed temporary solution? It doesn't require a parser.

hecon5 commented 2 years ago

@hecon5, do you see any technical difficulties with https://github.com/joyfullservice/msaccess-vcs-integration/issues/336#issuecomment-1142720473? It doesn't require a parser.

That's a parser by definition, as far as I can tell... If that line isn't in the .bas file, it'll fail on import, in my experience. If a user deletes the .sql file (such as users with established code bases who revert to a clean slate prior to building), this will cause building to not work.

I think the best way to deal with this is to either live with it, or we could build a parser (which, admittedly would have other benefits as we could parse out connections and the like).

See links above for related issues, but if this is something you're really after, I think a parser is going to be the way to go.

grv87 commented 2 years ago

A user who uses this feature obviously should not delete SQL file.

If *.bas starts with dbMemo "SQL" =

You don't need a parser to compare that one string starts with another.

Never mind. If it'll become a serious issue to me, I'll make a PR with what I propose.

joyfullservice commented 2 years ago

@grv87 - I do like the idea of extracting out the embedded SQL from dbMemo "SQL" =" and substituting it with a placeholder. This would put all the SQL changes in a single location in the source code, instead of being mirrored in both the .bas and .sql files. I might suggest using a comment placeholder for the SQL filename. Although Access SQL does not support comments, it would be intuitively obvious for anyone with general familiarity with SQL.

dbMemo "SQL" ="--qryMySampleQuery.sql"

Splitting out the embedded SQL statement would also have a second benefit. As a future enhancement, I would also like to format the SQL file content with some basic line breaks and indenting so that every query is exported in a consistent and easily readable format. This will make diffing SQL changes in queries a breeze in the native VCS tools, instead of having to format and compare the SQL manually. I made some headway on this project a couple years ago, but never finished it. (If anyone is interested in working on this, I was adapting https://github.com/zeroturnaround/sql-formatter to a single VBA class for use in this add-in and potentially other projects.)

I can see how managing the SQL in the .sql file and merging it into the .bas file during import would allow development work to happen directly on the .sql file and streamline the workflow in some scenarios.

hecon5 commented 2 years ago

To make sure @grv87 and @joyfullservice are clear...I like the idea of moving SQL out of the .bas file. I sincerely apologize for the misconception.

My chief concern is changing this is a breaking change (once you convert this way, going in reverse on your code base is not going to be possible), and users should be aware it's about to happen. I also think we should have some robust build error detection to ensure we don't forget to put the SQL back in (and so users are aware of what just happened so they can fix it).

If we do this (which, I think is a good idea, clearly that's not been conveyed, and that's on me), we should start out with this being an option, and off by default. We should advertise this, but I don't think we should strip out the SQL field automatically.

Secondly, the more I think of it...I think we should build at least a rudimentary parser. Being able to sanitize the connection string and the SQL would be heaven, and would allow me to get rid of the last vestiges of PII / server secrets in the Queries from my code base.

Being able to remove the Connect and SQL info from the .bas file would make the code base more human readable, which, IMO is a good thing. This would actually be a decent test case for the updated FitStringToColumn and FitStringToWidth tools to build the resultant lines....

If we did this, we could potentially save the SQL in .SQL, and the Connection in a JSON file (after sanitizing), and reinsert just before compiling.

I'm almost wondering if we could have one .JSON properties file to list the Queries with the same connection string, and add their name as a member of a dictionary when it matches. This would basically allow us to quickly see if the query has a relevant connection via If dQueryProperties("Connect").Exists or similar.

Then, prior to rebuilding, you could go through the QueryProperties.JSON and see if they are correct, moving entries around as needed.