joyfullservice / msaccess-vcs-addin

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

Issue when using subqueries in JOINs #76

Closed Indigo744 closed 3 years ago

Indigo744 commented 3 years ago

Hello,

Alright so this is more of an Access issue than this tool, but it's something that we should be aware.

Access can't seem to get the export/import right when the query performs a JOIN on a subquery.

For instance, simply create a new query TestQuery and use this SQL statement:

SELECT SubQuery.ObjectId
FROM MSysACEs 
LEFT JOIN (SELECT MSysACEs.* FROM MSysACEs) AS SubQuery ON MSysACEs.ACM = SubQuery.ACM;

Go to the VBE, and export it:

Application.SaveAsText acQuery, "TestQuery", "Test.bas"

Then delete it and import it:

Application.LoadFromText acQuery, "TestQuery", "Test.bas"

Try to run the query, you should be greeted by an error message: image

It seems that it think the subquery should be a table name, search for it, obviously not finding it and report the error.

Modifying slightly the query and saving it again will make it work.

It is clearly an issue with Access and LoadFromText, but I wonder if we could detect these subqueries and show a warning or something?

Thanks.

Indigo744 commented 3 years ago

After some more testing, there is something interesting.

I'm looking at the SQL statement produced by Access with the following piece of code in VBE:

?CurrentDb.QueryDefs("TestQuery").SQL

Before loading with LoadFromText, the SQL query is:

SELECT SubQuery.ObjectId
FROM MSysACEs LEFT JOIN (SELECT MSysACEs.* FROM MSysACEs)  AS SubQuery ON MSysACEs.ACM = SubQuery.ACM;

After loading with LoadFromText, the SQL query becomes:

SELECT SubQuery.ObjectId
FROM MSysACEs LEFT JOIN [SELECT MSysACEs.* FROM MSysACEs]  AS SubQuery ON MSysACEs.ACM = SubQuery.ACM;

Notice the brackets [...] around the SELECT subquery statement where it should be parenthesis.

It explains why Access fails because it looks for a table litterally called [SELECT MSysACEs.* FROM MSysACEs] instead of using it as subquery.

Now I wonder: could we try to identify a subquery, and replace the brackets with parenthesis when importing?

joyfullservice commented 3 years ago

Let me test this on my end to see if I get the same behavior...

joyfullservice commented 3 years ago

It works just fine on my system. It does not change the parentheses to brackets when exporting and importing. I am using Microsoft Access 2010 (English). I will be curious to learn what you are able to find out on this.

Indigo744 commented 3 years ago

That is surprising, but I'm using Access 2016 (French) so maybe it comes from either the new version or the language?

Sadly I don't have another version on hand to try it out...

joyfullservice commented 3 years ago

I may be able to borrow another computer at the office tomorrow that has Access 2016 on it... That might help us narrow down the issue.

joyfullservice commented 3 years ago

I was able to reproduce this issue on the English version of Access 2016, so I don't think this is specific to the French version. The output files generated by the SaveAsText function also look significantly different.

Access 2010

dbMemo "SQL" ="SELECT SubQuery.ObjectId\015\012FROM MSysACEs LEFT JOIN (SELECT MSysACEs.* FROM "
    "MSysACEs)  AS SubQuery ON MSysACEs.ACM = SubQuery.ACM;\015\012"
dbMemo "Connect" =""
dbBoolean "ReturnsRecords" ="-1"
dbInteger "ODBCTimeout" ="60"
dbBoolean "OrderByOn" ="0"
dbByte "Orientation" ="0"
dbByte "DefaultView" ="2"
dbBinary "GUID" = Begin
    0x0a37d332269ccb4e8e87e9a2b7bc6517
