parse parameter type annotations when generating native query configuration #128
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hallettj
requested review from
codedmart,
daniel-chambers,
danieljharvey and
BenoitRanque
hallettj
changed the base branch from
main
to
jessehallett/eng-1106-support-project-stage-when-generating-native-query-configs
hallettj
changed the base branch from
jessehallett/eng-1106-support-project-stage-when-generating-native-query-configs
to
main
hallettj
force-pushed
the
jessehallett/eng-1249-parse-parameter-type-annotations
branch
from
1ceb7f9
to
aacb803
Compare
hallettj
changed the base branch from
main
to
jessehallett/eng-1106-support-project-stage-when-generating-native-query-configs
codedmart
reviewed
Nov 22, 2024
| @@ -114,7 +117,8 @@ fn unable_to_infer_types_message( | |||
| for name in problem_parameter_types { | |||
| message += &format!("- {name}\n"); | |||
| } | |||
| message += "\nTry adding type annotations of the form: {{parameter_name|[int!]!}}\n"; | |||
| message += "\nTry adding type annotations of the form: {{ parameter_name | [int!]! }}\n"; | |||
| message += "\nIf you added an annotation, and you are still seeing this error then the type you gave may not be compatible with the context where the parameter is used.\n"; | |||
There was a problem hiding this comment.
Can we link to somewhere they can read on supported types?
There was a problem hiding this comment.
I need to do a docs pass on the whole native query DX feature, then I'll have something to link to
codedmart
reviewed
Nov 22, 2024
| }, | ||
|
|
||
| // TODO: We probably want a separate step that swaps ElementOf and FieldOf constraints with | ||
| // constraint of the targeted structure. We might do a similar thing with | ||
| // WithFieldOverrides. |
There was a problem hiding this comment.
I removed the TODO because I don't think we need to worry about this
codedmart
reviewed
Nov 22, 2024
| Ok(()) | ||
| } | ||
|
|
||
| // TODO: |
There was a problem hiding this comment.
Yes - I wrote the test to see if it works, and it doesn't. I want to fix that later so I want to keep the test, but I don't want to commit a failing test.
codedmart
approved these changes
Nov 22, 2024
Base automatically changed from
jessehallett/eng-1106-support-project-stage-when-generating-native-query-configs
to
main
November 22, 2024 22:50
hallettj
force-pushed
the
jessehallett/eng-1249-parse-parameter-type-annotations
branch
from
aacb803
to
30bdfa8
Compare
hallettj
deleted the
jessehallett/eng-1249-parse-parameter-type-annotations
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is work an an in-progress feature that is gated behind a feature flag,
native-query-subcommand.Parses type annotations in placeholders that reference native query parameters. We already have an error message suggesting doing this, now the system actually reads these. For example,
{ "$match": { "imdb.rating": { "$gt": "{{ min_rating | int! }}" } } }The generated query for that configuration includes a parameter named
min_ratingwith typeint. Without the annotation it would have inferreddouble.Type annotations are checked against the inferred type for the position so you will see errors if the annotated type is not compatible. Parameters are treated as contravariant so you can only provide a subtype of the inferred type - for example you can constrain a parameter type to be non-nullable even if it is in a nullable context. There are cases where the type checker cannot infer a type in which case annotations are necessary.
I know we already have a similar parser for type expressions in hml files. I thought it would be easier to write a new one specialized for this connector and for MongoDB scalar types since it's about 100 LOC.
Type expressions use GraphQL syntax to match types as seen in GraphQL and in hml. There is one addition: I invented a syntax for predicate types,
predicate<ObjectTypeName>. The parser happens to be written so that if the angle brackets are absent the wordpredicatewill be interpreted as an object type so we don't have a problem if a user want to use an object type named "predicate".On the other hand this parser does not allow object type names that match MongoDB scalar type names.
While I was working on this I noticed some issues with unifying types in the presence of nullability, and with displaying errors when a type annotation can't be unified with inferred constraints for a parameter's context. So I included some fixes in those areas.