Improve invalid argument error #416
Merged
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
obi1kenobi
reviewed
Jul 19, 2019
…chnologies/graphql-compiler into improve-invalid-argument-error
added 2 commits
July 22, 2019 11:11
…chnologies/graphql-compiler into improve-invalid-argument-error
obi1kenobi
approved these changes
Jul 22, 2019
vmaksimovski
pushed a commit
that referenced
this pull request
Jul 25, 2019
* Partial implementation of the Cypher lowering and query emission code. * Refactor the common IR lowering steps into their own subpackage. * Make location rewriting always use the query metadata table. * Implement conversion of IR blocks to CypherQuery object. * Finish up IR lowering. Implement entry points into Cypher compilation. * Add Cypher expression formatting, and some fixes for Gremlin lowering. * Various small bugfixes discovered through addition of Cypher test code. * Merge master and fix minor bugs. * Implement proper optional + filter semantics. * Enable a few Cypher tests. * Fix recursive Cypher, and add more test cases. * Remove merge artifact. * Record the GraphQL type of LocalField expressions. Required to make progress on Cypher backend, since a lowering step there converts LocalField expressions into ContextField expressions. Prior to this commit, ContextField expressions include the GraphQL type of the field, whereas LocalField expressions do not, making that lowering step impossible to implement. * Properly lower LocalField into ContextField expressions, preserving type info. * Add support for filtering within optional traversals. * Delint. * Add tests For not_in_collection and not_contains filters as well as contains and in_collection tests that were missing. * Delint * Change Cypher version * Add filter operators documentation README documentation added for filter operators not_in_collection and not_contains. * Fix formatting * Add "vertex" after "Animal" * Clarify what filter returns * Emitting Cypher and some compilation tests (#374) * Partial implementation of the Cypher lowering and query emission code. * Refactor the common IR lowering steps into their own subpackage. * Make location rewriting always use the query metadata table. * Implement conversion of IR blocks to CypherQuery object. * Finish up IR lowering. Implement entry points into Cypher compilation. * Add Cypher expression formatting, and some fixes for Gremlin lowering. * Various small bugfixes discovered through addition of Cypher test code. * Merge master and fix minor bugs. * Implement proper optional + filter semantics. * Enable a few Cypher tests. * Fix recursive Cypher, and add more test cases. * Remove merge artifact. * Record the GraphQL type of LocalField expressions. Required to make progress on Cypher backend, since a lowering step there converts LocalField expressions into ContextField expressions. Prior to this commit, ContextField expressions include the GraphQL type of the field, whereas LocalField expressions do not, making that lowering step impossible to implement. * Properly lower LocalField into ContextField expressions, preserving type info. * Add support for filtering within optional traversals. * Delint. * Fix SchemaGraph connection documentation * Rename variables Small change suggested at #369 (review) but better to split that off the integration test PR. * Add schema merging without tests * Nit * Nit * Fix GraphQLScalarType object generation from schema text (#388) * Add test for date parameter bug * Add serialization and parsing for custom scalar types inferred from a schema string * Add missing birthday fields in OrientDB integration data * Fix linter issues * Undo README.md changes * Remove GraphQLString as a custom scalar type * Remove disabling of pylint messages * Various name changes * Add name validity check * Renamed schema (#390) * Add schema renaming without tests * Small name change * Fix lint errors * Change to using function, not constructor RenamedSchema is now a lightweight namedtuple that can be made by the function rename_schema RenameTypesVisitor has shrunk in size, without repeated method definitions frozensets are used in various places * Add copyright lines * Address comments * Address comments * Address comments * Fix lint errors * Pack validity checking into helper; some name changes * Use ast instead of string * Address comments, various name changes * Change output dict to only contain modified name pairs * Fix lint issues * Nit * Add name validity check * Fix lint errors and import issues * Nit * Missed micro nit * Micro nit grammar * Add underscore to pattern * Remove validity/conflict checking from field renaming; add error docs * Fix lint error * Nit * Small comments update * Fix lint error * Add tests for rename_schema * Nit * More tests; relative imports * Address some comments * Rename tests; add init file to schema_transformation * Implement Cypher support for Neo4j and Redisgraph (#381) * Compiler changes for integration tests * Nits * Undo Cypher version change Makes more sense to make this change in a separate branch, so I'm undoing it here. * nits * Implement _safe_cypher_* methods * delint * Fix python version string encoding comment * Manually interpolate parameters for Redisgraph only Neo4j's client can automatically do its own interpolation so it's better than doing it ourselves. Unfortunately Redisgraph doesn't support parameters at all, so we have to interpolate them ourselves. * Revert "delint" This reverts commit eddb5e2. This doesn't have anything to do with compiler changes to support Cypher for Neo4j and Redisgraph-- it was just the linter getting overexcited. * lint * Address comments * Remove Cypher version from queries Neo4j does accept Cypher version in queries but it doesn't match the OpenCypher docs, which specifies Cypher 9. Instead, it only accepts 2.3, 3.4, and 3.5 according to this Github issue: neo4j/neo4j#12239 RedisGraph doesn't support the version number syntax at all, according to this Github issue: RedisGraph/RedisGraph#552 * Fix _safe_cypher_string comment * Nits * Nit * Nit * Split renaming and validation * Revert to deepcopy * Split tests into validation and renaming * All validity checking moved into one func; some name and docstring changes * Remove unnecessary name validity checks; small comment changes * Add reminder to install dependencies after adding package Previously the instructions just said to run the script and I didn't realize this didn't work until I'd run `pipenv sync --dev`. * Fix typo * Change name of MergedSchema attribute * Remove deepcopy * Nit (#402) Breaking up PR #369 (integration tests) into more logical chunks. This is a url formatting nit. * Add Docker containers to docker-compose.yml (#403) Breaking up PR #369 (integration tests) into more logical chunks. * Create integration data for Cypher backends (#405) The data here is meant to match the data in the file create_orientdb_integration.sql Breaking up PR #369 (integration tests) into more logical chunks. * Update Pipfile for Cypher backends (#404) * Update Pipfile for Cypher backends Add relevant libraries for integration tests later. Breaking up PR #369 (integration tests) into more logical chunks. * Move Cypher backend libraries to dev-packages only These are only used for integration tests, which will come later. * Fix Pipfile versioning * Create integration test clients Breaking up PR #369 (integration tests) into more logical chunks. * Add comment explaning RedisGraph vs. Neo4j temporal type support (#407) * Cypher support graph generation (#408) * Implement integration test graph setup Takes in the .cypher files and creates the graph needed for the integration tests. Breaking up PR #369 (integration tests) into more logical chunks. * Add missing docstrings * Create integration test fixtures (#410) * Create integration test fixtures Currently waiting on clients and data_tool to get merged into master. Breaking up PR #369 (integration tests) into more logical chunks. * Remove unnecessary keyword from retry decorator * Add missing docstrings * Change database name into global variable * Nit * Delint * Refactor merge_schemas (#406) * Address comments on merge_schemas * Address comments * Remove util change meant for query renaming * Update comments and docstrings * Address comments * Further detail error message * Import COMPILED_NAME_PATTERN and remove _graphql_type_name_pattern (#418) * Import COMPILED_NAME_PATTERN and remove _graphql_type_name_pattern * Remove unused import * Create the integration tests themselves for Cypher backends (#411) * Create the integration tests themselves for Cypher backends Breaking up PR #369 (integration tests) into more logical chunks. * Add comment explaining excluded test * Combine backend decorators into one function * Indentation nit * Fix silly mistake for backend choice * Move docstring explanation into its own comment * Amend custom GraphQLScalarType objects in parsed schemas (#398) * Amend custom GraphQLScalarType in parsed schemas * Nit * Fix linter issues * Undo unintented changes * Address comments * Nit * Nit * Improve invalid argument error (#416) * Improve invalid argument error * Nit * Nit * Nit * Fix typo * Add newline (#423) * Improve schema graph documentation (#413) * Improve SchemaGraph docummentation * Nit * Nits * Nit * Nit * Remove added line * Nit * Fix typo * Address comments * Nit * Address comments * Improve string validation error (#414) * Improve validation error message. * Nit * Nit * Improve argument or output name validation error * Improve safe string validation error * Refacto validate safe string into validate_safe_or_special_string * Nit * Mention which are the illegal characters in string validation method * Replace validate_safe_or_special_string with validate_safe_string in many places * Stop special casing space error message. * Replace validate_safe_or_special_string with validate_safe_string in many places * Fix failing tests by loosening validation * Made the validation for cypher expression names more tight * Release v1.11.0. (#396) * Add version v1.11.0 to changelog * Nit * Fix changelog * Add missing parts to the changelog * Remove mention of indexes and mention GraphQL schema generation * Update changelog with pr links * Add missing filter operations * Add amend_custom_scalar_types to changelog * Release v1.11.0. * Update CHANGELOG.md * Add cost estimation and validation error message notes to the changelog. * Add tests for merge_schemas (#400) * Add tests for merge_schemas * Add test for checking original ASTs unmodified * Fix declaration of ordered dicts * Explain output name id map in doc string * Undo changes from other branch (add back deepcopy) * Remove deepcopy again * Various name changes, tests for identifier * Temporarily disable neo4j tests (#426) * Cypher support emit output tests (#379) * Test Cypher emit output * delint * Revert "delint" This reverts commit 35d211c. Undo a lot of changes automatically done by a linter. * Delint properly * Remove "CYPHER 3.5" from tests * Undo unnecessary lint See here: https://github.com/kensho-technologies/graphql-compiler/pull/379/files#r304957815 * Use test schema in Cypher emit output tests * Nit * Add missing comment * Remove extra newlines * Update MATCH emit output tests (#421) * Add GlobalOperationsStart block to each ir_blocks list Addresses comment from PR #379 #379 (comment) * Fix imports * Adapt MATCH emit output tests to use test schema * Fix incorrect emit output test test_simple_traverse_filter_output() is supposed to comapre a child vertex's name with the parent's name, and just comparing to a variable named `desired_name` defeats the point of this test. EmitGremlinTests seems to do it right. * Nit * remove newline * Update gremlin emit output tests (#422) * Find and replace to match test schema Foo -> Animal Bar -> BornAt etc * Add global blocks * Add equivalent GraphQL strings for reference * Add test_datetime_variable_representation() test for Gremlin This test existed for MATCH but not for Gremlin * Replace received_match with received_gremlin * Lint * Delint * remove newline * Delint * Improve error message for contains and not_contains operations (#425) * Improve error message for contains and not_contains operations * Add missing dot at the end * Fix typo * Add back neo4j client (#428)
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.
No description provided.