End
dbBoolean "FilterOnLoad" ="0"
dbBoolean "OrderByOnLoad" ="-1"
dbLongBinary "DOL" = Begin
    0x0acc0e55000000000f54a1be685f0044b2feb103aa9bf06e00000000ae7397e5 ,
    0xed8be54000000000000000004d00530079007300410043004500730000000000 ,
    0x0000000000000000000000000000000000000000000000000000000000000000 ,
    0x00000000000053007500620051007500650072007900000000000000f239590e ,
    0x63fa694c8c33386a5a77b166070000000f54a1be685f0044b2feb103aa9bf06e ,
    0x410043004d00000000000000000000000000000000000000000000000c000000 ,
    0x050000000000000000000000000000000000
End
dbByte "PublishToWeb" ="1"
Begin
    Begin
        dbText "Name" ="SubQuery.ObjectId"
        dbLong "AggregateType" ="-1"
        dbBinary "GUID" = Begin
            0x6e3958b0b37aaf4fb3c43ae7df4646b9
        End
    End
End

Access 2016

Operation =1
Option =0
Begin InputTables
    Name ="MSysACEs"
    Name ="SELECT MSysACEs.* FROM MSysACEs"
    Alias ="SubQuery"
End
Begin OutputColumns
    Expression ="SubQuery.ObjectId"
End
Begin Joins
    LeftTable ="MSysACEs"
    RightTable ="SubQuery"
    Expression ="MSysACEs.ACM = SubQuery.ACM"
    Flag =2
End
dbBoolean "ReturnsRecords" ="-1"
dbInteger "ODBCTimeout" ="60"
dbByte "RecordsetType" ="0"
dbBoolean "OrderByOn" ="0"
dbByte "Orientation" ="0"
dbByte "DefaultView" ="2"
dbBinary "GUID" = Begin
    0x7e01850ab2416142b24f7b5e75de12ed
End
dbBoolean "FilterOnLoad" ="0"
dbBoolean "OrderByOnLoad" ="-1"
dbBoolean "TotalsRow" ="0"
dbLongBinary "DOL" = Begin
    0x0acc0e55000000004208b296f9f8c443bd3082d10552101100000000c55f4529 ,
    0xed8be54000000000000000004d00530079007300410043004500730000000000 ,
    0x0000000000000000000000000000000000000000000000000000000000000000 ,
    0x000000000000530075006200510075006500720079000000000000009693ec49 ,
    0x79c91e41ac23fbfc31d58a52070000004208b296f9f8c443bd3082d105521011 ,
    0x410043004d00000000000000000000000000000000000000000000000c000000 ,
    0x050000000000000000000000000000000000
End
dbByte "PublishToWeb" ="1"
Begin
    Begin
        dbText "Name" ="SubQuery.ObjectId"
        dbLong "AggregateType" ="-1"
    End
End

Surprisingly, the 2016 file imports just fine into the Access 2010 database, and the query even runs just fine after import. Outputting the SQL with ?CurrentDb.QueryDefs("TestQuery").SQL shows the brackets at first (even thought the query runs) but after a modification to the query, it switches to the parentheses.

Indigo744 commented 3 years ago

MS internal shenanigans I guess!

In any case, what about trying to fix the offending SQL by replacing brackets to parentheses for these subqueries? I have a minimal working code to do this, so I can come up quickly with a PR.

It could even be used behind a configuration flag "Try to fix subqueries after import", deactivated by default. That way, we are sure we won't break anything.

Indigo744 commented 3 years ago

I have found something interesting.

1st : create a new query without touching the designer

Create a new query, go to the SQL table, paste the SQL value and save (do not touch the visual designer or anything). Then perform SaveAsText.

