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

deterministic ordering of objects in exported metadata (close #3125) #3230

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 4 commits into from
Nov 6, 2019
Merged

deterministic ordering of objects in exported metadata (close #3125) #3230

merged 4 commits into from
Nov 6, 2019

Conversation

@rakeshkky
Copy link
Member

@rakeshkky rakeshkky commented Oct 24, 2019 •
edited
Loading

Description

Always export metadata objects in a deterministic order. Previously the order depends on how the Postgres supplies the data by default (with simple SELECT without ordering). In this PR, the server explicitly selects the data from Postgres using ORDER BY clause, thus making the export of metadata objects deterministic.

Affected components

  • Server
  • Tests

Related Issues

Close #3125

Solution and Design

  1. Fetch metadata objects from the database using ORDER BY in export_metadata metadata query API.
  2. Use insert ordered hashmap to keep the order of table metadata map.

Steps to test and verify

Define metadata objects in any order but the order of exported metadata should remain the same.

@rakeshkky rakeshkky self-assigned this Oct 24, 2019
@rakeshkky rakeshkky changed the title deterministic ordering of objects in exported metadata, close #3125 deterministic ordering of objects in exported metadata (close #3125) Oct 24, 2019
@netlify
Copy link

netlify bot commented Oct 24, 2019 •
edited
Loading

Deploy preview for hasura-docs ready!

Built with commit 58db57e

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

@arvi3411301
Copy link
Member

arvi3411301 commented Oct 25, 2019

/heroku deploy

@hasura-bot
Copy link
Contributor

hasura-bot commented Oct 25, 2019

Review app for commit 6f80691 deployed to Heroku: https://hge-ci-pull-3230.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3230-6f806914

@hasura-bot
Copy link
Contributor

hasura-bot commented Oct 25, 2019

Review app for commit b080558 deployed to Heroku: https://hge-ci-pull-3230.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3230-b0805587

@rakeshkky rakeshkky mentioned this pull request Oct 28, 2019
Merged
@rakeshkky rakeshkky force-pushed the issue-3125-deterministic-export branch from b080558 to 6f5d599 Compare October 31, 2019 08:51
@hasura-bot
Copy link
Contributor

hasura-bot commented Oct 31, 2019

Review app for commit 6f5d599 deployed to Heroku: https://hge-ci-pull-3230.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3230-6f5d5996

@rakeshkky rakeshkky force-pushed the issue-3125-deterministic-export branch from 6f5d599 to 1724e40 Compare November 4, 2019 06:41
@rakeshkky rakeshkky added the c/server Related to server label Nov 4, 2019
@rakeshkky rakeshkky marked this pull request as ready for review November 4, 2019 06:48
@rakeshkky rakeshkky added the p/high candidate for being included in the upcoming sprint label Nov 4, 2019
@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 4, 2019

Review app for commit 1724e40 deployed to Heroku: https://hge-ci-pull-3230.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3230-1724e407

@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 5, 2019

Review app for commit 58db57e deployed to Heroku: https://hge-ci-pull-3230.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3230-58db57e2

lexi-lambda
lexi-lambda approved these changes Nov 6, 2019
View reviewed changes
Copy link
Contributor

@lexi-lambda lexi-lambda left a comment

Choose a reason for hiding this comment

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

LGTM.

This seems like it could be something that could benefit a lot from random testing, since it could uncover edge cases the handwritten test might have missed (either now or in the future). It seems like it would probably be nontrivial to add, though, so I’m happy to wait and see if it ever becomes an actual problem (since this functionality doesn’t seem deeply critical).

@lexi-lambda lexi-lambda merged commit 1b96ebc into hasura:master Nov 6, 2019
@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 6, 2019

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

@rakeshkky rakeshkky mentioned this pull request Dec 3, 2019
Merged
16 tasks
polRk pushed a commit to polRk/graphql-engine that referenced this pull request Feb 12, 2020
@rakeshkky @polRk
deterministic ordering of objects in exported metadata (close hasura#…
14b8b71 
…3125) (hasura#3230)

* deterministic ordering of objects in exported metadata, close hasura#3125
* refactor 'Hasura.RQL.DDL.Metadata'
@Allymahmoud Allymahmoud mentioned this pull request Aug 31, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@lexi-lambda lexi-lambda lexi-lambda approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rakeshkky rakeshkky

Labels

c/server Related to server p/high candidate for being included in the upcoming sprint

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

deterministic ordering of objects in exported metadata

4 participants

@rakeshkky @arvi3411301 @hasura-bot @lexi-lambda