Skip to content

Duplicate module definitions silently discard the earlier declaration #6166

Open

Description

@prql-bot

A module declared twice at the same level compiles clean and keeps only the last block — the first one's contents are gone with no diagnostic. Same for an enum (or any other declaration) followed by a module of that name.

module m { let a = 5 }
module m { let b = 6 }
from t | filter x == m.a
Error:
   ╭─[ :3:22 ]
   │
 3 │ from t | filter x == m.a
   │                      ─┬─
   │                       ╰─── Unknown name `m.a`
───╯

m.b resolves fine, so the second block replaced the first rather than merging with it. The reverse of the enum case behaves the same way — enum m { Paid = 0 } followed by module m { let a = 5 } silently drops the enum, while module then enum correctly errors with duplicate declarations of m after #6164.

The cause is the one #6164 fixes for type/enum defs: fold_module_def_stmt calls Module::insert, which overwrites the entry, rather than RootModule::declare, which reports a duplicate. fold_import_def_stmt has the same shape and is covered by #6150.

Only source-level duplicates reach this path — multi-file projects are safe, since insert_stmts_at_path merges a file's statements into an existing module-def stmt of the same name before the resolver runs.

Why the one-line swap to declare doesn't work as-is

Swapping the insert for declare(ident, kind, stmt.id, stmt.annotations) fails 31 lib tests with duplicate declarations of std: Module::new_root pre-seeds a std entry, so the module def that load_std_lib injects collides with the placeholder. A fix needs to let a module def fill a pre-seeded placeholder (and possibly merge into an existing module) while still rejecting genuine source-level duplicates — which is why this is a separate change from #6164 rather than part of it.

Activity

  1. prql-bot commented on Aug 8, 2026

    @prql-bot
    CollaboratorAuthor

    Reproduced on e32f9fe7 — all three shapes in the report behave as described. Deliberately not opening a fix PR, because the direction here is already an open question in front of @kgutwin from #6147 rather than an unclaimed bug: that PR fixed this exact call site by merging, was reviewed as "implicit merging of namespaces is a design smell", and closed. The direction described there instead — "we should be giving them error messages, not squishy merge behavior" — was offered as a fresh PR and left as a maintainer's call, so this issue is waiting on that call, not on an implementation. Flagging it here so a later triage pass doesn't open a third PR on the same behavior.

    Three things this run verified that weren't on either thread yet:

    It isn't confined to a single file. A directory project fails today, with no repeated block anywhere in the source — a/b.prql containing let y = 2, plus a root Main.prql that declares module a { let x = 1 }, gives Unknown name a.b.y. The file tree's a and the root file's a are two sibling ModuleDef stmts by the time the resolver runs, and the second wins.

    prqlc collect output no longer compiles. collect flattens that project to a repeated module a { … } pair, which is exactly the form that loses declarations, so the compiler can't read its own serializer's output — silently lossy today, and a hard error under the error direction. That's a collect bug whichever way the semantics land.

    The error direction is not a one-line swap. Confirmed the note in the report: replacing the insert in fold_module_def_stmt with RootModule::declare fails 31 prqlc lib tests with duplicate declarations of std, because Module::new_root pre-seeds a std entry that load_std_lib's own module def then collides with. Any fix has to let a module def fill a pre-seeded placeholder while still rejecting source-level duplicates.

    #6164 covers the half where a module is the second declaration; this issue is the other half plus module/module.

    Reproduction transcript

    Built at e32f9fe7.

    Single file — the first block's contents are gone, the second block's resolve:

    $ cat dup1.prql
    module m { let a = 5 }
    module m { let b = 6 }
    from t | filter x == m.a
    
    $ prqlc compile dup1.prql
    Error:
       ╭─[ :3:22 ]
       │
     3 │ from t | filter x == m.a
       │                      ─┬─
       │                       ╰─── Unknown name `m.a`
       │
       │ Help: available columns: t.x
    ───╯
    

    Swapping m.a for m.b compiles to WHERE x = 6. enum m { Paid = 0 } followed by module m { let a = 5 } compiles to WHERE x = 5 — the enum is silently dropped.

    Multi-file — a/b.prql is let y = 2, Main.prql is:

    module a {
      let x = 1
    }
    from t
    derive {p = a.x, q = a.b.y}
    
    $ prqlc compile collecttest
    Error:
       ╭─[ Main.prql:5:22 ]
       │
     5 │ derive {p = a.x, q = a.b.y}
       │                      ──┬──
       │                        ╰──── Unknown name `a.b.y`
    ───╯
    
    $ prqlc collect collecttest
    module a {
      module b {
        let y = 2
      }
    }
    
    module a {
      let x = 1
    }
    
    from t
    derive {p = a.x, q = a.b.y}
    

    Recompiling that collected output gives the same Unknown name a.b.y.

    declare swap — patched fold_module_def_stmt to call self.root_mod.declare(ident, decl.kind, stmt.id, decl.annotations):

    test result: FAILED. 47 passed; 31 failed; 1 ignored
    
    thread 'semantic::test::test_header' panicked at prqlc/prqlc/src/semantic/mod.rs:249:13:
    called `Result::unwrap()` on an `Err` value: Errors([Error { … reason: Simple("duplicate declarations of std") … }])
    

    Patch reverted; working tree clean.

  2. kgutwin commented on Aug 19, 2026

    @kgutwin
    Collaborator

    I believe that redeclaration of a module should result in an error message, not merging the two modules.

    However, this issue remains open to track discussion on this topic, since there are other maintainers who may wish to comment before a decision is finalized.

    @prql-bot should not open any more PRs that attempt to fix this issue unless specifically directed.

  3. prql-bot commented on Aug 19, 2026

    @prql-bot
    CollaboratorAuthor

    Understood — no further PRs on this unless a maintainer asks for one. #6206 took the merging direction and is now closed; to stop that recurring I've opened #6212, which adds a "parked issues" note to the repo's tend guidance so a future run reads the PRs already closed against an issue before opening its own, rather than relying on a note in the thread.

    One thing worth having on the record for the merge-vs-error decision, re-verified on 7dc4962 just now: this call site also fires with no repeated block anywhere in the source. A project whose root Main.prql declares module a { … } while a sibling directory a/ exists ends up with two ModuleDef stmts for a by the time the resolver runs, and the second wins — so the error direction is also deciding whether a file-tree module and a root-file block of the same name are a collision. prqlc collect serializes such a project to a repeated module a { … } pair (transcript in the earlier comment), so its output would become a hard error under that rule — a collect bug whichever way the semantics land.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions