Handling of PRQL source files without main pipeline

[max-sixty]

As discussed in PRQL/prql#1803 (and screenshot at PRQL/prql#1825 (comment)), currently the extension will raise an error on files that don't have main pipelines.

Because we're planning to start allowing "module" files, this will become more prevalent. (At the moment it really only affects the developers who have std.prql open...)

As discussed on the linked issue, these should be possible to parse, even if not compile. So it would be great to understand why we get this error, given I'd think we don't need to compile PRQL files before we want to preview them.

[RandomFractals]

PRQL vscode extension only uses prql compile, always has been. From https://github.com/PRQL/prql-vscode/blob/main/src/compiler.ts#L27

 try {
    // run prql compile
    return prql.compile(prqlString, compileOptions) as string;
  }
  catch (error) {
    if ((error as any)?.message) {
      try {
        const errorMessages = JSON.parse((error as any).message);
        return errorMessages.inner as ErrorMessage[];
      }
      catch (ignored) {
        throw error;
      }
    }
    throw error;
  }

See my comment in the original ticket (PRQL/prql#1803 (comment)) about parse before compile, and any errors that might generate.

We can change this behaviour, but then let's specify what we should call and in what order, and do we still display both parse and compiler errors the way we do now?

Maybe let's change this ticket title to Display Parse Errors? or provide Parse and Compile PRQL options in extension? It's not clear what the ask is here, but sounds like a UX enhancement.

Per current compiler API design, I think that missing main pipeline error is displayed correctly and should be there until there is a better modular prql support implemented and finalized in PRQL compiler as being discussed in the mentioned PRQL repo tickets above.

Please tag me in the discussions of modular PRQL parse/compile design in the core repo. I don't think the proposed custom error code for extension to check is a good solution not just for extension, but for the other PRQL compiler api clients too (Py, CLIs and other plugins).

cc @aljazerzen & @richb-hanover

[RandomFractals]

also, please review the suggested UI design to support multiple pipeline definitions in a single .prql file I shared in discord for the modular PRQL support and Previews initially:

https://discord.com/channels/936728116712316989/1028380004226171032/1078302829380386899

[richb-hanover]

@max-sixty @aljazerzen @RandomFractals As we discuss the "no pipeline" state, it might be useful to have an output state from the compiler that let me type the following without error:

# This is a comment
let x = 2
func a b -> c

If the PRQL that was entered is syntactically correct, perhaps the "no pipeline" could instead be a "No SQL emitted" or "No SQL Query" state. That also could be used in the right pane of the Playground to indicate that there's no error, but that there isn't any SQL (yet). Thanks.

[aljazerzen]

I think that the contract of the compile function should remain "I give you PRQL query and you return me SQL query". If the input is a source file that does not contain a query, this contact is broken and "correct" thing would be to return an error.

I see two ways around this:

1. the extension can check the error code and treat files with error because of missing main pipeline as "definition files" instead of "query files". In practice this would:

   • discard the "no main pipeline error", so file is not marked as containing an error,
   • preview would show "PRQL file does not contain an SQL query",
   • this obviously does not apply when executing the file

2. use function prql_to_pl instead of compile to determine errors in the file. This will only parse and will not run semantic analysis. If we also wanted to run that, we'd have to expose some other function that runs resolve_only without lowering (which produces the "no main pipeline" error). For compiling PRQL for the "SQL preview" we'd still use compile function.

---

and @richb-hanover yes - the error message from the prql-compiler should be improved.

[richb-hanover]

@aljazerzen - Either of these plans addresses my concern. Thanks.

[RandomFractals]

@aljazerzen:

1. This is new functionality request. Let's change this ticket title to reflect that and describe requested functionality better.

2. All DSL compilers parse some definitions code, display definition parse errors, if any, and produce some binary or other code artifacts.

Why should PRQL compiler behave or be any different?

Perhaps prql compile() should change to check and produce parse errors first, then when a PRQL query/pipeline definition is encountered, generate SQL we can show in SQL Preview or terminal for the CLI clients. Otherwise, produce a valid PRQL info message for the Preview just like you do with errors now. Then we can check message type info and display that in Preview.

Still sounds odd that a compiler would generate no result, or that error even for an empty prql document as it does now :)

Also, changing the compiler behavior to parse before compile should simplify its public api too.

3. None of the above addresses the multiple pipeline definitions per .prql file I suggested we add just like Malloy does.

[RandomFractals]

@max-sixty @aljazerzen @RandomFractals As we discuss the "no pipeline" state, it might be useful to have an output state from the compiler that let me type the following without error:

# This is a comment
let x = 2
func a b -> c

If the PRQL that was entered is syntactically correct, perhaps the "no pipeline" could instead be a "No SQL emitted" or "No SQL Query" state. That also could be used in the right pane of the Playground to indicate that there's no error, but that there isn't any SQL (yet). Thanks.

How is this used in PRQL documents now?

Can you provide a full example that actually has some output after those definitions, or uses that var and function?

It does sound like the change should be made in the compiler itself to not produce that error, or generate an Info type message instead.

It would be cleaner to display parse/compile state info messages, rather than some cryptic error codes that will require more documentation.

Besides, we are using standard vscode diagnostics and display those messages in the Problems panel already. Info messages can be displayed there too, and changing the compiler and extension to display parse/compile outcome state this way would be more like how all other built-in and 3rd extensions do it in vscode.

With that, I believe the fix for the reported pipeline error should be made in the compiler first.

[RandomFractals]

Example of info message display in Problems panel in vscode below. If/when PRQL compiler generates info messages for valid prql documents without sql output, we should be showing them there:

[aljazerzen]

I see that we should first define what is a PRQL source file and how it can export stuff. Before we do that, I see that it is confusing to talk about compile function that did receive a valid PRQL source that does export a main pipeline.

[RandomFractals]

@max-sixty @aljazerzen @RandomFractals As we discuss the "no pipeline" state, it might be useful to have an output state from the compiler that let me type the following without error:

# This is a comment
let x = 2
func a b -> c

If the PRQL that was entered is syntactically correct, perhaps the "no pipeline" could instead be a "No SQL emitted" or "No SQL Query" state. That also could be used in the right pane of the Playground to indicate that there's no error, but that there isn't any SQL (yet). Thanks.

so, I just tried it with v0.6.1 ext. and prql compiler update from #150 . That error message looks more informative now.

@max-sixty Not sure if you want to keep this open, or close it and require a query in valid prql files.

[richb-hanover]

@max-sixty @aljazerzen @RandomFractals This is a thing of beauty. Thanks, all.

My one comment is that the message could be "No query" or "No SQL Query". The "Missing query" message could lead the reader to think that there's something wrong. Thanks again.

[max-sixty]

I was thinking that we wouldn't generate an error in VS Code here. Because it's actually fine to have a .prql file with just definitions. Is that consistent with what others were thinking?

---

My one comment is that the message could be "No query" or "No SQL Query". The "Missing query" message could lead the reader to think that there's something wrong. Thanks again.

I can make the change if we all agree! Though maybe this is moot if we're planning to hide the error.

[richb-hanover]

You decide. I don't feel strongly how this sugars out in the end. Thanks