microsoft / AL

Home of the Dynamics 365 Business Central AL Language extension for Visual Studio Code. Used to track issues regarding the latest version of the AL compiler and developer tools available in the Visual Studio Code Marketplace or as part of the AL Developer Preview builds for Dynamics 365 Business Central.
MIT License
744 stars 245 forks source link

AL Compiler does not catch Query errors when an option field has been changed to an Enum. #6864

Closed KristjanGaukur closed 2 years ago

KristjanGaukur commented 2 years ago

When you have a query that works on an Option field (f.ex. filtering on the „Document Type“ of a Sales Header), and that field is then changed to an Enum field, compiler does not catch that anything is wrong with the code that is setting a filter to that field in the query, and you get a runtime error.

This was a problem 18 months ago when we did our first upgrade to BC17, and again now in BC19 when more Option Fields were changed to Enum fields, and our queries stopped working.

Here is a picture with an example of a code setting a filter on an Option field, that has been changed to an Enum field:

image

dzzzb commented 2 years ago

What runtime error?

Does it work if you change it to Enum::"Purchase Document Type [or whatever]"::"Order"?

KristjanGaukur commented 2 years ago

Here is a picture of the error: image

By changeing the code to reference the enum values directly corrects the error (but the compiler or codecop do not pick up on this).

So the error line was: QueryVariable.SetFilter(Document_Type, '%1..%2', QueryVariable.Document_Type::Order, QueryVariable.Document_Type::Invoice);

And we corrected it with: QueryVariable.SetFilter(Document_Type, '%1..%2', DocTypeEnumVar::Order, DocTypeEnumVar::Invoice);

dzzzb commented 2 years ago

Since the old Option-type syntax still works for Record.Field::LikeThis, I'd expect it to work too with Query.Field::LikeThis. Hopefully MS agree!

https://github.com/microsoft/AL/issues/6598 is related although that was initially crashing for Options too, and still is for Enums (it seems!)

dzzzb commented 2 years ago

Btw, you don't need to declare a DocTypeEnumVar to reference the enumerated values. You can just do Enum::"The Enum's Name"::"The Value's Name", and in fact the Enum:: prefix is not even required (unless to disambiguate presumably).

dzzzb commented 2 years ago

It seems this may only occur in some cases. For instance, this using a base query is fine:

pageextension 50100 Test extends "Customer List"
{
    trigger OnOpenPage()
    var
        SalesOutstdAmountOnVAT: Query "Sales Outstd. Amount On VAT";
    begin
        SalesOutstdAmountOnVAT.SetFilter(Document_Type, '%1|%2', SalesOutstdAmountOnVAT.Document_Type::Invoice, SalesOutstdAmountOnVAT.Document_Type::"Credit Memo");
        Message('OK!');
    end;
}

Maybe it needs to be a user-defined query, and/or a user-defined field/enum?

KristjanGaukur commented 2 years ago

I have fixed this in my code, so I do not have a problem anymore (unless maybe if MS changes more option fields to enums). So, I just wanted to notify you of the problem. Thank you.

dzzzb commented 2 years ago

I don't work for MS, if that's what you mean.

It does seem like a bug, which should be fixed. Preferably by making the platform not crash on code the compiler says is valid... but at least by, if that syntax is not meant to work, making it error at compile-time.

KristjanGaukur commented 2 years ago

Will MS notice this here, do you think?

dzzzb commented 2 years ago

Yeah, I think so, although it might take a while as there's quite a backlog of issues already. Sometimes newer ones are picked up quicker though.

thloke commented 2 years ago

Hi, sorry for taking a while to get to this - as mentioned, we've got a backlog of issues we're working through. Right now what we need is a minimal repro to take action on this, can you provide a small project that reproduces the issue?

thloke commented 2 years ago

Closing as no response has been given in more than two weeks.

KristjanGaukur commented 2 years ago

Hmmm... strange.

Response was given last Friday from my boss, see attachments.

Cheers, Kristján

From: Thaddeus Loke @.> Sent: mánudagur, 17. janúar 2022 11:23 To: microsoft/AL @.> Cc: Kristján Gaukur Kristjánsson @.>; Author @.> Subject: Re: [microsoft/AL] AL Compiler does not catch Query errors when an option field has been changed to an Enum. (Issue #6864)

Closing as no response has been given in more than two weeks.

- Reply to this email directly, view it on GitHubhttps://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2FAL%2Fissues%2F6864%23issuecomment-1014413593&data=04%7C01%7Cgaukur%40wise.is%7C7f77eca1fc8d4ce8435708d9d9abb10e%7C78817a159e9841fc9d587082228c4b10%7C1%7C0%7C637780153707660272%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=%2FNp4xMJrQAJvYcczOLfmxBSuN1RdV0rw1iIjJpvflkw%3D&reserved=0, or unsubscribehttps://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FANZBLX7JXIETYIPFROGEJ3TUWP3YNANCNFSM5JTOMGLQ&data=04%7C01%7Cgaukur%40wise.is%7C7f77eca1fc8d4ce8435708d9d9abb10e%7C78817a159e9841fc9d587082228c4b10%7C1%7C0%7C637780153707660272%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=HfmjI5jPOt0yvQ5NEhujeIbP9xhNDfO5gU8ri83262c%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7Cgaukur%40wise.is%7C7f77eca1fc8d4ce8435708d9d9abb10e%7C78817a159e9841fc9d587082228c4b10%7C1%7C0%7C637780153707660272%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=10dYDWgW%2BuGHMRnfEl0FWy8c%2B7qVqAROtSz3qZbN5Qg%3D&reserved=0 or Androidhttps://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7Cgaukur%40wise.is%7C7f77eca1fc8d4ce8435708d9d9abb10e%7C78817a159e9841fc9d587082228c4b10%7C1%7C0%7C637780153707660272%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=qju8gAZrJhhojViA0%2BNBmxBgggU07P0kRKfsb5LT4xg%3D&reserved=0. You are receiving this because you authored the thread.Message ID: @.**@.>>

thloke commented 2 years ago

@KristjanGaukur - I think your boss will need to comment on this issue directly rather than replying to the github emails. There's no record of any reply on this issue itself.