-
Notifications
You must be signed in to change notification settings - Fork 244
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
Experiment with multi-file schema handling in PSL #4243
Conversation
CodSpeed Performance ReportMerging #4243 will degrade performances by 5.45%Comparing Summary
Benchmarks breakdown
|
14c28e3
to
ccf1968
Compare
ccf1968
to
f34efb7
Compare
WASM Query Engine file Size
|
❌ WASM query-engine performance will worsen by 1.53%Full benchmark report
After changes in 3fee3aa |
This commit implements multi-file schema handling in the Prisma Schema Language. At a high level, instead of accepting a single string, `psl::validate_multi_file()` is an alternative to `psl::validate()` that accepts something morally equivalent to: ```json { "./prisma/schema/a.prisma": "datasource db { ... }", "./prisma/schema/nested/b.prisma": "model Test { ... }" } ``` There are tests for PSL validation with multiple schema files, but most of the rest of engines still consumes the single file version of `psl::validate()`. The implementation and the return type are shared between `psl::validate_multi_file()` and `psl::validate()`, so the change is completely transparent, other than the expectation of passing in a list of (file_name, file_contents) instead of a single string. The `psl::validate()` entry point should behave exactly the same as `psl::multi_schema()` with a single file named `schema.prisma`. In particular, it has the exact same return type. Implementation ============== This is achieved by extending `Span` to contain, in addition to a start and end offset, a `FileId`. The `FileId` is a unique identifier for a file and its parsed `SchemaAst` inside `ParserDatabase`. The identifier types for AST items in `ParserDatabase` are also extended to contain the `FileId`, so that they can be uniquely referred to in the context of the (multi-file) schema. After the analysis phase (the `parser_database` crate), consumers of the analyzed schema become multi-file aware completely transparently, no change is necessary in the other engines. The only changes that will be required at scattered points across the codebase are the `psl::validate()` call sites that will need to receive a `Vec<Box<Path>, SourceFile>` instead of a single `SourceFile`. This PR does _not_ deal with that, but it makes where these call sites are obvious by what entry points they use: `psl::validate()`, `psl::parse_schema()` and the various `*_assert_single()` methods on `ParserDatabase`. The PR contains tests confirming that schema analysis, validation and displaying diagnostics across multiple files works as expected. Status of this PR ================= This is going to be directly mergeable after review, and it will not affect the current schema handling behaviour when dealing with a single schema file. Next steps ========== - Replace all calls to `psl::validate()` with calls to `psl::validate_multi_file()`. - The `*_assert_single()` calls should be progressively replaced with their multi-file counterparts across engines. - The language server should start sending multiple files to prisma-schema-wasm in all calls. This is not in the spirit of the language server spec, but that is the most immediate solution. We'll have to make `range_to_span()` in `prisma-fmt` multi-schema aware by taking a FileId param. Links ===== Relevant issue: prisma/prisma#2377 Also see the [internal design doc](https://www.notion.so/prismaio/Multi-file-Schema-24d68fe8664048ad86252fe446caac24?d=68ef128f25974e619671a9855f65f44d#2889a038e68c4fe1ac9afe3cd34978bd).
f34efb7
to
1e42d35
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, let's
This commit implements multi-file schema handling in the Prisma Schema Language.
At a high level, instead of accepting a single string,
psl::validate_multi_file()
is an alternative topsl::validate()
that accepts something morally equivalent to:There are tests for PSL validation with multiple schema files, but most of the rest of engines still consumes the single file version of
psl::validate()
. The implementation and the return type are shared betweenpsl::validate_multi_file()
andpsl::validate()
, so the change is completely transparent, other than the expectation of passing in a list of (file_name, file_contents) instead of a single string. Thepsl::validate()
entry point should behave exactly the same aspsl::multi_schema()
with a single file namedschema.prisma
. In particular, it has the exact same return type.Implementation
This is achieved by extending
Span
to contain, in addition to a start and end offset, aFileId
. TheFileId
is a unique identifier for a file and its parsedSchemaAst
insideParserDatabase
. The identifier types for AST items inParserDatabase
are also extended to contain theFileId
, so that they can be uniquely referred to in the context of the (multi-file) schema. After the analysis phase (theparser_database
crate), consumers of the analyzed schema become multi-file aware completely transparently, no change is necessary in the other engines.The only changes that will be required at scattered points across the codebase are the
psl::validate()
call sites that will need to receive aVec<Box<Path>, SourceFile>
instead of a singleSourceFile
. This PR does not deal with that, but it makes where these call sites are obvious by what entry points they use:psl::validate()
,psl::parse_schema()
and the various*_assert_single()
methods onParserDatabase
.The PR contains tests confirming that schema analysis, validation and displaying diagnostics across multiple files works as expected.
Status of this PR
This is going to be directly mergeable after review, and it will not affect the current schema handling behaviour when dealing with a single schema file.
Next steps
psl::validate()
with calls topsl::validate_multi_file()
.*_assert_single()
calls should be progressively replaced with their multi-file counterparts across engines.range_to_span()
inprisma-fmt
multi-schema aware by taking a FileId param.Links
Relevant issue: prisma/prisma#2377
Also see the internal design doc.
Close prisma/team-orm#1034