这是indexloc提供的服务，不要输入任何密码
Skip to content

add a json path argument to query values inside json columns (close #1598) #1661

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 20 commits into from
Mar 25, 2019
Merged

add a json path argument to query values inside json columns (close #1598) #1661

merged 20 commits into from
Mar 25, 2019

Conversation

@hgiasac
Copy link
Contributor

@hgiasac hgiasac commented Feb 25, 2019 •
edited
Loading

Description

What component does this PR affect?

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Requires changes from other components? If yes, please mark the components:

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Related Issue

#1598

Solution and Design

  • Define GraphQL Argument path for json/jsonb type
  • Support jsonPath parsing (WIP)
  • Convert arguments to DML Select
  • If there is the argument, convert to SEOpApp "->" operator

Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Docs update
  • Community content

Checklist:

  • I have read the contributing guide and my code conforms to the guidelines.
  • This change requires a change in the documentation.
  • I have updated the documentation accordingly.
  • I have added required tests.

@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 25, 2019

Beep boop! 🤖

Hey @hgiasac, thanks for your PR!

One of my human friends will review this PR and get back to you as soon as possible. 🕐

Stay awesome! 😎

@CLAassistant
Copy link

CLAassistant commented Feb 25, 2019 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

rakeshkky
rakeshkky reviewed Feb 26, 2019
View reviewed changes
rakeshkky
rakeshkky reviewed Feb 26, 2019
View reviewed changes
rakeshkky
rakeshkky reviewed Feb 26, 2019
View reviewed changes
rakeshkky
rakeshkky reviewed Feb 26, 2019
View reviewed changes
rakeshkky
rakeshkky reviewed Feb 26, 2019
View reviewed changes
rakeshkky
rakeshkky reviewed Feb 26, 2019
View reviewed changes
rakeshkky
rakeshkky reviewed Feb 26, 2019
View reviewed changes
@shahidhk shahidhk changed the title Add GraphQL Query JSON (close #1598) add a json path argument to query values inside json columns (close #1598) Feb 28, 2019
@shahidhk shahidhk added the c/server Related to server label Feb 28, 2019
@hgiasac
Copy link
Contributor Author

hgiasac commented Feb 28, 2019

@rakeshkky I update the code with JSONPath parser and test cases. Can you review it? Thanks

rakeshkky
rakeshkky reviewed Mar 1, 2019
View reviewed changes
rakeshkky
rakeshkky reviewed Mar 1, 2019
View reviewed changes
rakeshkky
rakeshkky reviewed Mar 1, 2019
View reviewed changes
@rakeshkky
Copy link
Member

rakeshkky commented Mar 1, 2019

@hgiasac please apply stylish haskell on files you updated.

rakeshkky
rakeshkky reviewed Mar 1, 2019
View reviewed changes
rakeshkky
rakeshkky reviewed Mar 1, 2019
View reviewed changes
rakeshkky
rakeshkky reviewed Mar 1, 2019
View reviewed changes
@hgiasac
Copy link
Contributor Author

hgiasac commented Mar 2, 2019 •
edited
Loading

@rakeshkky I update new code. Please check if you have time, thanks.

Attoparsec prefers performance, so several parse error and message are skipped. I work around with some manual code. Please leave some advice If you get any better improvement. Thanks

Have a nice weekend!

rakeshkky
rakeshkky reviewed Mar 2, 2019
View reviewed changes
@shahidhk
Copy link
Member

shahidhk commented Mar 8, 2019

@coco98 looks like preview app is now deployed. https://deploy-preview-1661--hasura-docs.netlify.com/graphql/manual/queries/simple-object-queries.html#fetch-json-fields

@shahidhk
Copy link
Member

shahidhk commented Mar 8, 2019

@arvi3411301 Can you check why Heroku preview app was never created for this PR?

@arvi3411301
Copy link
Member

arvi3411301 commented Mar 12, 2019

@shahidhk Review apps will be deployed only when the circleci tests were run under hasura org.

@shahidhk
Copy link
Member

shahidhk commented Mar 20, 2019

@arvi3411301, what does that mean? This PR's tests are running on a different org?

@netlify
Copy link

netlify bot commented Mar 22, 2019 •
edited
Loading

Deploy preview for hasura-docs ready!

Built with commit d6fcca5

https://deploy-preview-1661--hasura-docs.netlify.com

@shahidhk shahidhk mentioned this pull request Mar 22, 2019
Closed
@shahidhk
Copy link
Member

shahidhk commented Mar 22, 2019 •
edited
Loading

@hgiasac Can you update master and fix the conflicts? Thanks. This is ready to be merged. @dsandip has made some docs updates. Thanks.

@arvi3411301
Copy link
Member

arvi3411301 commented Mar 22, 2019

@shahidhk yes. https://circleci.com/gh/hgiasac/graphql-engine/219?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

@shahidhk
Copy link
Member

shahidhk commented Mar 25, 2019

@hgiasac Can you disable CircleCI on your repo?

@hgiasac
Copy link
Contributor Author

hgiasac commented Mar 25, 2019

Done. Sorry about that

@shahidhk shahidhk marked this pull request as ready for review March 25, 2019 05:04
@shahidhk
Copy link
Member

shahidhk commented Mar 25, 2019

Thanks! 👍

@hasura-bot
Copy link
Contributor

hasura-bot commented Mar 25, 2019

Review app for commit 0c1d6af deployed to Heroku: https://hge-ci-pull-1661.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1661-0c1d6af

rikinsk-zz
rikinsk-zz approved these changes Mar 25, 2019
View reviewed changes
Copy link

@rikinsk-zz rikinsk-zz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs LGTM

@hasura-bot
Copy link
Contributor

hasura-bot commented Mar 25, 2019

Review app for commit ea2f319 deployed to Heroku: https://hge-ci-pull-1661.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1661-ea2f319

@hasura-bot
Copy link
Contributor

hasura-bot commented Mar 25, 2019

Review app for commit d6fcca5 deployed to Heroku: https://hge-ci-pull-1661.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1661-d6fcca5

@shahidhk shahidhk merged commit 560c31f into hasura:master Mar 25, 2019
@hasura-bot
Copy link
Contributor

hasura-bot commented Mar 25, 2019

Beep boop! 🤖

Whoa! 🎉 🎉 💃

GIF

Awesome work @hgiasac! 💪 🏆 All of us at Hasura ❤️ what you did.

Thanks again 🤗

@hasura-bot
Copy link
Contributor

hasura-bot commented Mar 25, 2019

Review app https://hge-ci-pull-1661.herokuapp.com is deleted

hasura-bot pushed a commit that referenced this pull request Mar 3, 2025
@danieljharvey @hasura-bot
Ensure we are allowed to access built-in operators in OpenDD (#1661)
666dc0e 
<!-- The PR description should answer 2 important questions: -->

### What

Previously, using non-custom `ComparisonOperator` enum values meant we
would not lookup in the boolean expression type to check we are allowed
to use it. Now we do a reverse lookup from the found
`DataConnectorOperatorName`.

There is a small complexity / compromise here. In OpenDD, the filter
from a `ModelSelectOne` (ie, a simple primary key comparison) is
possible without a `BooleanExpressionType` being defined. Once the
`OpenDD IR` is created there is no way to differentiate these filters
from others. Therefore, we allow `ComparisonOperator::Equals` to skip
this reverse lookup, so long as the filtering is happening at the root
(ie, not within an array, nested field or relationship).

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

Reviewers

@0x777 0x777 0x777 approved these changes

@rakeshkky rakeshkky rakeshkky left review comments

+1 more reviewer

@rikinsk-zz rikinsk-zz rikinsk-zz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c/server Related to server

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.