Resulting file uses dbMemo format (click to expand) ``` dbMemo "SQL" ="SELECT SubQuery.*\015\012FROM MSysACEs LEFT JOIN (SELECT * FROM MSysACEs) AS Su" "bQuery ON MSysACEs.ACM = SubQuery.ACM;\015\012" dbMemo "Connect" ="" dbBoolean "ReturnsRecords" ="-1" dbInteger "ODBCTimeout" ="60" dbBoolean "OrderByOn" ="0" dbByte "Orientation" ="0" dbByte "DefaultView" ="2" dbBinary "GUID" = Begin 0xa89a082ce5e7264ab0878cd0faed31e3 End dbBoolean "FilterOnLoad" ="0" dbBoolean "OrderByOnLoad" ="-1" dbLongBinary "DOL" = Begin 0x0acc0e550000000087a7edc3e91dfc4a9b4135c5117d81fc00000000845bdb7a , 0xdc8be54000000000000000004d00530079007300410043004500730000000000 , 0x0000000000000000000000000000000000000000000000000000000000000000 , 0x00000000000053007500620051007500650072007900000000000000ff62df07 , 0x0a56af41bb89e7e2814fca3f0700000087a7edc3e91dfc4a9b4135c5117d81fc , 0x410043004d00000000000000000000000000000000000000000000000c000000 , 0x050000000000000000000000000000000000 End dbByte "PublishToWeb" ="1" Begin End ```

Most important thing is: importing it works without the bracket issue!

2nd : modify the query with the designer

Go back to the query, go to the visual designer, move around a table and save. Then perform SaveAsText.

Resulting file uses the other format (click to expand) ``` Operation =1 Option =0 Begin InputTables Name ="MSysACEs" Name ="SELECT * FROM MSysACEs" Alias ="SubQuery" End Begin OutputColumns Expression ="SubQuery.*" End Begin Joins LeftTable ="MSysACEs" RightTable ="SubQuery" Expression ="MSysACEs.ACM = SubQuery.ACM" Flag =2 End dbBoolean "ReturnsRecords" ="-1" dbInteger "ODBCTimeout" ="60" dbBoolean "OrderByOn" ="0" dbByte "Orientation" ="0" dbByte "DefaultView" ="2" dbBinary "GUID" = Begin 0xa89a082ce5e7264ab0878cd0faed31e3 End dbBoolean "FilterOnLoad" ="0" dbBoolean "OrderByOnLoad" ="-1" dbLongBinary "DOL" = Begin 0x0acc0e550000000087a7edc3e91dfc4a9b4135c5117d81fc00000000845bdb7a , 0xdc8be54000000000000000004d00530079007300410043004500730000000000 , 0x0000000000000000000000000000000000000000000000000000000000000000 , 0x00000000000053007500620051007500650072007900000000000000ff62df07 , 0x0a56af41bb89e7e2814fca3f0700000087a7edc3e91dfc4a9b4135c5117d81fc , 0x410043004d00000000000000000000000000000000000000000000000c000000 , 0x050000000000000000000000000000000000 End dbByte "PublishToWeb" ="1" dbByte "RecordsetType" ="0" dbBoolean "TotalsRow" ="0" Begin Begin dbText "Name" ="SubQuery.MSysACEs.ACM" dbLong "AggregateType" ="-1" End Begin dbText "Name" ="SubQuery.MSysACEs.FInheritable" dbLong "AggregateType" ="-1" End Begin dbText "Name" ="SubQuery.MSysACEs.ObjectId" dbLong "AggregateType" ="-1" End Begin dbText "Name" ="SubQuery.MSysACEs.SID" dbLong "AggregateType" ="-1" End End Begin State =0 Left =0 Top =0 Right =1370 Bottom =842 Left =-1 Top =-1 Right =1354 Bottom =271 Left =0 Top =0 ColumnsShown =539 Begin Left =48 Top =12 Right =192 Bottom =156 Top =0 Name ="MSysACEs" Name ="" End Begin Left =275 Top =40 Right =419 Bottom =184 Top =0 Name ="SubQuery" Name ="" End End ```

Importing it fails due to the bracket issue...

3rd: create a new query from scratch

Create a new working query like the first one, then:

