Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dataflow error in a default of a type-ascribed argument results in a type error instead of being propagated #7137

Closed
radeusgd opened this issue Jun 27, 2023 · 4 comments
Assignees
Labels

Comments

@radeusgd
Copy link
Member

radeusgd commented Jun 27, 2023

There seems to be some issue when a default argument value returns a dataflow error and that argument has a type ascription resulting in a typecheck. Weirdly, it is not happening always. Below I present a repro where it consistently (wrongly) fails with a type error, but later I also show attempted repros where it actually works as expected.

Run the following repro (it assumes code from #7072 - which is waiting to be merged as of this moment):

from Standard.Base import all
import Standard.Base.Runtime.Context

from Standard.Table import Table, Value_Type

from Standard.Database import Database, SQLite, In_Memory, Column_Description, Update_Action
from Standard.Database.Internal.Upload_Table import default_key_columns

main =
    c = Database.connect (SQLite In_Memory)
    Context.Output.with_disabled <|
        t = c.create_table "table123" [Column_Description.Value "Y" Value_Type.Char] primary_key=Nothing

        r0 = default_key_columns t
        IO.println r0

        src = Table.new [["Y", ["d", "e", "f"]]]
        r1 = src.update_database_table t Update_Action.Insert
        IO.println r1

Expected behaviour

It should print the same dataflow error twice (in the r1 call, the dataflow error from the default value should get propagated to the return value).

(Error: (Illegal_Argument.Error 'Could not determine the primary key for table enso-dry-run-table123. Please provide it explicitly.' Nothing))
(Error: (Illegal_Argument.Error 'Could not determine the primary key for table enso-dry-run-table123. Please provide it explicitly.' Nothing))

Actual behaviour

Instead, in the update_database_table case, it results in a type error:

(Error: (Illegal_Argument.Error 'Could not determine the primary key for table enso-dry-run-table123. Please provide it explicitly.' Nothing))
Execution finished with an error: Type error: expected `key_columns` to be Vector | Nothing, but got Error.
        at <enso> Table.update_database_table(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Database\0.0.0-dev\src\Extensions\Upload_In_Memory_Table.enso:123-124)
        at <enso> weird_type_error.main<arg-1>(weird_type_error.enso:18:14-61)
        at <enso> Runtime.with_disabled_context(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\Runtime.enso:228:5-81)
        at <enso> Context.with_disabled(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\Runtime.enso:181:9-79)
        at <enso> weird_type_error.main(weird_type_error.enso:11-19)

It looks like the type check introduced by the ascription key_columns : Vector | Nothing somehow ignores the fact that the argument is a dataflow error to be propagated and sees that an 'Error' does not (obviously) fit the expected type.

I have spent some time trying to figure out a simpler example, but failed to do so. This bug seems to trigger under some specific conditions. But the repro above works reliably for me (100% replayability).

Below is a repro that didn't work (i.e. it actually works as expected):

Details
from Standard.Base import all
import Standard.Base.Errors.Illegal_State.Illegal_State

default_value t =
    if t == "should error" then Error.throw (Illegal_State.Error "error in defaults") else ["X", "Y"]

foo (t : Text) (x : Vector | Nothing = default_value t) =
    t + " - " + x.to_text

type Bar
    Value y

Bar.baz : Text -> Vector | Nothing -> Text
Bar.baz self (t : Text) (x : Vector | Nothing = default_value t) =
    t + " - " + self.y.to_text + " - " + x.to_text

main =
    #IO.println (foo "X")
    #IO.println (foo "should error")

    b = Bar.Value 42
    #IO.println (b.baz "X")
    IO.println (b.baz "should error")

and another one

from Standard.Base import all
import Standard.Base.Errors.Illegal_State.Illegal_State

foo_unchecked x =
    10 + x + 100

foo_checked (x : Integer) =
    10 + x + 100

compute_default x =
    if x == 10 then Error.throw (Illegal_State.Error "ISE: from defaults") else x*10

bar_unchecked x (y = compute_default x) =
    x + y

bar_checked (x : Integer) (y : Integer = compute_default x) =
    x + y

main =
    err = Error.throw (Illegal_State.Error "ISE: error message")

    r1 = foo_unchecked err
    IO.println r1

    r2 = foo_checked err
    IO.println r2

    r3 = bar_unchecked 1
    IO.println r3

    r4 = bar_checked 1
    IO.println r4

    r5 = bar_unchecked 10
    IO.println r5

    r6 = bar_checked 10
    IO.println r6

So the bug only seems to occur under some specific conditions, that I could not track down yet.

@radeusgd radeusgd changed the title Dataflow results in a type error instead of being propagated Dataflow error in a default of a type-ascribed argument results in a type error instead of being propagated Jun 27, 2023
@Akirathan Akirathan removed the triage label Jun 28, 2023
@jdunkerley jdunkerley moved this from ❓New to 📤 Backlog in Issues Board Jul 4, 2023
@JaroslavTulach
Copy link
Member

I've just executed:

enso$ git log | head -n 2
commit b243a5f5292bf01c810cd37ecd2f04ee751c1601
Author: somebody1234 <[email protected]>
enso$ cat >test.enso
from Standard.Base import all
import Standard.Base.Runtime.Context

from Standard.Table import Table, Value_Type

from Standard.Database import Database, SQLite, In_Memory, Column_Description, Update_Action
from Standard.Database.Internal.Upload_Table import default_key_columns

main =
    c = Database.connect (SQLite In_Memory)
    Context.Output.with_disabled <|
        t = c.create_table "table123" [Column_Description.Value "Y" Value_Type.Char] primary_key=Nothing

        r0 = default_key_columns t
        IO.println r0

        src = Table.new [["Y", ["d", "e", "f"]]]
        r1 = src.update_database_table t Update_Action.Insert
        IO.println r1
^D
enso$ sbt
sbt:enso> runEngineDistribution --run test.enso
Nothing
(Database Table enso-dry-run-table123)

e.g. I don't see any error.

@github-project-automation github-project-automation bot moved this from 📤 Backlog to 🟢 Accepted in Issues Board Jul 5, 2023
@enso-bot
Copy link

enso-bot bot commented Jul 6, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-07-05):

Progress: - reply to emails

Next Day: More vacationing

@enso-bot
Copy link

enso-bot bot commented Jul 9, 2023

Jaroslav Tulach reports a new STANDUP for the last Friday (2023-07-07):

Progress: - rewriting read argument node type check:

Next Day: Polish #7009 to be ready for review

@enso-bot
Copy link

enso-bot bot commented Jul 10, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-07-09):

Progress: - PR-7009 is ready for review: #7009

Next Day: Benchmarking & travel

GitHub
Pull Request Description There are a lot of Enso-only benchmarks in test/Benchmarks project. These benchmarks are not run on the CI (they are, but only in dry-run). We want to run these benchmarks ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

3 participants