CurrentDb.CreateQueryDef "Temp", CurrentDb.QueryDefs("TestQuery").SQL
Application.SaveAsText acQuery, "Temp", "Test.bas"
Resulting file uses dbMemo format and is surprisingly small (click to expand) ``` dbMemo "SQL" ="SELECT SubQuery.*\015\012FROM MSysACEs LEFT JOIN (SELECT * FROM MSysACEs) AS Su" "bQuery ON MSysACEs.ACM = SubQuery.ACM;\015\012" dbMemo "Connect" ="" dbBoolean "ReturnsRecords" ="-1" dbInteger "ODBCTimeout" ="60" Begin End ```

Summary

So to sum up:

  1. Creating a query with only SQL (either manually or by VBA): the simpler format dbMemo is used for export and importing it will works for subqueries
  2. Creating/modifying a query with the visual designer: the full format is used for export and importing it will create a bad query if a subquery is present
joyfullservice commented 3 years ago

Interesting... I found that I can actually reproduce the issue in Microsoft Access 2010! The key was that I had to adjust the layout in the designer. Another interesting note was that the format of the output file was virtually identical to the 2016 version. (Which would also explain why Access 2010 was able to import the file generated from Access 2016.)

I am hesitant to replace brackets with parentheses for a couple reasons. First, brackets are legitimately used for several purposes within queries, such as prompting for input and around table/field names that are not compatible with SQL syntax. (Such as names that include spaces, or use reserved words.) Second, it would probably be pretty difficult to come up with a reliable way to determine which brackets need to be replaced with parentheses, especially in cases where the rebuilt SQL looks like this: FROM MSysACEs LEFT JOIN [SELECT MSysACEs].[* FROM MSysACEs] AS SubQuery ON MSysACEs.ACM = SubQuery.ACM;

A better approach might be to import the export .sql version of the query directly into the .SQL property of a new QueryDef. This would potentially miss some information on the query layout and other metadata from the designer, but it should work around the problem with the subqueries. It would be simple enough to add a build option where you could import from the .sql files instead of the *.bas files. I guess the question then would be whether this option would be on/off for all queries, or only used for the queries that experience this issue. Would we add a UI component to change the import type for specific queries, or leave that to the developer to manually add entries to the vcs-options.json file. What do you think?

Indigo744 commented 3 years ago

I agree with you. String replacement in query would be very brittle with a lot of potential edge cases.

I like your new proposition. Could we go even further and maybe import the .bas as before, but actively replace the resulting QueryDef.SQL property with the one from .sql file? That way, I think other properties would be preserved and SQL query always correct?

joyfullservice commented 3 years ago

Could we go even further and maybe import the .bas as before, but actively replace the resulting QueryDef.SQL property with the one from .sql file?

That's an interesting idea... I had not thought of doing that, but it might prove the very best case scenario if it preserved both the layout and the original SQL. I suppose we could just do it across the board for all queries since the overhead would be fairly small compared with the risk of corrupting queries that contain subqueries.

What would you think about an option that would turn on this feature for those that encounter this issue? I have never seen it with any of the databases I have worked on, but we can see how things go in the months to come and perhaps make the option on by default if the benefit seems to outweigh the risk.

Indigo744 commented 3 years ago

I agree, we can start with a flag deactivated by default. I'll activate it and use it extensively for my export/import and we'll see if it's stable (I have around 300 queries).

Note that I could simply rewrite the only query that happen to have a subquery, but resolving the issue globally would be better as someone else could face this issue and not be able to migrate from subqueries 😺

Indigo744 commented 3 years ago

I'll try to start working on a PR if you want 😉

joyfullservice commented 3 years ago

Great work on a solid solution for this issue! You did an excellent job on the pull request. It made it really easy to review and merge the changes into the core. 👍 I did make a few tweaks, which I will describe in some inline notes on bf337b3.

Indigo744 commented 3 years ago

Thank you! Good improvements for sure 👍

I've taken the liberty to update the Wiki page using your nice explanation: https://github.com/joyfullservice/msaccess-vcs-integration/wiki/Documentation

joyfullservice commented 3 years ago

Thanks! I just updated the documentation screenshot as well.